hokein added inline comments.
================
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:
> 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).
>
oh, my bad. I misunderstood this part of code -- I missed the critical function
`isUnusedResultAWarning`, I saw it when reading this part of code, but I
skipped it because the function name made me think "this is a trivial function
that just detects whether the -Wunused flag is on".
I think you analysis is right here. Thanks!
> what makes you think this is firing on the *inner* expression? The diagnostic
> location is that of the outer expression.
hmm, I was confused by the ~annotation of the diagnostic.
```
(Derived(Derived()));
^ ~~~~~~~
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D85716/new/
https://reviews.llvm.org/D85716
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
