[llvm-rc] Add MENU parsing ability. [4/8]

[mnbvmar]

Summary

This extends llvm-rc parsing tool by MENU resource (msdn.microsoft.com/en-us/library/windows/desktop/aa381025(v=vs.85).aspx). As for now, MENUEX (msdn.microsoft.com/en-us/library/windows/desktop/aa381023(v=vs.85).aspx) seems unnecessary.

[mnbvmar]

Small bugfixes (compilation problems on gcc and missing newlines at the end of the files).

[ecbeckmann]

llvm/tools/llvm-rc/ResourceScriptParser.cpp

316

I've never seen this before. Is there a reason to using a comma instead of just putting as a separate statement?

320

do the early continue for better readability, and remove the else block.

i.e.

while (....) {
......
  if (IsMenuItem && isNextTokenKind(Kind::Identifier)) {
  .....
  continue;
  }
  ASSIGN_OR_RETURN(CaptionResult, readString());
  .......
}

323

doesn't ASSIGN_OR_RETURN go here?

344

can use same early continue pattern here.

393

Sorry this isn't part of this revision, but this should have a better name, like just "createError()"

[mnbvmar]

Small fix-ups for the problems mentioned above.

[mnbvmar]

llvm/tools/llvm-rc/ResourceScriptParser.cpp

393

OK, might be. As this name is scattered around the file, I suggest that I refactor it as soon as all the parser parts are accepted.

[ecbeckmann]

This looks fine now.

[rnk]

Can you rebase this and the other patches and I'll take another look?

llvm/test/tools/llvm-rc/parser.test

120

The same comment re: pipes applies here.

[mnbvmar]

Use pipes instead of temporary files.

[rnk]

llvm/tools/llvm-rc/ResourceScriptParser.cpp

343

ArrayRef will help here too.

llvm/test/tools/llvm-rc/Inputs/parser-correct-everything.rc

LANGUAGE 0, 2
   "^C", 10
   14, 11
   5, 12, VIRTKEY
   0, 0, ASCII
   1, 1, VIRTKEY, CONTROL
   2, 2, CONTROL, VIRTKEY
   3, 3, ALT, CONTROL, SHIFT, NOINVERT, ASCII, VIRTKEY
 }
+
+LLVMTest MENU
+LANGUAGE 4, 1
+{
+  POPUP "&OneMenu"
+  {
+    POPUP "Menu&1"
+    {
+      MENUITEM "Item&1", 301, MENUBREAK, CHECKED
+      MENUITEM "Item&2", 302, CHECKED, MENUBARBREAK
+      MENUITEM "Item&3", 303, MENUBREAK, INACTIVE, HELP
+      MENUITEM "Item&4", 304, GRAYED
+    }
+    POPUP "Menu&2"
+    {
+      MENUITEM "&A", 401
+      MENUITEM "&B", 402
+    }
+  }
+  POPUP "&Items"
+  {
+    MENUITEM "&Row", 500
+    MENUITEM "&Column", 501, CHECKED
+    MENUITEM SEPARATOR
+    MENUITEM "&Word", 502
+  } 
+}

llvm/test/tools/llvm-rc/Inputs/parser-menu-bad-flag.rc

+1 MENU {
+  MENUITEM "&Item", 500, MENUBREAK, ERRONEOUS, HELP
+}

llvm/test/tools/llvm-rc/Inputs/parser-menu-bad-id.rc

+1 MENU {
+  MENUITEM "Hello", A
+}

llvm/test/tools/llvm-rc/Inputs/parser-menu-missing-block.rc

+1 MENU {
+  POPUP "1"
+  POPUP "2" {}
+}

llvm/test/tools/llvm-rc/Inputs/parser-menu-misspelled-separator.rc

+1 MENU {
+  MENUITEM NOTSEPARATOR
+}

