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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
