[PATCH] D78116: [AST] dont invaliate VarDecl when the initializer contains errors.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.


================
Comment at: clang/lib/Sema/SemaDecl.cpp:11998
     ExprResult Result = InitSeq.Perform(*this, Entity, Kind, Args, &DclT);
     if (Result.isInvalid()) {
+      auto RecoveryExpr =
----------------
sammccall wrote:
> if the variable is an undeduced auto (and the type of the recoveryexpr is unknown), we still have to mark it invalid. Does that happen somewhere else?
Yeah, the `auto` case is handled earlier (Line 11845 of this file), so we are safe at this point.


================
Comment at: clang/test/AST/ast-dump-invalid-initialized.cpp:19
+  // CHECK: `-VarDecl {{.*}} a6 'A'
+  A a6(invalid());
+  // CHECK: `-VarDecl {{.*}} a7 'A'
----------------
sammccall wrote:
> what actually is the AST here? invalid() is a recoveryexpr, does it  get wrapped in a second recoveryexpr by the code in this patch? Or is this testing existing behavior?
the AST looks like:

```
 -VarDecl 0x9b80970 <col:3, col:17> col:5 a6 'A' callinit
        `-ParenListExpr 0x9b80ac0 <col:7, col:17> 'NULL TYPE' contains-errors
          `-RecoveryExpr 0x9b80a78 <col:8, col:16> '<dependent type>' contains-errors lvalue
            `-UnresolvedLookupExpr 0x9b809d8 <col:8> '<overloaded function type>' lvalue (ADL) = 'invalid' empty
```

my attemption was to verify the valid-bit of VarDecl, but they can be covered by ast-dump-recovery.cpp.




================
Comment at: clang/test/CXX/stmt.stmt/stmt.iter/stmt.ranged/p1.cpp:1
-// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
-// RUN: %clang_cc1 -std=c++14 -fsyntax-only -verify %s
-// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -frecovery-ast -verify %s
+// RUN: %clang_cc1 -std=c++14 -fsyntax-only -frecovery-ast -verify %s
----------------
sammccall wrote:
> not sure we should be switching this until we flip the default.
> Are we regressing the diagnostics under default flags?
> 
> (and in 2 other files)
yes, we have a few regressions without -frecovery-ast, the worst regression is that we stop emitting the "selected 'begin' function with iterator type 'Data *'" diagnostic note in an ill-formed range-range stmt.

As discussed in another patch, lets not change the default flag in existing tests, reverted the new flag, adjusted exiting tests, and created a new test for -frecovery-ast.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78116