[ms] [llvm-ml] Implement the expression expansion operator

[epastor]

Summary

In text-item contexts, %expr expands to a string containing the results of evaluating expr.

[epastor]

Fix missing brackets around a case block

[epastor]

Removed debug line

[thakis]

The diff looks like maybe there was a rebasing accident (?)

llvm/lib/MC/MCParser/MasmParser.cpp

3302

This branch seems unrelated to this patch (?)

5541

also looks unrelated (?)

6018

llvm code style says no else after return; lhs code was better

[epastor]

Rebase on HEAD

[thakis]

llvm/lib/MC/MCParser/MasmParser.cpp

3216–3217

Was the patch that added this already landed? If not, maybe remove it there instead of here :)

6018

(this is still not addressed)

[epastor]

Rebase on parent (pushed a change further up the stack)

[epastor]

Remove "return/else" branches to match LLVM style

[epastor]

llvm/lib/MC/MCParser/MasmParser.cpp

3302

This is part of unifying handling for all textitem forms, including textmacros.

5541

This fixes an unrelated bug discovered in the course of testing this commit - before, there was no error for a failure to parse the textitem.

6018

Addressed now; thanks!

llvm/lib/MC/MCParser/MasmParser.cpp

private:
   bool parseDirectiveCFIRestore(SMLoc DirectiveLoc);
   bool parseDirectiveCFIEscape();
   bool parseDirectiveCFIReturnColumn(SMLoc DirectiveLoc);
   bool parseDirectiveCFISignalFrame();
   bool parseDirectiveCFIUndefined(SMLoc DirectiveLoc);
 
   // macro directives
   bool parseDirectivePurgeMacro(SMLoc DirectiveLoc);
-  bool parseDirectiveExitMacro(StringRef Directive, std::string &Value);
+  bool parseDirectiveExitMacro(SMLoc DirectiveLoc, StringRef Directive,
+                               std::string &Value);
   bool parseDirectiveEndMacro(StringRef Directive);
   bool parseDirectiveMacro(StringRef Name, SMLoc NameLoc);
 
   bool parseDirectiveStruct(StringRef Directive, DirectiveKind DirKind,
                             StringRef Name, SMLoc NameLoc);
   bool parseDirectiveNestedStruct(StringRef Directive, DirectiveKind DirKind);
   bool parseDirectiveEnds(StringRef Name, SMLoc NameLoc);
   bool parseDirectiveNestedEnds();
