[PATCH] D78365: [AST] Preserve the invalid initializer for auto VarDecl.

[Haojian Wu via Phabricator via cfe-commits]

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


================
Comment at: clang/lib/Sema/SemaDecl.cpp:11852
+      // There are unresolved typos in Init. Usually it indicates the
+      // initializer is very ill-formed, just drop it.
       RealDecl->setInvalidDecl();
----------------
sammccall wrote:
> Do you have an example to back this up? (Not to include in the comment, just curious).
> 
> ISTM when there are errors in the input, you can get "lucky" and they turn into recoveryexpr and survive here, or you get "unlucky" and they turn into typoexpr and are dropped here after correction fails. And lucky/unlucky might mostly depend on details about the type of error, not how ill-formed the code is?
> 
> This suggests two possible recovery strategies:
>  - if CorrectDelayedTyposInExpr fails typo-correction, it could rebuild with the typos "degraded" to RecoveryExpr of some sort
>  - or we could just wrap `Init` in `RecoveryExpr`, TypoExprs and all. With the assumption that stuff that doesn't handle TypoExpr should either treate RecoveryExpr as opaque or should bail out at the top-level based on ContainsErrors.
> 
> The former seems more precise and more general, probably more work too.
> 
> None of this needs to be addressed in this patch except maybe a FIXME and/or a tweak to the existing comment.
> Do you have an example to back this up?

we have an example, see the test.

 `auto unresolved_typo = gned.*[] {};` the initExpr looks like below, it seems less useful. I'm not sure it is worth the effort to preserve the AST if the typo correction is not resolved. it requires some work either #1 or #2. 

```
BinaryOperator 0x8992288 '<dependent type>' contains-errors '.*'
|-TypoExpr 0x8991a78 '<dependent type>' contains-errors lvalue
`-LambdaExpr 0x8992150 'class (lambda at /tmp/t4.cpp:11:32)'
  |-CXXRecordDecl 0x8991b30  implicit class definition
  | |-DefinitionData lambda pass_in_registers empty standard_layout trivially_copyable literal can_const_default_init
  | | |-DefaultConstructor defaulted_is_constexpr
  | | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
  | | |-MoveConstructor exists simple trivial needs_implicit
  | | |-CopyAssignment trivial has_const_param needs_implicit implicit_has_const_param
  | | |-MoveAssignment
  | | `-Destructor simple irrelevant trivial
  | |-CXXMethodDecl 0x8991c70  constexpr operator() 'auto (void) const -> void' inline
  | | `-CompoundStmt 0x8991d20
  | |-CXXConversionDecl 0x8991fe8  implicit constexpr operator void (*)() 'auto (*(void) const noexcept)(void) -> void' inline
  | |-CXXMethodDecl 0x8992098  implicit __invoke 'auto (void) -> void' static inline
  | `-CXXDestructorDecl 0x8992180  implicit referenced ~ 'void (void) noexcept' inline default trivial
  `-CompoundStmt 0x8991d20
```


> And lucky/unlucky might mostly depend on details about the type of error, not how ill-formed the code is?

probably, but this also implies our current recovery strategy can't handle this case well.  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78365