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

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast added a comment.
This revision is now accepted and ready to land.

LGTM with minor comments; thanks!



================
Comment at: llvm/include/llvm/MC/MCAsmInfo.h:268-270
+  /// This directive allows emission of an ascii string without the standard C
+  /// escape characters embedded into it.  If a target doesn't support this, it
+  /// can be set to null. Defaults to null.
----------------
See suggested edit. Emphasizing the difference from `.ascii` by starting with its associated comment is nice. Need to indicate that this is the zero-terminated case though.


================
Comment at: llvm/lib/MC/MCAsmStreamer.cpp:1012-1014
+  if (isPrint(Data.back()) || Data.back() == 0)
+    return true;
+  return false;
----------------
Minor comment: Can just return the expression instead of using it for `if`/`else`.


================
Comment at: llvm/lib/MC/MCAsmStreamer.cpp:1128
+               isPrintableString(Data)) {
+      // For target with DoubleQuteString constants, .string and .byte are used
+      // as replacement of .asciz and .ascii.
----------------
Minor nit: typo.


================
Comment at: llvm/lib/MC/MCAsmStreamer.cpp:1139-1140
+        Data = Data.substr(0, Data.size() - 1);
+      } else
+        OS << MAI->getByteListDirective();
     } else if (MAI->getByteListDirective()) {
----------------
The style guide was clarified some time ago to indicate that mixing use/non-use of braces in an `if`/`else` chain is discouraged.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102814