[PATCH] D154773: [AST] Use correct APSInt width when evaluating string literals

Sun Jul 9 02:38:51 PDT 2023

cor3ntin accepted this revision.
cor3ntin added a subscriber: aaron.ballman.
cor3ntin added a comment.
This revision is now accepted and ready to land.

I think this is fine, and it's a nice simplification.

However it doesn't seem to do much of anything in practice: if you look at `StringLiteral::mapCharByteWidth`, supporting different `CHAR_BIT` would be more involved. And that's just part of the front end. 
I really don't how much effort would be involved for complete support, and... well, as you say, we have no way to test anything.

So my feeling is that we can land that because it simplifies the code, but further work into non 8-bits platforms probably require the input of more folks
and a RFC may be warranted.

Looking around non 8 bits CHAR_BIT seems to have been discussed before https://discourse.llvm.org/t/rfc-on-non-8-bit-bytes-and-the-target-for-it/53455 
but i don't know if a conclusion was reached. Maybe worth poking that thread if you are interested?

WDYT @aaron.ballman ?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154773