dblaikie added a comment. In D82728#2133951 <https://reviews.llvm.org/D82728#2133951>, @Quuxplusone wrote:
> In D82728#2133720 <https://reviews.llvm.org/D82728#2133720>, @dblaikie wrote: > > > (the presence of at least one "override" being a signal the user intended > > to use override and missed some [...] > > > I'm in favor of `-Wsuggest-override`, and would definitely use it if it > existed. I've no doubt a non-trivial number of folks would - we'd probably enable it in LLVM itself. > The problem I see with `-Wmissing-override`, as it's been implemented, is > that it uses the wrong granularity for "intent": it looks only across the > methods of a single class, rather than across all the classes of a single > header, or across a single translation unit, or across my entire codebase. In > real life, I //always// want to look across my entire codebase (excluding > system headers). If //any// class in my project uses `override`, I want Clang > to take that as a clear declaration of intent to use `override` throughout; I > don't want Clang treating class A differently from class B. But of course > Clang can't look at my whole codebase simultaneously. Right - Clang's doing its best (well, debatable - that's what we're debating) with what it's got. > So the next best thing is to give the user a simple way to "preload the > intent flag": to say "As soon as you start processing //any// class, please > act as if intent has been declared for that class." Adding > `-Wsuggest-override` to my build line seems like a perfect way to implement > that "preload" facility. The issue is that such a warning then needs to be off by default, because we can't assume the user's intent. And Clang's historically been fairly averse to off-by-default warnings due to low user-penetration (not zero, like I said, I imagine LLVM itself would use such a warning, were it implemented) & so concerns about the cost/benefit tradeoff of the added complexity (source code and runtime) of the feature. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82728/new/ https://reviews.llvm.org/D82728 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