llvm/test/tools/llvm-rc/parser.test

 ; PGOOD-NEXT:    Option: Language: 0, Sublanguage: 2
 ; PGOOD-NEXT:    Accelerator: "^C" 10
 ; PGOOD-NEXT:    Accelerator: 14 11
 ; PGOOD-NEXT:    Accelerator: 5 12 VIRTKEY
 ; PGOOD-NEXT:    Accelerator: 0 0 ASCII
 ; PGOOD-NEXT:    Accelerator: 1 1 VIRTKEY CONTROL
 ; PGOOD-NEXT:    Accelerator: 2 2 VIRTKEY CONTROL
 ; PGOOD-NEXT:    Accelerator: 3 3 ASCII VIRTKEY NOINVERT ALT SHIFT CONTROL
+; PGOOD-NEXT:  Menu (LLVMTest):
+; PGOOD-NEXT:    Option: Language: 4, Sublanguage: 1
+; PGOOD-NEXT:    Menu list starts
+; PGOOD-NEXT:    Popup ("&OneMenu"):
+; PGOOD-NEXT:    Menu list starts
+; PGOOD-NEXT:    Popup ("Menu&1"):
+; PGOOD-NEXT:    Menu list starts
+; PGOOD-NEXT:    MenuItem ("Item&1"), ID = 301 CHECKED MENUBREAK
+; PGOOD-NEXT:    MenuItem ("Item&2"), ID = 302 CHECKED MENUBARBREAK
+; PGOOD-NEXT:    MenuItem ("Item&3"), ID = 303 HELP INACTIVE MENUBREAK
+; PGOOD-NEXT:    MenuItem ("Item&4"), ID = 304 GRAYED
+; PGOOD-NEXT:    Menu list ends
+; PGOOD-NEXT:    Popup ("Menu&2"):
+; PGOOD-NEXT:    Menu list starts
+; PGOOD-NEXT:    MenuItem ("&A"), ID = 401
+; PGOOD-NEXT:    MenuItem ("&B"), ID = 402
+; PGOOD-NEXT:    Menu list ends
+; PGOOD-NEXT:    Menu list ends
+; PGOOD-NEXT:    Popup ("&Items"):
+; PGOOD-NEXT:    Menu list starts
+; PGOOD-NEXT:    MenuItem ("&Row"), ID = 500
+; PGOOD-NEXT:    MenuItem ("&Column"), ID = 501 CHECKED
+; PGOOD-NEXT:    Menu separator
+; PGOOD-NEXT:    MenuItem ("&Word"), ID = 502
+; PGOOD-NEXT:    Menu list ends
+; PGOOD-NEXT:    Menu list ends
 
 
 ; RUN: not llvm-rc /V %p/Inputs/parser-stringtable-no-string.rc 2> %t2
 ; RUN: FileCheck %s --check-prefix PSTRINGTABLE1 --input-file %t2
 
 ; PSTRINGTABLE1:  llvm-rc: Error parsing file: expected string, got }
 
 

 
 ; PHTML1:  llvm-rc: Error parsing file: expected string, got ThisPassesInTheOriginalToolButDocSaysItShouldBeQuoted
 
 
 ; RUN: not llvm-rc /V %p/Inputs/parser-html-extra-comma.rc 2> %t12
 ; RUN: FileCheck %s --check-prefix PHTML2 --input-file %t12
 
 ; PHTML2:  llvm-rc: Error parsing file: expected string, got ,
+
+
+; RUN: not llvm-rc /V %p/Inputs/parser-menu-bad-id.rc 2> %t13
+; RUN: FileCheck %s --check-prefix PMENU1 --input-file %t13
+
+; PMENU1:  llvm-rc: Error parsing file: expected integer, got A
+
+
+; RUN: not llvm-rc /V %p/Inputs/parser-menu-bad-flag.rc 2> %t14
+; RUN: FileCheck %s --check-prefix PMENU2 --input-file %t14
+
+; PMENU2:  llvm-rc: Error parsing file: expected CHECKED/GRAYED/HELP/INACTIVE/MENUBARBREAK/MENUBREAK, got ERRONEOUS
+
+
+; RUN: not llvm-rc /V %p/Inputs/parser-menu-missing-block.rc 2> %t15
+; RUN: FileCheck %s --check-prefix PMENU3 --input-file %t15
+
+; PMENU3:  llvm-rc: Error parsing file: expected '{', got POPUP
+
+
+; RUN: not llvm-rc /V %p/Inputs/parser-menu-misspelled-separator.rc 2> %t16
+; RUN: FileCheck %s --check-prefix PMENU4 --input-file %t16
+
+; PMENU4:  llvm-rc: Error parsing file: expected SEPARATOR or string, got NOTSEPARATOR
 No newline at end of file

