[PATCH] D93031: Enable fexec-charset option

Abhina Sree via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Apr 22 06:32:03 PDT 2021


abhina.sreeskantharajan added a comment.

In D93031#2706660 <https://reviews.llvm.org/D93031#2706660>, @cor3ntin wrote:

>>> We should use the original source form of the string literal when pretty-printing a `StringLiteral` or `CharacterLiteral`; there are a bunch of UTF-8 assumptions baked into `StmtPrinter` that will need revisiting. And we'll need to modify the handful of places that put the contents of `StringLiteral`s into diagnostics (`#warning`, `#error`, `static_assert`) and make them use a different `ConversionState`, since our assumption is that diagnostic output should be in UTF-8.
>>
>> Yes, these are some of the complications we will need to visit in later patches. We may need to somehow save the original string or reverse the translation.
>
> The operation is destructive and therefore cannot be reverted.
> So I do believe the correct behavior here would indeed be to keep the original spelling around - with *some* of phase 5 applied (replacement of UCNs and replacement of numeric escape sequences).
> An alternative would be to do the conversion lazily when the strings are evaluated, rather than during lexing, although that might be more involved

Thanks for the input! I agree doing the conversion lazily will help avoid hitting these issues since we push translation to a later stage but as you mentioned it will be more involved. I think keeping the original spelling might be the best solution. We can make a extra member in StringLiteralParser to save the string prior to translation. But we would need to go through each use of StringLiteralParser and save the original encoding (possibly print it in the .ll file along with the translated string or as an attribute?). Let me know what you think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93031



More information about the cfe-commits mailing list