[PATCH] D32792: [LLVM][inline-asm] Altmacro string escape character '!'

Renato Golin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri May 5 03:22:32 PDT 2017


rengolin added inline comments.


================
Comment at: lib/MC/MCParser/AsmParser.cpp:1207
   while ((*CharPtr != '>') && (*CharPtr != '\'') && (*CharPtr != '\n') &&
 	     (*CharPtr != '\r') && (*CharPtr != '\0')){
+	  if(*CharPtr == '!')
----------------
This could be:

    while ( ((...) && (*CharPtr != '\0')) || (*CharPtr == '!')) {
      CharPtr++;


================
Comment at: lib/MC/MCParser/AsmParser.cpp:1219
+/// \brief creating a string without the escape characters '!'. 
+void AsmParser::altMacroString(StringRef AltMacroStr,std::string &Res) {
+  for (int Pos = 0; Pos < AltMacroStr.size(); Pos++) {
----------------
This should really use `isAltmacroString` to make sure it really is a string, or it could have the same problems you detect on the other cases. But doing that, the same logic would need to be implemented twice.

A better approach would be if `isAltmacroString` was actually just `altMacroString` and, if the result is not empty, it "is an alt-macro string".




================
Comment at: test/MC/AsmParser/altmacro_string_escape.s:3
+# CHECK: workForFun:
+# CHECK: workForFun2:
+
----------------
put the check lines near where they make sense, for instance, near the macro definition, or its use.


https://reviews.llvm.org/D32792





More information about the llvm-commits mailing list