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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
