hbatagelo wrote:

> I knew I saw this before, because I recently reviewed #200161 and that's 
> fixing the same bug as here. But that patch is much better than this one, 
> because it doesn't rely on checking the invalid bit.

I don't think https://github.com/llvm/llvm-project/pull/200161 covers the 
reproducer here. In that patch, the template is invalidated during 
substitution. Here, the template declaration gets marked invalid in phase 1. 
Because of that, the instantitaion is skipped in `Sema::InstantiateClassImpl`:
https://github.com/llvm/llvm-project/blob/c48e258f617c231c6be1bbf15d239dfdcdd8c30c/clang/lib/Sema/SemaTemplateInstantiate.cpp#L3614-L3619

My original fix mirrored how failed lookups are handled for class template 
partial specializations:
https://github.com/llvm/llvm-project/blob/c48e258f617c231c6be1bbf15d239dfdcdd8c30c/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp#L2445-L2455

This handles both invalid declarations and failed instantiations. Given that we 
now have two counterexamples to the invariant tested by `assert(!Found.empty() 
&& "Instantiation found nothing?");` (#198890 and #195988), this bailout seems 
reasonable IMO.

With that said, keeping a valid AST for our case is also simple. We could 
intercept the invalid type in `Sema::ActOnVariableDeclarator` and recover it to 
`int` as you suggested.

https://github.com/llvm/llvm-project/pull/202006
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to