llvm/tools/llvm-rc/ResourceScriptParser.h

private:
   parseSingleOptionalStatement(bool UseExtendedStatements = false);
 
   // Top-level resource parsers.
   ParseType parseLanguageResource();
   ParseType parseAcceleratorsResource();
   ParseType parseCursorResource();
   ParseType parseIconResource();
   ParseType parseHTMLResource();
+  ParseType parseMenuResource();
   ParseType parseStringTableResource();
 
+  // Helper MENU parser.
+  Expected<MenuDefinitionList> parseMenuItemsList();
+
   // Optional statement parsers.
   ParseOptionType parseLanguageStmt();
   ParseOptionType parseCharacteristicsStmt();
   ParseOptionType parseVersionStmt();
 
   // Raises an error. If IsAlreadyRead = false (default), this complains about
   // the token that couldn't be parsed. If the flag is on, this complains about
   // the correctly read token that makes no sense (that is, the current parser

llvm/tools/llvm-rc/ResourceScriptParser.cpp

RCParser::ParseType RCParser::parseSingleResource() {
   if (TypeToken->equalsLower("ACCELERATORS"))
     Result = parseAcceleratorsResource();
   else if (TypeToken->equalsLower("CURSOR"))
     Result = parseCursorResource();
   else if (TypeToken->equalsLower("ICON"))
     Result = parseIconResource();
   else if (TypeToken->equalsLower("HTML"))
     Result = parseHTMLResource();
+  else if (TypeToken->equalsLower("MENU"))
+    Result = parseMenuResource();
   else
     return getExpectedError("resource type", /* IsAlreadyRead = */ true);
 
   if (Result)
     (*Result)->setName(*NameToken);
 
   return Result;
 }
RCParser::ParseType RCParser::parseIconResource() {
   return make_unique<IconResource>(*Arg);
 }
 
 RCParser::ParseType RCParser::parseHTMLResource() {
   ASSIGN_OR_RETURN(Arg, readString());
   return make_unique<HTMLResource>(*Arg);
 }
 
