[PATCH] D89729: [ms] [llvm-ml] Enable support for MASM-style macro procedures

[Nico Weber via Phabricator via llvm-commits]

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

Basically lg. Please add some tests for the error cases.



================
Comment at: llvm/lib/MC/MCParser/MasmParser.cpp:443
+  // Number of locals defined.
+  uint16_t LocalCounter = 0;
+
----------------
why not just `unsigned`?


================
Comment at: llvm/lib/MC/MCParser/MasmParser.cpp:2259
-    case DK_NOALTMACRO:
-      return parseDirectiveAltmacro(IDVal);
     case DK_EXITM:
----------------
did the altmacro call here disappear intentionally? probably, but delete parseDirectiveAltmacro()'s definition too then?


================
Comment at: llvm/lib/MC/MCParser/MasmParser.cpp:2400
+    Lex();
+    return parseDirectiveMacro(IDVal, IDLoc);
   }
----------------
why is this moving down here?


================
Comment at: llvm/lib/MC/MCParser/MasmParser.cpp:2622
 
   // A macro without parameters is handled differently on Darwin:
   // gas accepts no arguments and does no substitutions
----------------
comment is now outdated


================
Comment at: llvm/lib/MC/MCParser/MasmParser.cpp:2688
+      Pos += Argument.size();
+      if (Body[Pos] == '&') {
+        ++Pos;
----------------
is this guaranteed to be in bounds?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89729