if (DirKind != DK_NO_DIRECTIVE) {
     case DK_CFI_UNDEFINED:
       return parseDirectiveCFIUndefined(IDLoc);
     case DK_CFI_REGISTER:
       return parseDirectiveCFIRegister(IDLoc);
     case DK_CFI_WINDOW_SAVE:
       return parseDirectiveCFIWindowSave();
     case DK_EXITM:
       Info.ExitValue = "";
-      return parseDirectiveExitMacro(IDVal, *Info.ExitValue);
+      return parseDirectiveExitMacro(IDLoc, IDVal, *Info.ExitValue);
     case DK_ENDM:
       Info.ExitValue = "";
       return parseDirectiveEndMacro(IDVal);
     case DK_PURGE:
       return parseDirectivePurgeMacro(IDLoc);
     case DK_END:
       return parseDirectiveEnd(IDLoc);
     case DK_ERR:
if (DirKind == DK_EQU || DirKind == DK_TEXTEQU) {
     std::string Value;
     if (!parseTextItem(Value)) {
       Var.IsText = true;
       Var.TextValue = Value;
 
       // Accept a text-list, not just one text-item.
       auto parseItem = [&]() -> bool {
         if (parseTextItem(Value))
-          return true;
+          return TokError("expected text item");
         Var.TextValue += Value;
         return false;
       };
       if (parseOptionalToken(AsmToken::Comma) && parseMany(parseItem))
         return addErrorSuffix(" in '" + Twine(IDVal) + "' directive");
 
       return false;
     }
   }
   if (DirKind == DK_TEXTEQU)
     return TokError("expected <text> in '" + Twine(IDVal) + "' directive");
 
   // Parse as expression assignment.
   const MCExpr *Expr;
   SMLoc EndLoc, StartLoc = Lexer.getLoc();
if (isAngleBracketString(StartLoc, EndLoc)) {
     Data = angleBracketString(StringRef(StartChar, EndChar - StartChar));
     return false;
   }
   return true;
 }
 
 /// textItem ::= textLiteral | textMacroID | % constExpr
 bool MasmParser::parseTextItem(std::string &Data) {
-  // TODO(epastor): Support textMacroID and % expansion of expressions.
+  switch (getTok().getKind()) {
+  default:
+    return true;
+  case AsmToken::Percent: {
+    int64_t Res;
+    if (parseToken(AsmToken::Percent) || parseAbsoluteExpression(Res))
+      return true;
+    Data = std::to_string(Res);
+    return false;
+  }
+  case AsmToken::Less:
+  case AsmToken::LessEqual:
+  case AsmToken::LessLess:
+  case AsmToken::LessGreater:
     return parseAngleBracketString(Data);
+  case AsmToken::Identifier: {
+    StringRef ID;
+    if (parseIdentifier(ID))
+      return true;
+
+    auto it = Variables.find(ID);
+    if (it == Variables.end())
+      return true;
+
+    const Variable &Var = it->second;
+    if (!Var.IsText)
+      return true;
+    Data = Var.TextValue;
+    return false;
+  }
+  }
+  llvm_unreachable("unhandled token kind");
 }
 
 /// parseDirectiveAscii:
 ///   ::= ( .ascii | .asciz | .string ) [ "string" ( , "string" )* ]
 bool MasmParser::parseDirectiveAscii(StringRef IDVal, bool ZeroTerminated) {
   auto parseOp = [&]() -> bool {
     std::string Data;
     if (checkForValidSection() || parseEscapedString(Data))
bool MasmParser::parseDirectiveMacro(StringRef Name, SMLoc NameLoc) {
   DEBUG_WITH_TYPE("asm-macros", dbgs() << "Defining new macro:\n";
                   Macro.dump());
   getContext().defineMacro(Name, std::move(Macro));
   return false;
 }
 
 /// parseDirectiveExitMacro
 /// ::= "exitm" [textitem]
-bool MasmParser::parseDirectiveExitMacro(StringRef Directive,
+bool MasmParser::parseDirectiveExitMacro(SMLoc DirectiveLoc,
+                                         StringRef Directive,
                                          std::string &Value) {
-  if (getTok().isNot(AsmToken::EndOfStatement)) {
-    parseTextItem(Value);
-  }
+  SMLoc EndLoc = getTok().getLoc();
+  if (getTok().isNot(AsmToken::EndOfStatement) && parseTextItem(Value))
+    return Error(EndLoc,
+                 "unable to parse text item in '" + Directive + "' directive");
   eatToEndOfStatement();
 
   if (!isInsideMacroInstantiation())
     return TokError("unexpected '" + Directive + "' in file, "
                                                  "no current macro definition");
 
   // Exit all conditionals that are active in the current macro.
   while (TheCondStack.size() != ActiveMacros.back()->CondStackDepth) {
if (TheCondState.Ignore) {
     TheCondState.CondMet = ExprValue;
     TheCondState.Ignore = !TheCondState.CondMet;
   }
 
   return false;
 }
 
 /// parseDirectiveIfb
-/// ::= .ifb string
+/// ::= .ifb textitem
 bool MasmParser::parseDirectiveIfb(SMLoc DirectiveLoc, bool ExpectBlank) {
   TheCondStack.push_back(TheCondState);
   TheCondState.TheCond = AsmCond::IfCond;
 
   if (TheCondState.Ignore) {
     eatToEndOfStatement();
   } else {
     std::string Str;
     if (parseTextItem(Str))
-      return TokError("expected string parameter for 'ifb' directive");
+      return TokError("expected text item parameter for 'ifb' directive");
 
     if (parseToken(AsmToken::EndOfStatement,
                    "unexpected token in 'ifb' directive"))
       return true;
 
     TheCondState.CondMet = ExpectBlank == Str.empty();
     TheCondState.Ignore = !TheCondState.CondMet;
   }
 
   return false;
 }
 
 /// parseDirectiveIfidn
-///   ::= ifidn string1, string2
-bool MasmParser::parseDirectiveIfidn(SMLoc DirectiveLoc, bool ExpectEqual, bool CaseInsensitive) {
+///   ::= ifidn textitem, textitem
+bool MasmParser::parseDirectiveIfidn(SMLoc DirectiveLoc, bool ExpectEqual,
+                                     bool CaseInsensitive) {
   std::string String1, String2;
 
   if (parseTextItem(String1)) {
     if (ExpectEqual)
-      return TokError("expected string parameter for 'ifidn' directive");
-    return TokError("expected string parameter for 'ifdif' directive");
+      return TokError("expected text item parameter for 'ifidn' directive");
+    return TokError("expected text item parameter for 'ifdif' directive");
   }
 
   if (Lexer.isNot(AsmToken::Comma)) {
     if (ExpectEqual)
       return TokError(
           "expected comma after first string for 'ifidn' directive");
     return TokError("expected comma after first string for 'ifdif' directive");
   }
   Lex();
 
   if (parseTextItem(String2)) {
     if (ExpectEqual)
-      return TokError("expected string parameter for 'ifidn' directive");
-    return TokError("expected string parameter for 'ifdif' directive");
+      return TokError("expected text item parameter for 'ifidn' directive");
+    return TokError("expected text item parameter for 'ifdif' directive");
   }
 
   TheCondStack.push_back(TheCondState);
   TheCondState.TheCond = AsmCond::IfCond;
   if (CaseInsensitive)
     TheCondState.CondMet =
         ExpectEqual == (StringRef(String1).equals_lower(String2));
   else
if (LastIgnoreState || TheCondState.CondMet) {
     TheCondState.CondMet = ExprValue;
     TheCondState.Ignore = !TheCondState.CondMet;
   }
 
   return false;
 }
 
 /// parseDirectiveElseIfb
-/// ::= elseifb expression
+/// ::= elseifb textitem
 bool MasmParser::parseDirectiveElseIfb(SMLoc DirectiveLoc, bool ExpectBlank) {
   if (TheCondState.TheCond != AsmCond::IfCond &&
       TheCondState.TheCond != AsmCond::ElseIfCond)
     return Error(DirectiveLoc, "Encountered an elseif that doesn't follow an"
                                " if or an elseif");
   TheCondState.TheCond = AsmCond::ElseIfCond;
 
   bool LastIgnoreState = false;
   if (!TheCondStack.empty())
     LastIgnoreState = TheCondStack.back().Ignore;
   if (LastIgnoreState || TheCondState.CondMet) {
     TheCondState.Ignore = true;
     eatToEndOfStatement();
   } else {
     std::string Str;
-    if (parseTextItem(Str))
-      return TokError("expected string parameter for 'elseifb' directive");
+    if (parseTextItem(Str)) {
+      if (ExpectBlank)
+        return TokError("expected text item parameter for 'elseifb' directive");
+      return TokError("expected text item parameter for 'elseifnb' directive");
+    }
 
     if (parseToken(AsmToken::EndOfStatement,
                    "unexpected token in 'elseifb' directive"))
       return true;
 
     TheCondState.CondMet = ExpectBlank == Str.empty();
     TheCondState.Ignore = !TheCondState.CondMet;
   }
if (LastIgnoreState || TheCondState.CondMet) {
     TheCondState.CondMet = (is_defined == expect_defined);
     TheCondState.Ignore = !TheCondState.CondMet;
   }
 
   return false;
 }
 
 /// parseDirectiveElseIfidn
-/// ::= elseifidn string1, string2
+/// ::= elseifidn textitem, textitem
 bool MasmParser::parseDirectiveElseIfidn(SMLoc DirectiveLoc, bool ExpectEqual,
                                          bool CaseInsensitive) {
   if (TheCondState.TheCond != AsmCond::IfCond &&
       TheCondState.TheCond != AsmCond::ElseIfCond)
     return Error(DirectiveLoc, "Encountered an elseif that doesn't follow an"
                                " if or an elseif");
   TheCondState.TheCond = AsmCond::ElseIfCond;
 
   bool LastIgnoreState = false;
   if (!TheCondStack.empty())
     LastIgnoreState = TheCondStack.back().Ignore;
   if (LastIgnoreState || TheCondState.CondMet) {
     TheCondState.Ignore = true;
     eatToEndOfStatement();
   } else {
     std::string String1, String2;
 
     if (parseTextItem(String1)) {
       if (ExpectEqual)
-        return TokError("expected string parameter for 'elseifidn' directive");
-      return TokError("expected string parameter for 'elseifdif' directive");
+        return TokError(
+            "expected text item parameter for 'elseifidn' directive");
+      return TokError("expected text item parameter for 'elseifdif' directive");
     }
 
     if (Lexer.isNot(AsmToken::Comma)) {
       if (ExpectEqual)
         return TokError(
             "expected comma after first string for 'elseifidn' directive");
       return TokError(
           "expected comma after first string for 'elseifdif' directive");
     }
     Lex();
 
     if (parseTextItem(String2)) {
       if (ExpectEqual)
-        return TokError("expected string parameter for 'elseifidn' directive");
-      return TokError("expected string parameter for 'elseifdif' directive");
+        return TokError(
+            "expected text item parameter for 'elseifidn' directive");
+      return TokError("expected text item parameter for 'elseifdif' directive");
     }
 
     if (CaseInsensitive)
       TheCondState.CondMet =
           ExpectEqual == (StringRef(String1).equals_lower(String2));
     else
       TheCondState.CondMet = ExpectEqual == (String1 == String2);
     TheCondState.Ignore = !TheCondState.CondMet;
bool MasmParser::parseDirectiveErrorIfdef(SMLoc DirectiveLoc,
   Lex();
 
   if (IsDefined == ExpectDefined)
     return Error(DirectiveLoc, Message);
   return false;
 }
 
 /// parseDirectiveErrorIfidn
-///   ::= .erridn textitem1, textitem2[, message]
+///   ::= .erridn textitem, textitem[, message]
 bool MasmParser::parseDirectiveErrorIfidn(SMLoc DirectiveLoc, bool ExpectEqual,
                                           bool CaseInsensitive) {
   if (!TheCondStack.empty()) {
     if (TheCondStack.back().Ignore) {
       eatToEndOfStatement();
       return false;
     }
   }

llvm/test/tools/llvm-ml/macro_function.test

 factorial2 MACRO n
   IF n LE 1
     EXITM <(1)>
   ELSE
     EXITM <(n)*(factorial(n-1))>
   ENDIF
 ENDM
 
+string_recursive_test PROC
+; CHECK-LABEL: string_recursive_test:
+
+  mov eax, factorial(5)
+; CHECK: mov eax, 120
+  mov eax, factorial2(4)
+; CHECK: mov eax, 24
+  mov eax, 11 + factorial(6) - 11
+; CHECK: mov eax, 720
+
+  ret
+string_recursive_test ENDP
+
 fibonacci MACRO n
   IF n LE 2
-    EXITM <(1)>
+    EXITM %1
   ELSE
-    EXITM <(fibonacci(n-1)+fibonacci(n-2))>
+    EXITM %fibonacci(n-1)+fibonacci(n-2)
   ENDIF
 ENDM
 
-recursive_test PROC
-; CHECK-LABEL: recursive_test:
+expr_recursive_test PROC
+; CHECK-LABEL: expr_recursive_test:
 
-  mov eax, factorial(5)
-; CHECK: mov eax, 120
-  mov eax, factorial2(4)
-; CHECK: mov eax, 24
-  mov eax, 11 + fibonacci(7) - 11
-; CHECK: mov eax, 13
+  mov eax, fibonacci(10)
+; CHECK: mov eax, 55
 
   ret
-recursive_test ENDP
+expr_recursive_test ENDP
 
 end