+RCParser::ParseType RCParser::parseMenuResource() {
+  ASSIGN_OR_RETURN(OptStatements, parseOptionalStatements());
+  ASSIGN_OR_RETURN(Items, parseMenuItemsList());
+  return make_unique<MenuResource>(std::move(*OptStatements),
+                                   std::move(*Items));
+}
+
+Expected<MenuDefinitionList> RCParser::parseMenuItemsList() {
+  RETURN_IF_ERROR(consumeType(Kind::BlockBegin));
+
+  MenuDefinitionList List;
+
+  // Read a set of items. Each item is of one of three kinds:
+  //   MENUITEM SEPARATOR
+  //   MENUITEM caption:String, result:Int [, menu flags]...
+  //   POPUP caption:String [, menu flags]... { items... }
+  while (!consumeOptionalType(Kind::BlockEnd)) {
+    ASSIGN_OR_RETURN(ItemTypeResult, readIdentifier());
+
+    bool IsMenuItem = ItemTypeResult->equals_lower("MENUITEM"),
+         IsPopup = ItemTypeResult->equals_lower("POPUP");
+    if (!IsMenuItem && !IsPopup)
+      return getExpectedError("MENUITEM, POPUP, END or '}'", true);
+
+    if (IsMenuItem && isNextTokenKind(Kind::Identifier)) {
+      // Now, expecting SEPARATOR.
+      auto SeparatorResult = readIdentifier();
+      (void)!SeparatorResult; // We know it's an identifier.
+      if (SeparatorResult->equals_lower("SEPARATOR"))
+        List.addDefinition(make_unique<MenuSeparator>());
+      else
+        return getExpectedError("SEPARATOR or string", true);
+    } else {
+      // Read the caption.
+      ASSIGN_OR_RETURN(CaptionResult, readString());
+
+      // If MENUITEM, expect also a comma and an integer.
+      uint32_t MenuResult = -1;
+
+      if (IsMenuItem) {
+        RETURN_IF_ERROR(consumeType(Kind::Comma));
+        ASSIGN_OR_RETURN(IntResult, readInt());
+        MenuResult = *IntResult;
+      }
+
+      ASSIGN_OR_RETURN(FlagsResult, parseFlags(MenuDefinition::OptionsStr,
+                                               MenuDefinition::NumFlags));
+
+      if (IsPopup) {
+        // If POPUP, read submenu items recursively.
+        ASSIGN_OR_RETURN(SubMenuResult, parseMenuItemsList());
+        List.addDefinition(make_unique<PopupItem>(*CaptionResult, *FlagsResult,
+                                                  std::move(*SubMenuResult)));
+      } else {
+        List.addDefinition(
+            make_unique<MenuItem>(*CaptionResult, MenuResult, *FlagsResult));
+      }
+    }
+  }
+
+  return std::move(List);
+}
+
 RCParser::ParseType RCParser::parseStringTableResource() {
   ASSIGN_OR_RETURN(OptStatements, parseOptionalStatements());
   RETURN_IF_ERROR(consumeType(Kind::BlockBegin));
 
   auto Table = make_unique<StringTableResource>(std::move(*OptStatements));
 
   // Read strings until we reach the end of the block.
   while (!consumeOptionalType(Kind::BlockEnd)) {
RCParser::ParseOptionType RCParser::parseCharacteristicsStmt() {
   return make_unique<CharacteristicsStmt>(*Arg);
 }
 
 RCParser::ParseOptionType RCParser::parseVersionStmt() {
   ASSIGN_OR_RETURN(Arg, readInt());
   return make_unique<VersionStmt>(*Arg);
 }
 
 Error RCParser::getExpectedError(const Twine Message, bool IsAlreadyRead) {
   return make_error<ParserError>(
       Message, IsAlreadyRead ? std::prev(CurLoc) : CurLoc, End);
 }
 
 } // namespace rc
 } // namespace llvm

llvm/tools/llvm-rc/ResourceScriptStmt.h

 class HTMLResource : public RCResource {
   StringRef HTMLLoc;
 
 public:
   HTMLResource(StringRef Location) : HTMLLoc(Location) {}
   raw_ostream &log(raw_ostream &) const override;
 };
 
