[PATCH] D79160: [AST] Preserve the type in RecoveryExprs for broken function calls.

[Haojian Wu via Phabricator via cfe-commits]

hokein marked an inline comment as done.
hokein added inline comments.


================
Comment at: clang/test/SemaCXX/enable_if.cpp:417
 
-constexpr int B = 10 + // the carat for the error should be pointing to the problematic call (on the next line), not here.
-    callTemplated<0>(); // expected-error{{initialized by a constant expression}} expected-error at -3{{no matching function for call to 'templated'}} expected-note{{in instantiation of function template}} expected-note at -10{{candidate disabled}}
+constexpr int B = 10 + // expected-error {{initialized by a constant expression}}
+    callTemplated<0>(); // expected-error at -3{{no matching function for call to 'templated'}} expected-note{{in instantiation of function template}} expected-note at -10{{candidate disabled}} expected-note {{in call to 'callTemplated()'}} expected-note at -3 {{subexpression not valid in a constant expression}}
----------------
hokein wrote:
> sammccall wrote:
> > hokein wrote:
> > > this diagnostic is changed slightly (without `-frecovery-ast-type`). I think this is acceptable.
> > > 
> > > before this patch:
> > > 
> > > ```
> > > /tmp/t5.cpp:7:10: error: no matching function for call to 'templated'
> > >   return templated<N>();  // expected-error {{no matching function for call to 'templated'}} 
> > >          ^~~~~~~~~~~~
> > > /tmp/t5.cpp:13:5: note: in instantiation of function template specialization 'enable_if_diags::callTemplated<0>' requested here
> > >     callTemplated<0>(); // expected-note {{in call to 'callTemplated()'}} expected-note at -6 {{subexpression not valid in a constant expression}}
> > >     ^
> > > /tmp/t5.cpp:2:32: note: candidate disabled: <no message provided>
> > > template <int N> constexpr int templated() __attribute__((enable_if(N, ""))) {
> > >                                ^                                    ~
> > > /tmp/t5.cpp:13:5: error: constexpr variable 'B' must be initialized by a constant expression
> > >     callTemplated<0>(); // expected-note {{in call to 'callTemplated()'}} expected-note at -6 {{subexpression not valid in a constant expression}}
> > >     ^~~~~~~~~~~~~~~~~~
> > > 2 errors generated.
> > > ```
> > > 
> > > vs after this patch:
> > > 
> > > ```
> > > /tmp/t5.cpp:7:10: error: no matching function for call to 'templated'
> > >   return templated<N>();  // expected-error {{no matching function for call to 'templated'}} 
> > >          ^~~~~~~~~~~~
> > > /tmp/t5.cpp:13:5: note: in instantiation of function template specialization 'enable_if_diags::callTemplated<0>' requested here
> > >     callTemplated<0>(); // expected-note {{in call to 'callTemplated()'}} expected-note at -6 {{subexpression not valid in a constant expression}}
> > >     ^
> > > /tmp/t5.cpp:2:32: note: candidate disabled: <no message provided>
> > > template <int N> constexpr int templated() __attribute__((enable_if(N, ""))) {
> > >                                ^                                    ~
> > > /tmp/t5.cpp:12:15: error: constexpr variable 'B' must be initialized by a constant expression
> > > constexpr int B = 10 +  // expected-error {{constexpr variable 'B' must be initialized by a constant expression}}
> > >               ^   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > /tmp/t5.cpp:7:10: note: subexpression not valid in a constant expression
> > >   return templated<N>();  // expected-error {{no matching function for call to 'templated'}} 
> > >          ^
> > > /tmp/t5.cpp:13:5: note: in call to 'callTemplated()'
> > >     callTemplated<0>(); // expected-note {{in call to 'callTemplated()'}} expected-note at -6 {{subexpression not valid in a constant expression}}
> > >     ^
> > > 2 errors generated.
> > > ```
> > > 
> > > 
> > Yeah, this is noisier but seems OK.
> > Can you take a look at the blame for the split-line test and see if there was any special code to support it that can be cleaned up?
> `git blame` showed https://github.com/llvm/llvm-project/commit/c7d3e609fbf05d1a1236f99efd1e2fd344554f4b, but I didn't see any special code that we need clean up.
actually we don't need this change if we land this patch before the enable-recovery-expr patch, defer this change to D78350.


================
Comment at: clang/test/SemaTemplate/instantiate-function-params.cpp:1
 // RUN: %clang_cc1 -triple i686-unknown-unknown -fsyntax-only -verify %s
 
----------------
sammccall wrote:
> hokein wrote:
> > this test is very tricky, and hard for human to understand. It was added to test clang not crashing.
> > 
> > Different compilers (gcc, msvc) emit different diagnostics, with this patch, clang emits 3 more errs `no type named 'type'...`, it seems reasonable, and gcc also emits them.
> Ugh, I remember this test. It's partially reduced from boost IIRC.
the same to this file, defer it to D78350.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79160/new/

https://reviews.llvm.org/D79160