[PATCH] D89046: [AST] Build recovery expression by default for all language.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.


================
Comment at: clang/test/CodeGen/builtins-ppc-error.c:51
 void testCTF(int index) {
-  vec_ctf(vsi, index); //expected-error {{argument to '__builtin_altivec_vcfsx' must be a constant integer}}
-  vec_ctf(vui, index); //expected-error {{argument to '__builtin_altivec_vcfsx' must be a constant integer}}
+  vec_ctf(vsi, index); //expected-error {{argument to '__builtin_altivec_vcfsx' must be a constant integer}} expected-error {{argument to '__builtin_altivec_vcfux' must be a constant integer}}
+  vec_ctf(vui, index); //expected-error {{argument to '__builtin_altivec_vcfsx' must be a constant integer}} expected-error {{argument to '__builtin_altivec_vcfux' must be a constant integer}}
----------------
hmm, this doesn't exactly look right to me - we know the type of `vsi` so we should only be considering the signed case I thought.

However the existing diagnostic for `vui` indicates that it's considering the **signed** case, so I guess this is already broken/bad.


================
Comment at: clang/test/CodeGen/builtins-systemz-zvector-error.c:424
+  vsc = vec_rl_mask(vsc, vuc, idx); // expected-error {{no matching function}} \
+                                    // expected-error {{argument to '__builtin_s390_verimb' must be a constant integer}} \
+                                    // expected-error {{argument to '__builtin_s390_verimh' must be a constant integer}} \
----------------
you may want to make these assertions fuzzier and possibly give a count rather than repeating them, but I guess the owners of these headers would be best to decide


================
Comment at: clang/test/Index/complete-switch.c:9
 
-// RUN: not %clang_cc1 -fsyntax-only -code-completion-at=%s:4:10 %s | FileCheck %s -allow-empty
+// RUN: not %clang_cc1 -fsyntax-only -Xclang -fno-recovery-ast -code-completion-at=%s:4:10 %s | FileCheck %s -allow-empty
 // CHECK-NOT: COMPLETION: foo
----------------
nit: no need for xclang, this is cc1 already

I guess this is a crash test (and it also doesn't crash with recovery ast)?


================
Comment at: clang/test/Sema/__try.c:55
+    // expected-error{{too few arguments to function call, expected 1, have 0}} \
+    // expected-error{{expected ';' after expression}}
   }
----------------
this seems bad, am I missing something?


================
Comment at: clang/test/Sema/enum.c:104
+  switch (PR7911V); // expected-error {{statement requires expression of integer type}} \
+                    // expected-warning {{switch statement has empty body}} expected-note {{put the semicolon on a separate line to silence this warning}}
 }
----------------
or just move the semicolon to suppress, pretty sure that's not what's being tested here.


================
Comment at: clang/test/Sema/typo-correction.c:56
+  f(THIS_IS_AN_ERROR,       // expected-error {{use of undeclared identifier 'THIS_IS_AN_ERROR'}}
+    afunction(afunction_)); // expected-error {{use of undeclared identifier 'afunction_'}}
 }
----------------
what's up with this change?
Do we see the LHS is dependent/contains-errors and then give up on correcting typos in the RHS?
Should we?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89046