rjmccall added inline comments.
================
Comment at: lib/Sema/SemaOverload.cpp:13075
+ if ((isa<CXXConstructorDecl>(CurContext) ||
+ isa<CXXDestructorDecl>(CurContext)) &&
+ isa<CXXThisExpr>(MemExpr->getBase()->IgnoreParenCasts()) &&
----------------
Intentionally suppressed in lambdas? I think that might be reasonable, but
also worth a comment.
================
Comment at: lib/Sema/SemaOverload.cpp:13080
+ // We already have a warning for pure virtual functions above, so don't
+ // consider them here
+ !TheCall->getMethodDecl()->isPure()) {
----------------
This should be folded into the same checks that lead into that warning, since
most of the conditions are identical and it's just a few final decisions (and
the purity of the function) that changes whether to emit a diagnostic and which
one.
================
Comment at: lib/Sema/SemaOverload.cpp:13087
+ const CXXMethodDecl *ContextMD = dyn_cast<CXXMethodDecl>(CurContext);
+ assert(ContextMD && "CurContext was expected be a method declaration");
+ const CXXRecordDecl *ContextRD = ContextMD->getParent();
----------------
Use `cast<>` instead of `dyn_cast`+`assert`.
================
Comment at: lib/Sema/SemaOverload.cpp:13091
+ // final
+ if (ContextRD->isPolymorphic() && !ContextRD->hasAttr<FinalAttr>()) {
+ Diag(MemExpr->getBeginLoc(),
----------------
`ContextRD` must be polymorphic, it provides a `virtual` method.
================
Comment at: lib/Sema/SemaOverload.cpp:13099
+ << FixItHint::CreateInsertion(
+ MemExpr->getBeginLoc(),
+ ContextMD->getParent()->getDeclName().getAsString() + "::");
----------------
As a matter of general principle, these diagnostics should be at
`TheCall->getExprLoc()` instead of `MemExpr->getBeginLoc()`. As a matter of
correctness, the insertion must start at `MemExpr->getMemberLoc()` instead of
`MemExpr->getBeginLoc()` because the base in the member expression might be
explicit.
Please add tests for the fix-its.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56366/new/
https://reviews.llvm.org/D56366
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
