sammccall added inline comments.
================
Comment at: clang/lib/Sema/SemaStmt.cpp:353
return;
+ if (isa<RecoveryExpr>(E))
+ return;
----------------
This seems like a very specific patch to a special case of a potentially more
general problem...
================
Comment at: clang/test/SemaCXX/recovery-expr-type.cpp:88
+void func() {
+ // verify that no -Wunused-variable diagnostic on the inner Derived
expression.
+ (Derived(Derived())); // expected-error {{call to implicitly-deleted default
constructor}}
----------------
(nit: I think this is -Wunused but not -Wunused-variable)
================
Comment at: clang/test/SemaCXX/recovery-expr-type.cpp:88
+void func() {
+ // verify that no -Wunused-variable diagnostic on the inner Derived
expression.
+ (Derived(Derived())); // expected-error {{call to implicitly-deleted default
constructor}}
----------------
sammccall wrote:
> (nit: I think this is -Wunused but not -Wunused-variable)
what makes you think this is firing on the *inner* expression? The diagnostic
location is that of the outer expression. I think the existing logic is pretty
close to correct (and the diagnostic is actually correct here, albeit through
luck), and if we want to change it, the right fix belongs somewhere else.
The warning doesn't fire for just `Derived();` - isUnusedResultAWarning returns
false for RecoveryExpr. Nor does it fire for `(Derived());` -
isUnusedResultAWarning looks inside the paren expr and gets the same result for
the contents.
For the case in this test, we hit the CXXFunctionalCastExpr/CStyleExprCast case
of isUnusedResultAWarning. It says:
> If this is a cast to a constructor conversion, check the operand.
> Otherwise, the result of the cast is unused.
But this is a bit misleading: given `Foo(2+2)` the "operand" isn't 2+2, but
rather a CXXConstructExpr. And isUnusedResultAWarning returns true for this
case (usually).
Back to the case at hand: we diverge because the cast type is Dependent instead
of ConstructorConversion.
This seems like it could be a bug - in this configuration the operand is not
type-dependent so why can't it resolve the overload?
But in any case, I don't think isUnusedResultAWarning should be assuming a
dependent cast is safe to warn on - rather either return false or delegate to
the inner expr (which in this case is a RecoveryExpr).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D85716/new/
https://reviews.llvm.org/D85716
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