+// -- MENU resource and its helper classes --
+// This resource describes the contents of an application menu
+// (usually located in the upper part of the dialog.)
+//
+// Ref: msdn.microsoft.com/en-us/library/windows/desktop/aa381025(v=vs.85).aspx
+
+// Description of a single submenu item.
+class MenuDefinition {
+public:
+  enum Options {
+    CHECKED = (1 << 0),
+    GRAYED = (1 << 1),
+    HELP = (1 << 2),
+    INACTIVE = (1 << 3),
+    MENUBARBREAK = (1 << 4),
+    MENUBREAK = (1 << 5)
+  };
+
+  static constexpr size_t NumFlags = 6;
+  static StringRef OptionsStr[NumFlags];
+  static raw_ostream &logFlags(raw_ostream &, uint8_t Flags);
+  virtual raw_ostream &log(raw_ostream &OS) const {
+    return OS << "Base menu definition\n";
+  }
+  virtual ~MenuDefinition() {}
+};
+
+// Recursive description of a whole submenu.
+class MenuDefinitionList : public MenuDefinition {
+  std::vector<std::unique_ptr<MenuDefinition>> Definitions;
+
+public:
+  void addDefinition(std::unique_ptr<MenuDefinition> Def) {
+    Definitions.push_back(std::move(Def));
+  }
+  raw_ostream &log(raw_ostream &) const override;
+};
+
+// Separator in MENU definition (MENUITEM SEPARATOR).
+//
+// Ref: msdn.microsoft.com/en-us/library/windows/desktop/aa381024(v=vs.85).aspx
+class MenuSeparator : public MenuDefinition {
+public:
+  raw_ostream &log(raw_ostream &) const override;
+};
+
+// MENUITEM statement definition.
+//
+// Ref: msdn.microsoft.com/en-us/library/windows/desktop/aa381024(v=vs.85).aspx
+class MenuItem : public MenuDefinition {
+  StringRef Name;
+  uint32_t Id;
+  uint8_t Flags;
+
+public:
+  MenuItem(StringRef Caption, uint32_t ItemId, uint8_t ItemFlags)
+      : Name(Caption), Id(ItemId), Flags(ItemFlags) {}
+  raw_ostream &log(raw_ostream &) const override;
+};
+
+// POPUP statement definition.
+//
+// Ref: msdn.microsoft.com/en-us/library/windows/desktop/aa381030(v=vs.85).aspx
+class PopupItem : public MenuDefinition {
+  StringRef Name;
+  uint8_t Flags;
+  MenuDefinitionList SubItems;
+
+public:
+  PopupItem(StringRef Caption, uint8_t ItemFlags,
+            MenuDefinitionList &&SubItemsList)
+      : Name(Caption), Flags(ItemFlags), SubItems(std::move(SubItemsList)) {}
+  raw_ostream &log(raw_ostream &) const override;
+};
+
+// Menu resource definition.
+class MenuResource : public RCResource {
+  OptionalStmtList OptStatements;
+  MenuDefinitionList Elements;
+
+public:
+  MenuResource(OptionalStmtList &&OptStmts, MenuDefinitionList &&Items)
+      : OptStatements(std::move(OptStmts)), Elements(std::move(Items)) {}
+  raw_ostream &log(raw_ostream &) const override;
+};
+
 // STRINGTABLE resource. Contains a list of strings, each having its unique ID.
 //
 // Ref: msdn.microsoft.com/en-us/library/windows/desktop/aa381050(v=vs.85).aspx
 class StringTableResource : public RCResource {
   OptionalStmtList OptStatements;
   std::vector<std::pair<uint32_t, StringRef>> Table;
 
 public:

llvm/tools/llvm-rc/ResourceScriptStmt.cpp

 raw_ostream &IconResource::log(raw_ostream &OS) const {
   return OS << "Icon (" << ResName << "): " << IconLoc << "\n";
 }
 
 raw_ostream &HTMLResource::log(raw_ostream &OS) const {
   return OS << "HTML (" << ResName << "): " << HTMLLoc << "\n";
 }
 
+StringRef MenuDefinition::OptionsStr[MenuDefinition::NumFlags] = {
+    "CHECKED", "GRAYED", "HELP", "INACTIVE", "MENUBARBREAK", "MENUBREAK"};
+
+raw_ostream &MenuDefinition::logFlags(raw_ostream &OS, uint8_t Flags) {
+  for (size_t i = 0; i < NumFlags; ++i)
+    if (Flags & (1U << i))
+      OS << " " << OptionsStr[i];
+  return OS;
+}
+
+raw_ostream &MenuDefinitionList::log(raw_ostream &OS) const {
+  OS << "  Menu list starts\n";
+  for (auto &Item : Definitions)
+    Item->log(OS);
+  return OS << "  Menu list ends\n";
+}
+
+raw_ostream &MenuItem::log(raw_ostream &OS) const {
+  OS << "  MenuItem (" << Name << "), ID = " << Id;
+  logFlags(OS, Flags);
+  return OS << "\n";
+}
+
+raw_ostream &MenuSeparator::log(raw_ostream &OS) const {
+  return OS << "  Menu separator\n";
+}
+
+raw_ostream &PopupItem::log(raw_ostream &OS) const {
+  OS << "  Popup (" << Name << ")";
+  logFlags(OS, Flags);
+  OS << ":\n";
+  return SubItems.log(OS);
+}
+
+raw_ostream &MenuResource::log(raw_ostream &OS) const {
+  OS << "Menu (" << ResName << "):\n";
+  OptStatements.log(OS);
+  return Elements.log(OS);
+}
+
 raw_ostream &StringTableResource::log(raw_ostream &OS) const {
   OS << "StringTable:\n";
   OptStatements.log(OS);
   for (const auto &String : Table)
     OS << "  " << String.first << " => " << String.second << "\n";
   return OS;
 }