hokein added a comment.
> As you said, we can't land this before the branch cut, and we shouldn't land
> this until we've run internal experiments to show it's not horribly crashy.
The internal experiment result is good, I think we're close to land it. After a
new rebase, we need to adjust more diagnostics, but all of them look like good
secondary-diagnostic improvements.
================
Comment at: clang/test/CXX/temp/temp.decls/temp.variadic/fixed-expansion.cpp:129
+ S<int, int, double> &s1 = f({}, 0, 0.0); // expected-error {{no matching
function}} \
+ expected-error {{non-const
lvalue reference to type 'S<int, int, double>' cannot bind to a value of
unrelated type 'int'}}
}
----------------
sammccall wrote:
> hokein wrote:
> > the secondary diagnostic is technically correct, but I don't quite like it,
> > it is confusing, ok to leave it as-is? or fix that before landing this
> > patch?
> I don't think it's technically correct (the `int` fallback is an
> implementation artifact, albeit one that leaks quite often).
> This example looks pretty obscure though, it'd be nice to fix it at some
> point but I don't think it's severe enough to block on the fix.
>
> Fix ideas:
> - the instantiation of `f` should be invalid, right? Maybe we avoid
> considering return types for invalid decls when computing recovery type (or
> always give up in this case).
> - we could eventually try to replace the use of `int` for these cases
> - we could special-case `int`, and don't allow RecoveryExprs to have type
> `int`, reflecting the fact that it's used for these cases
Fixed in D85714.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D82657/new/
https://reviews.llvm.org/D82657
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
