dblaikie added a comment.
I'm generally on board with this, but would like @rsmith 's sign off to be sure.
I think it might be nice to make the -Wno-inconsistent-missing-override
-Wsuggest-override situation a bit better (by having it still do the same thing
as -Wsuggest-override) but I don't feel /too/ strongly about it.
================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:3064
if (MD->size_overridden_methods() > 0) {
- unsigned DiagID = isa<CXXDestructorDecl>(MD)
- ?
diag::warn_destructor_marked_not_override_overriding
- : diag::warn_function_marked_not_override_overriding;
- Diag(MD->getLocation(), DiagID) << MD->getDeclName();
- const CXXMethodDecl *OMD = *MD->begin_overridden_methods();
- Diag(OMD->getLocation(), diag::note_overridden_virtual_function);
+ auto EmitDiag = [this, MD](unsigned DiagDtor, unsigned DiagFn) {
+ unsigned DiagID = isa<CXXDestructorDecl>(MD) ? DiagDtor : DiagFn;
----------------
Generally I'd recommend default ref capture `[&]` on any lambda that doesn't
escape its scope - normal scopes don't need to document which variables you use
inside them, and I think the same applies to lambdas (bit more debatable when
the lambda is named and called later, even within the same scope - so if you
feel strongly about keeping it the way it is, that's OK)
================
Comment at: clang/test/SemaCXX/warn-suggest-override:3-4
+
+class A {
+ public:
+ ~A() {}
----------------
I'd probably simplify these tests by using struct, so everything's implicitly
public, rather than class and having to make things public.
Also probably member function declarations rather than definitions would be
simpler?
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
