dblaikie added subscribers: sammccall, dblaikie. dblaikie added a comment. In D80531#2069637 <https://reviews.llvm.org/D80531#2069637>, @kwk wrote:
> In D80531#2069383 <https://reviews.llvm.org/D80531#2069383>, @njames93 wrote: > >> LGTM with 2 small nits, but I'd still give a few days see if anyone else >> wants to weight in. > > I'm okay with this but I'd like to understand when it's time to wait for > others? Only when a patch is approved or from the beginning of patch's life? > I mean who looks at patches that are approved unless you filter for the > amount of approvers? How many approvers must there be? FWIW, I tend to - I have a queue of things to review, which doesn't have /much/ to do with whether they've already been reviewed. Sometimes, sometimes not - usually if I've decided it's worth looking at I'll still at least glance over it (enough to see if someone's asking for further consensus, etc). While this is rather belated feedback, I've been seeing failures of this test consistently since it was committed, I think. Maybe there's something corrupted in my local client, but figured I'd mention/ask about it, in case it's useful. (+ @sammccall too): FAIL: Clang Tools :: clang-tidy/checkers/modernize-replace-disallow-copy-and-assign-macro.cpp (18811 of 74169) ******************** TEST 'Clang Tools :: clang-tidy/checkers/modernize-replace-disallow-copy-and-assign-macro.cpp' FAILED ******************** Script: -- : 'RUN: at line 1'; /usr/bin/python3.8 /usr/local/google/home/blaikie/dev/llvm/src/clang-tools-extra/test/../test/clang-tidy/check_clang_tidy.py -format-style=LLVM -check-suffix=DEFAULT /usr/local/google/home/blaikie/dev/llvm/src/clang-tools-extra/test/clang-tidy/checkers/modernize-replace-disallow-copy-and-assign-macro.cpp modernize-replace-disallow-copy-and-assign-macro /usr/local/google/home/blaikie/dev/llvm/build/default/tools/clang/tools/extra/test/clang-tidy/checkers/Output/modernize-replace-disallow-copy-and-assign-macro.cpp.tmp : 'RUN: at line 4'; /usr/bin/python3.8 /usr/local/google/home/blaikie/dev/llvm/src/clang-tools-extra/test/../test/clang-tidy/check_clang_tidy.py -format-style=LLVM -check-suffix=DIFFERENT-NAME /usr/local/google/home/blaikie/dev/llvm/src/clang-tools-extra/test/clang-tidy/checkers/modernize-replace-disallow-copy-and-assign-macro.cpp modernize-replace-disallow-copy-and-assign-macro /usr/local/google/home/blaikie/dev/llvm/build/default/tools/clang/tools/extra/test/clang-tidy/checkers/Output/modernize-replace-disallow-copy-and-assign-macro.cpp.tmp -config="{CheckOptions: [ {key: modernize-replace-disallow-copy-and-assign-macro.MacroName, value: MY_MACRO_NAME}]}" : 'RUN: at line 10'; /usr/bin/python3.8 /usr/local/google/home/blaikie/dev/llvm/src/clang-tools-extra/test/../test/clang-tidy/check_clang_tidy.py -format-style=LLVM -check-suffix=FINALIZE /usr/local/google/home/blaikie/dev/llvm/src/clang-tools-extra/test/clang-tidy/checkers/modernize-replace-disallow-copy-and-assign-macro.cpp modernize-replace-disallow-copy-and-assign-macro /usr/local/google/home/blaikie/dev/llvm/build/default/tools/clang/tools/extra/test/clang-tidy/checkers/Output/modernize-replace-disallow-copy-and-assign-macro.cpp.tmp -config="{CheckOptions: [ {key: modernize-replace-disallow-copy-and-assign-macro.MacroName, value: DISALLOW_COPY_AND_ASSIGN_FINALIZE}]}" : 'RUN: at line 16'; clang-tidy /usr/local/google/home/blaikie/dev/llvm/src/clang-tools-extra/test/clang-tidy/checkers/modernize-replace-disallow-copy-and-assign-macro.cpp -checks="-*,modernize-replace-disallow-copy-and-assign-macro" -config="{CheckOptions: [ {key: modernize-replace-disallow-copy-and-assign-macro.MacroName, value: DISALLOW_COPY_AND_ASSIGN_MORE_AGUMENTS}]}" | count 0 : 'RUN: at line 21'; clang-tidy /usr/local/google/home/blaikie/dev/llvm/src/clang-tools-extra/test/clang-tidy/checkers/modernize-replace-disallow-copy-and-assign-macro.cpp -checks="-*,modernize-replace-disallow-copy-and-assign-macro" -config="{CheckOptions: [ {key: modernize-replace-disallow-copy-and-assign-macro.MacroName, value: DISALLOW_COPY_AND_ASSIGN_NEEDS_PREEXPANSION}]}" | count 0 -- Exit Code: 1 Command Output (stdout): -- Running ['clang-tidy', '/usr/local/google/home/blaikie/dev/llvm/build/default/tools/clang/tools/extra/test/clang-tidy/checkers/Output/modernize-replace-disallow-copy-and-assign-macro.cpp.tmp.cpp', '-fix', '--checks=-*,modernize-replace-disallow-copy-and-assign-macro', '-format-style=LLVM', '--', '-std=c++11', '-nostdinc++']... ------------------------ clang-tidy output ----------------------- 1 warning generated. /usr/local/google/home/blaikie/dev/llvm/build/default/tools/clang/tools/extra/test/clang-tidy/checkers/Output/modernize-replace-disallow-copy-and-assign-macro.cpp.tmp.cpp:33:3: warning: prefer deleting copy constructor and assignment operator over using macro 'DISALLOW_COPY_AND_ASSIGN' [modernize-replace-disallow-copy-and-assign-macro] DISALLOW_COPY_AND_ASSIGN(TestClass1); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/local/google/home/blaikie/dev/llvm/build/default/tools/clang/tools/extra/test/clang-tidy/checkers/Output/modernize-replace-disallow-copy-and-assign-macro.cpp.tmp.cpp:33:3: note: FIX-IT applied suggested code changes clang-tidy applied 1 of 1 suggested fixes. ------------------------------------------------------------------ ------------------------------ Fixes ----------------------------- --- /usr/local/google/home/blaikie/dev/llvm/build/default/tools/clang/tools/extra/test/clang-tidy/checkers/Output/modernize-replace-disallow-copy-and-assign-macro.cpp.tmp.cpp.orig 2020-09-09 16:32:20.416366536 -0700 +++ /usr/local/google/home/blaikie/dev/llvm/build/default/tools/clang/tools/extra/test/clang-tidy/checkers/Output/modernize-replace-disallow-copy-and-assign-macro.cpp.tmp.cpp 2020-09-09 16:32:20.476366112 -0700 @@ -30,7 +30,8 @@ class TestClass1 { private: - DISALLOW_COPY_AND_ASSIGN(TestClass1); + TestClass1(const TestClass1 &) = delete; + const TestClass1 &operator=(const TestClass1 &) = delete; }; // // <lots more similar failures> Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80531/new/ https://reviews.llvm.org/D80531 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
