[PATCH] D84244: [clang] Disable -Wsuggest-override for unittests/

logan-5 added a comment. In D84244#2169142 , @hans wrote: > Looks good to me so far. We haven't tried doing any full builds yet, but I > checked for the specific problem I hit yesterday (building > ClangApplyReplacementsTests) and that's working fine

[PATCH] D84244: [clang] Disable -Wsuggest-override for unittests/

hans added a comment. > In that case, I've enabled it again using `add_compile_options` instead of > `add_definitions`. I've got my finger hovering over the button to revert. (For reference, the re-commit is 77e0e9e .) Looks

[PATCH] D84244: [clang] Disable -Wsuggest-override for unittests/

logan-5 added a comment. In D84244#2167663 , @dblaikie wrote: > Sometimes it's OK to commit stuff an see how it goes on the buildbots - doing > so out of peak times and with the intention of rolling back > quickly/immediately if the buildbots report

[PATCH] D84244: [clang] Disable -Wsuggest-override for unittests/

dblaikie added a comment. In D84244#2167661 , @logan-5 wrote: > In D84244#2167625 , @hans wrote: > > > I don't really know why this doesn't happen with other warning flags, but I > > think it would be better to

[PATCH] D84244: [clang] Disable -Wsuggest-override for unittests/

logan-5 added a comment. In D84244#2167625 , @hans wrote: > I don't really know why this doesn't happen with other warning flags, but I > think it would be better to add flags like this with add_compile_options > rather than add_compile_definitions. I

[PATCH] D84244: [clang] Disable -Wsuggest-override for unittests/

hans added a comment. In D84244#2167602 , @logan-5 wrote: > Thanks for reverting--I agree that that's the right move at this point. > > Pretty much totally out my depth here, and I don't have a way of debugging > the Windows issue, so I'm not sure how to

[PATCH] D84244: [clang] Disable -Wsuggest-override for unittests/

logan-5 added a comment. Thanks for reverting--I agree that that's the right move at this point. Pretty much totally out my depth here, and I don't have a way of debugging the Windows issue, so I'm not sure how to proceed. I'm tempted to just let this whole thing rest for now and maybe try

[PATCH] D84244: [clang] Disable -Wsuggest-override for unittests/

hans added inline comments. Comment at: clang/unittests/CMakeLists.txt:14 +if (CXX_SUPPORTS_SUGGEST_OVERRIDE_FLAG) + add_definitions("-Wno-suggest-override") +endif() Because it's added as a define, this gets passed to rc.exe in Windows builds, which is used

[PATCH] D84244: [clang] Disable -Wsuggest-override for unittests/

logan-5 added a comment. In D84244#2165592 , @dblaikie wrote: > Looks OK (for future reference, this sort of stuff should've been cleaned up > before enabling the flag so as to avoid this kind of hurry/breakage/etc) Loud and clear. I honestly thought I

[PATCH] D84244: [clang] Disable -Wsuggest-override for unittests/

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. Looks OK (for future reference, this sort of stuff should've been cleaned up before enabling the flag so as to avoid this kind of hurry/breakage/etc) Repository: rG LLVM Github

[PATCH] D84244: [clang] Disable -Wsuggest-override for unittests/

logan-5 added a comment. Committing this now to fix the bots, as per discussion in https://reviews.llvm.org/D84126. I'll happily revert and approach it differently later if anyone requests that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D84244: [clang] Disable -Wsuggest-override for unittests/

logan-5 added a comment. In D84244#2164537 , @Quuxplusone wrote: > For GTest and GMock specifically, it would be a good medium-term idea to try > to upstream patches into those libraries. I did eventually have success > upstreaming fixes for

[PATCH] D84244: [clang] Disable -Wsuggest-override for unittests/

Quuxplusone added a comment. For GTest and GMock specifically, it would be a good medium-term idea to try to upstream patches into those libraries. I did eventually have success upstreaming fixes for `-Wdeprecated` into GTest, for example: https://github.com/google/googletest/pull/2815

[PATCH] D84244: [clang] Disable -Wsuggest-override for unittests/

logan-5 created this revision. logan-5 added a reviewer: mgorny. logan-5 added a project: clang. Herald added a subscriber: cfe-commits. Yesterday I enabled `-Wsuggest-override` in the main LLVM build and have been fighting with the -Werror bots ever since. The key culprits making this