sammccall added a comment. In D82617#2118118 <https://reviews.llvm.org/D82617#2118118>, @dblaikie wrote:
> In D82617#2117441 <https://reviews.llvm.org/D82617#2117441>, @sammccall wrote: > > > In D82617#2117086 <https://reviews.llvm.org/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 hierarchy, the subclasses are never > > exposed. There's no actual danger of confusion. > > > > > the derived class violates the Liskov substitution principle: it doesn't > > > have an `obj.foo(42)` method. > > > > LSP doesn't say the classes are substitutable (indeed you couldn't template > > over the subclasses, for example). It says that *objects* of the classes > > should be. And they are. > > > I don't think I agree with this sentiment - "template<typename T> void f(T > t); int main() { base b; f(b); }" I think when it says (to quote the English > formulation in the Wikipedia article) " if S is a subtype of T, then objects > of type T may be replaced with objects of type S (i.e. an object of type T > may be substituted with any object of a subtype S) without altering any of > the desirable properties of the program" > > Then replacing "base b" with "derived d" should keep working You haven't just replaced the object type though, you've replaced the variable type too. If the variable type stays the same (say base&) and the object type changes, then we see that `derived`s are valid `base`s, which is what LSP is asking. (Note the examples are things like covariant return types, which fit this pattern) I'm not sure this matters - the precise definition of a particular piece of jargon doesn't imply we should draw our lines exactly there. But if we're going to cite this authority I'd like to be clear on what it claims :-) > I think that's a pretty reasonable thing & I think maybe it's OK to live up > to GCC's version of this warning. > ... > But I don't mind conforming to GCC's more stylistic indication. What's the right outcome for OK/don't mind? 1. people can write code with this style in mind? agree, no question 2. should continue to enforce for clang proper, absent consensus to change? I can live with that, wouldn't be my least favorite element of clang style ;-) 3. should enforce for all clang-related subprojects, even where the consensus is they do mind? I think stronger arguments are needed. (Uniformity would be one, but clang is the only part of llvm with this warning, so I think this actually cuts the other way) Basically, I'm waiting for someone to say "I want this warning on for clang, so just disable it for clangd". The focus on which style is better leaves me pretty unclear on whether people actually care about the warning or just want to air some style opinions. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82617/new/ https://reviews.llvm.org/D82617 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
