ilya-biryukov added a comment.

In D146426#4207423 <https://reviews.llvm.org/D146426#4207423>, @shafik wrote:

> As I noted in the bug report not doing `D.setInvalidType();` does fix this 
> bug and seems harmless since the error diagnostic should prevent us from 
> getting to codegen but it is not clear to me if this has other negative 
> impacts. Reading your replies it is not obvious you looked at this path or 
> not.

There are too many other code paths that set `setInvalidType()`, I'll try to 
add more tests that capture those too.
It would be easy to avoid calling `setInvalidType()` for `__fp16` in 
particular, but checking for nulls actually saves us from other potential 
crashes .

> I would not mind a quick fix but I think we should have a plan for a more 
> correct fix before we do that.

Based on conversations in this thread, I think the right way is to make sure 
parameter lists never have nulls.
One way to fix this is to make sure we always fill parameters with non-null 
values, even if `GetTypeSourceInfoForDeclarator` does not get called.
An alternative way would be to call `GetTypeSourceInfoForDeclarator` even for 
invalid types (IIUC, it can perfectly handle function types, but other types 
also fail there).

I'll try to explore some these fixes and try to come up with a follow-up change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146426

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to