https://github.com/steakhal approved this pull request.

Looks good to me.
One question I had while reading this for the second time was:

If we would canonicalize before the assert in `makeNullWithType`, then we 
wouldn't need to again canonicalize in `isLocType`.

My concern is that `isLocType` would now start accepting Atomic types, while 
there could be potential callsites not doing this canonicalization.
While I think the lack of static guarantees is not ideal, we are definitely 
better off with having this patch than not. Ideally, we should enforce 
canonicalization preventing this bug class.

@NagyDonat WDYT?

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

Reply via email to