[PATCH] D146971: [Sema] Populate declarations inside TypeLocs for some invalid types

Mon Mar 27 08:33:05 PDT 2023

ilya-biryukov added a comment.

In D146971#4224465 <https://reviews.llvm.org/D146971#4224465>, @erichkeane wrote:

> One other thing we probably should do is have an assert when creating a function type that none of its params are null.  WDYT?

This would definitely be great, however I don't think this is possible without some large redesign.
Currently, `FunctionType` does not store parameter decls. Instead, they are stored in the `FunctionTypeLoc`.
The latter gets created by allocating <https://github.com/llvm/llvm-project/blob/5f34259609f604bfcd5cbf324a32d265e6a5d347/clang/lib/AST/ASTContext.cpp#L3087> the necessary number of bytes and filling the empty state.
In principle, it would be much easier if `FuncionTypeLoc` and `FunctionType` were created in parallel, ensuring consistency there would be much easier. I have also though about this, but this looks like a large change and I didn't explore further.

================
Comment at: clang/lib/Sema/SemaType.cpp:5949
   assert(!T.isNull() && "T must not be null at the end of this function");
-  if (D.isInvalidType())
+  if (!AreDeclaratorChunksValid)
     return Context.getTrivialTypeSourceInfo(T);
----------------
erichkeane wrote:
> Shouldn't the `D.setInvalidType(true)` in each of the branches work here?  So this variable is unnecessary?  Else this is a good change IMO. 
(Let me know if I'm misreading the suggestion, I was not sure if my understanding is correct).
If we call `setInvalidType` more, we would actually get more crashes as the problematic branch is the one that calls `getTrivialTypeSourceInfo`.
To avoid the extra variable, we could instead ensure that the type always gets replaced with something trivial `T = Context.IntTy`. But I didn't want to go this path because this causes worse error recovery (going from something like `void(<recovered-to:int>)` to `<recovered-to:int>`) and possibly correctness issues (e.g. will we start getting function-decls or lambdas that do not have function types and other assertions may fire).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146971