[PATCH] D91625: [clang] Do not crash on pointer wchar_t pointer assignment.

[Adam Czachorowski via Phabricator via cfe-commits]

adamcz added inline comments.


================
Comment at: clang/lib/Sema/SemaExpr.cpp:8726
+    else if (!lhptee->isWideCharType() &&
+             lhptee->hasSignedIntegerRepresentation())
       ltrans = S.Context.getCorrespondingUnsignedType(ltrans);
----------------
hokein wrote:
> I'm wondering whether the fix is in `ASTContext::getCorrespondingUnsignedType()` 
> 
> if my reading of the standard is right:
> 
> - there exists a corresponding standard (or extended) unsigned integer type for each of standard (or extended) signed integer types, http://eel.is/c++draft/basic.fundamental#2
> - wchar_t is an integer type, http://eel.is/c++draft/basic.fundamental#11, and has an implementation-defined signed or unsigned integer type as its underlying type, http://eel.is/c++draft/basic.fundamental#8
> 
> I think the rule also applies on `wchar_t`, the fix seems to handle `WChar_S` case (return `ASTContext::getUnsignedWCharType()`) in `ASTContext::getCorrespondingUnsignedType()`. What do you think?
http://eel.is/c++draft/basic.fundamental#11 - wchar_t is not a signed or unsigned integer type. It is an integer type, but not signed integer type. Therefore it's underlying type might have an matching signed/unsigned version, but wchar_t does not.

I initially wanted to fix ASTContext::getCorrespondingUnsignedType(), but switched to this version with the idea that calling getCorrespondingUnsignedType() on wchar_t is most likely a bug in the first place and returning something there might just hide the issue.

However, based on your comment and the fact that, for char, we already return underlying type too, I think maybe this is a better approach after all. Updated. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91625