[PATCH] D102814: [AIX] Print printable byte list as quoted string

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: llvm/lib/MC/MCAsmStreamer.cpp:1007-1008
+static inline bool isPrintableString(StringRef Data) {
+  const auto BeginPtr = Data.begin(), EndPtr = Data.end();
+  for (const unsigned char C : make_range(BeginPtr, EndPtr)) {
+    if (!isPrint(C))
----------------
If going through the whole string, going through `make_range` is not needed.


================
Comment at: llvm/lib/MC/MCAsmStreamer.cpp:1009
+  for (const unsigned char C : make_range(BeginPtr, EndPtr)) {
+    if (!isPrint(C))
+      return false;
----------------
`isPrint` returns true for `"`, which does require escaping.


================
Comment at: llvm/lib/MC/MCAsmStreamer.cpp:1048-1051
   case MCAsmInfo::ACLS_SingleQuotePrefix:
-    printCharacterList(printOneCharacterFor([&OS](char C) {
-      const char AsmCharLitBuf[2] = {'\'', C};
-      OS << StringRef(AsmCharLitBuf, sizeof(AsmCharLitBuf));
-    }));
+    // If the whole string can be printed, print it directly.
+    if (isPrintableString(Data))
+      OS << '"' << Data << '"';
----------------
This breaks the abstraction. That a "byte-list" directive accepting a list of elements where there are single-quote-prefixed character literals is available does not mean that the same directive accepts a string argument.

Note: `PrintQuotedString` does print more general strings on AIX (that do not contain a newline); however, I do not believe it to be desirable to emit control characters and the such "raw".


================
Comment at: llvm/lib/MC/MCAsmStreamer.cpp:1123-1126
     if (MAI->getAscizDirective() && Data.back() == 0) {
       OS << MAI->getAscizDirective();
       Data = Data.substr(0, Data.size() - 1);
     } else if (LLVM_LIKELY(MAI->getAsciiDirective())) {
----------------
Here may be a nice point to add a condition that `hasPairedDoubleQuoteStringConstants` means we check `isPrintableString` before deciding to use AIX `.string` for `.asciz` and AIX `.byte` for `.ascii`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102814