[clang] [Clang] Fixed a crash when instantiating an invalid out-of-line static data member definition in a local class (PR #196772)

[Younan Zhang via cfe-commits]

zyn0217 wrote:

> > > I think this is a timing issue: when parsing function f, A only reports an error but doesn't mark it as invalid.
> > 
> > 
> > This is not a timing issue, but where the problem happens. You should invalidate the decl as soon as it gets diagnosed, rather than deferring them to the instantiation.
> 
> I don't think avoiding instantiation is feasible, as clang currently doesn't process `VarDecl`.
> 
> https://godbolt.org/z/e5WWMoqEh Not instantiating the template doesn't report a `VarDecl` error, and node `b` is valid. If we directly mark the entire `CXXRecordDecl` as invalid, I believe the reverted PR has already proven it's not feasible.

I don't understand what 'doesn't process VarDecl' means? It seems to me that we do build a VarDecl for `int A::B`:

```
      |-DeclStmt 0x491bc1e0 <line:5:3, col:11>
      | `-VarDecl 0x491bc130 parent 0x491bbe80 prev 0x491bc040 <col:3, col:10> col:10 B 'int'
      |   `-NestedNameSpecifier TypeSpec 'A'
```

Also, why do you think a reverted PR means being infeasible? AFAIK the regression it brought is just another case where we shouldn't call SetInvalid on an incomplete type, and we can simply guard that call from that case.

Regardless, unless @ojhunt's concerns can be addressed, I do think the change in this PR looks more arbitrary than the reverted one.


https://github.com/llvm/llvm-project/pull/196772