sammccall 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.



================
Comment at: 
clang/test/CXX/temp/temp.constr/temp.constr.order/function-templates.cpp:71
+// expected-error@-1 {{call to 'f' is ambiguous}} \
+   expected-error@-1 {{invalid application of 'sizeof' to an incomplete type 
'void'}}
 
----------------
this is really nice :-)


================
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'}}
 }
----------------
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


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

Reply via email to