[PATCH] D82617: Disable GCC's -Woverloaded-virtual, which has false positives.

dblaikie added a comment. In D82617#2119394 , @sammccall wrote: > Abandoning, we'll do this in clangd or find an acceptable way to silence it > (see D82736 ). > > In D82617#2119144

[PATCH] D82617: Disable GCC's -Woverloaded-virtual, which has false positives.

sammccall abandoned this revision. sammccall added a comment. Abandoning, we'll do this in clangd or find an acceptable way to silence it (see D82736 ). In D82617#2119144 , @dblaikie wrote: > In D82617#2119138

[PATCH] D82617: Disable GCC's -Woverloaded-virtual, which has false positives.

sammccall added a comment. > Guess perhaps a different question: Why don't you want this for clangd? Does > it make the codebase better by not adhering to this particular warning? Yes, exactly. (Sorry if this wasn't explicit). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D82617: Disable GCC's -Woverloaded-virtual, which has false positives.

dblaikie added a comment. In D82617#2119138 , @sammccall wrote: > > Guess perhaps a different question: Why don't you want this for clangd? > > Does it make the codebase better by not adhering to this particular warning? > > Yes, exactly. (Sorry if this w

[PATCH] D82617: Disable GCC's -Woverloaded-virtual, which has false positives.

dblaikie added a comment. In D82617#2118253 , @sammccall wrote: > In D82617#2118118 , @dblaikie wrote: > > > In D82617#2117441 , @sammccall > > wrote: > > > > > In D82617#21

[PATCH] D82617: Disable GCC's -Woverloaded-virtual, which has false positives.

sammccall added a comment. In D82617#2118118 , @dblaikie wrote: > In D82617#2117441 , @sammccall wrote: > > > In D82617#2117086 , @Quuxplusone > > wrote: > > > > > FWIW, I t

[PATCH] D82617: Disable GCC's -Woverloaded-virtual, which has false positives.

dblaikie added a comment. In D82617#2117441 , @sammccall wrote: > In D82617#2117086 , @Quuxplusone > wrote: > > > FWIW, I think the example you gave is **correct** for GCC to warn on. > > > Everything the warning s

[PATCH] D82617: Disable GCC's -Woverloaded-virtual, which has false positives.

Quuxplusone added a comment. > what would be really useful is to clarify your position on whether the > warning in clang is delivering value I have no special information on that. Clearly someone thought it was adding value when they added it (way back in 2009; https://github.com/llvm/llvm-pro

[PATCH] D82617: Disable GCC's -Woverloaded-virtual, which has false positives.

sammccall added a comment. > indicating how I would disentangle that code if I owned it. Hopefully that > comment clarifies my position somewhat. So your position on how to best write that code is interesting (I disagree, but I think there are probably types of projects where your priorities ar

[PATCH] D82617: Disable GCC's -Woverloaded-virtual, which has false positives.

Quuxplusone added a comment. In D82617#2117441 , @sammccall wrote: > In D82617#2117086 , @Quuxplusone > wrote: > > > FWIW, I think the example you gave is **correct** for GCC to warn on. > > > Everything the warnin

[PATCH] D82617: Disable GCC's -Woverloaded-virtual, which has false positives.

sammccall added a comment. In D82617#2117086 , @Quuxplusone wrote: > FWIW, I think the example you gave is **correct** for GCC to warn on. Everything the warning says is correct, but in this pattern the polymorphic usage is the whole point of the hierar

[PATCH] D82617: Disable GCC's -Woverloaded-virtual, which has false positives.

Quuxplusone added a comment. FWIW, I think the example you gave is **correct** for GCC to warn on. You said: class Base { virtual void foo(); // to be overridden void foo(int); // implemented in terms of foo() }; foo(int) is hidden in derived classes. So if someone derives from Base

[PATCH] D82617: Disable GCC's -Woverloaded-virtual, which has false positives.

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. LGTM, thanks! This was also my final plan after investigating in https://reviews.llvm.org/D81920, but then I forgot ... Comment at: clang/CMakeLists.txt:398 + if (CMAK

[PATCH] D82617: Disable GCC's -Woverloaded-virtual, which has false positives.

sammccall created this revision. sammccall added reviewers: kadircet, uabelho, bjope. Herald added subscribers: cfe-commits, mgorny. Herald added a project: clang. Currently this warning is on for both clang and GCC, when building clang. It's off for the rest of LLVM, so my sense is it isn't that