[llvm] [llvm-rc] add support for MENUEX (PR #67464)

[Martin Storsjö via llvm-commits]

================
@@ -682,6 +691,97 @@ Expected<MenuDefinitionList> RCParser::parseMenuItemsList() {
   return std::move(List);
 }
 
+Expected<MenuDefinitionList> RCParser::parseMenuExItemsList() {
+  RETURN_IF_ERROR(consumeType(Kind::BlockBegin));
+
+  MenuDefinitionList List;
+
+  // Read a set of items. Each item is of one of three kinds:
+  //   MENUITEM caption:String [,[id][, [type][, state]]]]
+  //   POPUP caption:String [,[id][, [type][, [state][, helpID]]]] { popupBody }
+  //   }
+  while (!consumeOptionalType(Kind::BlockEnd)) {
+    ASSIGN_OR_RETURN(ItemTypeResult, readIdentifier());
+
+    bool IsMenuItem = ItemTypeResult->equals_insensitive("MENUITEM");
+    bool IsPopup = ItemTypeResult->equals_insensitive("POPUP");
+    if (!IsMenuItem && !IsPopup)
+      return getExpectedError("MENUITEM, POPUP, END or '}'", true);
+
+    // Not a separator. Read the caption.
+    ASSIGN_OR_RETURN(CaptionResult, readString());
+
+    // If MENUITEM, expect [,[id][, [type][, state]]]]
+    uint32_t MenuId = 0;
+    uint32_t MenuType = 0;
+    uint32_t MenuState = 0;
+
+    if (IsMenuItem) {
+      if (consumeOptionalType(Kind::Comma)) {
+        auto IntId = readInt();
+        if (IntId) {
+          MenuId = *IntId;
+        }
+        if (consumeOptionalType(Kind::Comma)) {
+          auto IntType = readInt();
+          if (IntType) {
+            MenuType = *IntType;
+          }
+          if (consumeOptionalType(Kind::Comma)) {
+            auto IntState = readInt();
+            if (IntState) {
+              MenuState = *IntState;
+            }
+          }
+        }
+      }
+    }
+
+    uint32_t PopupId = 0;
+    uint32_t PopupType = 0;
+    uint32_t PopupState = 0;
+    uint32_t PopupHelpID = 0;
+    if (IsPopup) {
+      if (consumeOptionalType(Kind::Comma)) {
+        auto IntId = readInt();
+        if (IntId) {
+          PopupId = *IntId;
+        }
+        if (consumeOptionalType(Kind::Comma)) {
+          auto IntType = readInt();
+          if (IntType) {
+            PopupType = *IntType;
+          }
+          if (consumeOptionalType(Kind::Comma)) {
+            auto IntState = readInt();
+            if (IntState) {
+              PopupState = *IntState;
+            }
+            if (consumeOptionalType(Kind::Comma)) {
+              auto IntHelpID = readInt();
+              if (IntHelpID) {
+                PopupHelpID = *IntHelpID;
+              }
+            }
+          }
+        }
+      }
+      // If POPUP, read submenu items recursively.
+      ASSIGN_OR_RETURN(SubMenuResult, parseMenuExItemsList());
+      List.addDefinition(std::make_unique<PopupExItem>(
+          *CaptionResult, PopupId, PopupType, PopupState, PopupHelpID,
+          std::move(*SubMenuResult)));
+      continue;
+    }
+
+    assert(IsMenuItem);
+    List.addDefinition(std::make_unique<MenuExItem>(*CaptionResult, MenuId,
----------------
mstorsjo wrote:

This code feels rather weird here, even if I see how it's modelled after the existing case above. Could we move the whole menuitem code into the `if (IsMenuItem)` above, instead of having half of the code above and then the rest here? Then you could get rid of the `if (IsPopup)` and change it into `assert(isPopup)` and remove one level of indentation.

And then you could move the menuitem variables, `uint32_t MenuId` etc, into the `if (IsMenuItem)` scope, making the variable scoping clearer.

https://github.com/llvm/llvm-project/pull/67464