[PATCH] D76831: [AST] Preserve the DeclRefExpr when it refers to an invalid decl.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

In D76831#1952425 <https://reviews.llvm.org/D76831#1952425>, @sammccall wrote:

> I like the idea, but it seems to regress quite a lot of diagnostics... e.g. where we fail to deduce auto and then say it's not a pointer.
>
> Also this is regressing things in the -fno-recovery-ast case, because of the changes to CheckDeclInExpr with the existing callsites that allow invalid decls. (I'm not sure what passing AcceptInvalid actually *did* before)


Yeah, the solution makes diagnostics worse...

The motivation of the patch is to preserve the DeclRefExpr nodes even the referenced var-decl is invalid, so that clangd features can still work, e.g. the case we care about

  struct A { A(int); }
  void t() {
    A a; // VarDecl a is invalid.
    a.^ // none of clangd features would work as we don't have any AST node
  }

we have a few options:

1. build DeclRefExpr regardless the validity of VarDecl;
2. mark VarDecl valid even when there are errors during initialization;
3. using RecoveryExpr, e.g. build a RecoveryExpr that wraps a DeclRefExpr if the VarDecl is invalid;

as shown in this patch, 1 is not ideal and hurts about clang diagnostics. 2 looks promising, and safer (probably not regress clang diagnostic a lot).

I think VarDecls in following cases should be valid. Interestingly, clang doesn't seem to have consistent behavior -- some of them are valid already, so 2 has the greatest chance of not introducing large regressions.

  struct A {
    A(int, int);
  };
  
  void test() {
    // 1) default initialization (without any initializers)
    A a1; // invalid
  
    // 2) direct initialization
    A a2(1); // invalid
    A a3{1}; // invalid
    A a4(invalid()); // valid
    A a5{invalid()}; // valid
  
    int var;
    // 3) copy initializaiton
    A a6 = 1; // invalid
    A a7 = A(1); // valid
    A a8 = A(var); // valid
    A a10 = A{1}; // valid
    A a9 = {1}; // invalid
  }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76831