[llvm-rc] Refactoring needed for ACCELERATORS and MENU resources.

This is a part of llvm-rc serialization patch set (serialization, pt 1.5).

This:

* Unifies the internal representation of flags in ACCELERATORS and MENU
   with the corresponding representation in .res files (noticed in
   https://reviews.llvm.org/D37828#inline-329828).
* Creates an RCResource subclass, OptStatementsRCResource, describing
   resource statements that can declare resource-local optional statements
   (proposed in https://reviews.llvm.org/D37824#inline-329775).

These modifications don't fit to any of the current patches, so I'm
submitting them as a separate patch.

Differential Revision: https://reviews.llvm.org/D37841

llvm-svn: 314541

tools/llvm-rc/ResourceScriptParser.cpp

@@ -293,9 +293,10 @@ RCParser::readIntsWithCommas(size_t MinCount, size_t MaxCount) {
   return std::move(Result);
 }
 
-Expected<uint32_t> RCParser::parseFlags(ArrayRef<StringRef> FlagDesc) {
-  assert(FlagDesc.size() <= 32 && "More than 32 flags won't fit in result.");
+Expected<uint32_t> RCParser::parseFlags(ArrayRef<StringRef> FlagDesc,
+                                        ArrayRef<uint32_t> FlagValues) {
   assert(!FlagDesc.empty());
+  assert(FlagDesc.size() == FlagValues.size());
 
   uint32_t Result = 0;
   while (isNextTokenKind(Kind::Comma)) {
@@ -307,7 +308,7 @@ Expected<uint32_t> RCParser::parseFlags(ArrayRef<StringRef> FlagDesc) {
       if (!FlagResult->equals_lower(FlagDesc[FlagId]))
         continue;
 
-      Result |= (1U << FlagId);
+      Result |= FlagValues[FlagId];
       FoundFlag = true;
       break;
     }
@@ -372,8 +373,10 @@ RCParser::ParseType RCParser::parseAcceleratorsResource() {
     ASSIGN_OR_RETURN(EventResult, readIntOrString());
     RETURN_IF_ERROR(consumeType(Kind::Comma));
     ASSIGN_OR_RETURN(IDResult, readInt());
-    ASSIGN_OR_RETURN(FlagsResult,
-                     parseFlags(AcceleratorsResource::Accelerator::OptionsStr));
+    ASSIGN_OR_RETURN(
+        FlagsResult,
+        parseFlags(AcceleratorsResource::Accelerator::OptionsStr,
+                   AcceleratorsResource::Accelerator::OptionsFlags));
     Accels->addAccelerator(*EventResult, *IDResult, *FlagsResult);
   }
 
@@ -536,7 +539,8 @@ Expected<MenuDefinitionList> RCParser::parseMenuItemsList() {
       MenuResult = *IntResult;
     }
 
-    ASSIGN_OR_RETURN(FlagsResult, parseFlags(MenuDefinition::OptionsStr));
+    ASSIGN_OR_RETURN(FlagsResult, parseFlags(MenuDefinition::OptionsStr,
+                                             MenuDefinition::OptionsFlags));
 
     if (IsPopup) {
       // If POPUP, read submenu items recursively.

tools/llvm-rc/ResourceScriptParser.h

@@ -106,10 +106,11 @@ private:
 
   // Read an unknown number of flags preceded by commas. Each correct flag
   // has an entry in FlagDesc array of length NumFlags. In case i-th
-  // flag (0-based) has been read, the i-th bit of the result is set.
+  // flag (0-based) has been read, the result is OR-ed with FlagValues[i].
   // As long as parser has a comma to read, it expects to be fed with
   // a correct flag afterwards.
-  Expected<uint32_t> parseFlags(ArrayRef<StringRef> FlagDesc);
+  Expected<uint32_t> parseFlags(ArrayRef<StringRef> FlagDesc,
+                                ArrayRef<uint32_t> FlagValues);
 
   // Reads a set of optional statements. These can change the behavior of
   // a number of resource types (e.g. STRINGTABLE, MENU or DIALOG) if provided

tools/llvm-rc/ResourceScriptStmt.cpp

@@ -40,13 +40,17 @@ StringRef AcceleratorsResource::Accelerator::OptionsStr
     [AcceleratorsResource::Accelerator::NumFlags] = {
         "ASCII", "VIRTKEY", "NOINVERT", "ALT", "SHIFT", "CONTROL"};
 
+uint32_t AcceleratorsResource::Accelerator::OptionsFlags
+    [AcceleratorsResource::Accelerator::NumFlags] = {ASCII, VIRTKEY, NOINVERT,
+                                                     ALT,   SHIFT,   CONTROL};
+
 raw_ostream &AcceleratorsResource::log(raw_ostream &OS) const {
   OS << "Accelerators (" << ResName << "): \n";
-  OptStatements.log(OS);
+  OptStatements->log(OS);
   for (const auto &Acc : Accelerators) {
     OS << "  Accelerator: " << Acc.Event << " " << Acc.Id;
     for (size_t i = 0; i < Accelerator::NumFlags; ++i)
-      if (Acc.Flags & (1U << i))
+      if (Acc.Flags & Accelerator::OptionsFlags[i])
         OS << " " << Accelerator::OptionsStr[i];
     OS << "\n";
   }
@@ -68,9 +72,12 @@ raw_ostream &HTMLResource::log(raw_ostream &OS) const {
 StringRef MenuDefinition::OptionsStr[MenuDefinition::NumFlags] = {
     "CHECKED", "GRAYED", "HELP", "INACTIVE", "MENUBARBREAK", "MENUBREAK"};
 
-raw_ostream &MenuDefinition::logFlags(raw_ostream &OS, uint8_t Flags) {
+uint32_t MenuDefinition::OptionsFlags[MenuDefinition::NumFlags] = {
+    CHECKED, GRAYED, HELP, INACTIVE, MENUBARBREAK, MENUBREAK};
+
+raw_ostream &MenuDefinition::logFlags(raw_ostream &OS, uint16_t Flags) {
   for (size_t i = 0; i < NumFlags; ++i)
-    if (Flags & (1U << i))
+    if (Flags & OptionsFlags[i])
       OS << " " << OptionsStr[i];
   return OS;
 }
@@ -101,13 +108,13 @@ raw_ostream &PopupItem::log(raw_ostream &OS) const {
 
 raw_ostream &MenuResource::log(raw_ostream &OS) const {
   OS << "Menu (" << ResName << "):\n";
-  OptStatements.log(OS);
+  OptStatements->log(OS);
   return Elements.log(OS);
 }
 
 raw_ostream &StringTableResource::log(raw_ostream &OS) const {
   OS << "StringTable:\n";
-  OptStatements.log(OS);
+  OptStatements->log(OS);
   for (const auto &String : Table)
     OS << "  " << String.first << " => " << String.second << "\n";
   return OS;
@@ -136,7 +143,7 @@ raw_ostream &DialogResource::log(raw_ostream &OS) const {
   OS << "Dialog" << (IsExtended ? "Ex" : "") << " (" << ResName << "): loc: ("
      << X << ", " << Y << "), size: [" << Width << ", " << Height
      << "], help ID: " << HelpID << "\n";
-  OptStatements.log(OS);
+  OptStatements->log(OS);
   for (auto &Ctl : Controls)
     Ctl.log(OS);
   return OS;

tools/llvm-rc/ResourceScriptStmt.h

@@ -160,6 +160,14 @@ public:
   }
 };
 
+class OptStatementsRCResource : public RCResource {
+public:
+  std::unique_ptr<OptionalStmtList> OptStatements;
+
+  OptStatementsRCResource(OptionalStmtList &&Stmts)
+      : OptStatements(llvm::make_unique<OptionalStmtList>(std::move(Stmts))) {}
+};
+
 // LANGUAGE statement. It can occur both as a top-level statement (in such
 // a situation, it changes the default language until the end of the file)
 // and as an optional resource statement (then it changes the language
@@ -183,37 +191,41 @@ public:
 // ACCELERATORS resource. Defines a named table of accelerators for the app.
 //
 // Ref: msdn.microsoft.com/en-us/library/windows/desktop/aa380610(v=vs.85).aspx
-class AcceleratorsResource : public RCResource {
+class AcceleratorsResource : public OptStatementsRCResource {
 public:
   class Accelerator {
   public:
     IntOrString Event;
     uint32_t Id;
-    uint8_t Flags;
+    uint16_t Flags;
 
     enum Options {
-      ASCII = (1 << 0),
-      VIRTKEY = (1 << 1),
-      NOINVERT = (1 << 2),
-      ALT = (1 << 3),
-      SHIFT = (1 << 4),
-      CONTROL = (1 << 5)
+      // This is actually 0x0000 (accelerator is assumed to be ASCII if it's
+      // not VIRTKEY). However, rc.exe behavior is different in situations
+      // "only ASCII defined" and "neither ASCII nor VIRTKEY defined".
+      // Therefore, we include ASCII as another flag. This must be zeroed
+      // when serialized.
+      ASCII = 0x8000,
+      VIRTKEY = 0x0001,
+      NOINVERT = 0x0002,
+      ALT = 0x0010,
+      SHIFT = 0x0004,
+      CONTROL = 0x0008
     };
 
     static constexpr size_t NumFlags = 6;
     static StringRef OptionsStr[NumFlags];
+    static uint32_t OptionsFlags[NumFlags];
   };
 
-  AcceleratorsResource(OptionalStmtList &&OptStmts)
-      : OptStatements(std::move(OptStmts)) {}
-  void addAccelerator(IntOrString Event, uint32_t Id, uint8_t Flags) {
+  using OptStatementsRCResource::OptStatementsRCResource;
+  void addAccelerator(IntOrString Event, uint32_t Id, uint16_t Flags) {
     Accelerators.push_back(Accelerator{Event, Id, Flags});
   }
   raw_ostream &log(raw_ostream &) const override;
 
 private:
   std::vector<Accelerator> Accelerators;
-  OptionalStmtList OptStatements;
 };
 
 // CURSOR resource. Represents a single cursor (".cur") file.
@@ -272,17 +284,18 @@ public:
 class MenuDefinition {
 public:
   enum Options {
-    CHECKED = (1 << 0),
-    GRAYED = (1 << 1),
-    HELP = (1 << 2),
-    INACTIVE = (1 << 3),
-    MENUBARBREAK = (1 << 4),
-    MENUBREAK = (1 << 5)
+    CHECKED = 0x0008,
+    GRAYED = 0x0001,
+    HELP = 0x4000,
+    INACTIVE = 0x0002,
+    MENUBARBREAK = 0x0020,
+    MENUBREAK = 0x0040
   };
 
   static constexpr size_t NumFlags = 6;
   static StringRef OptionsStr[NumFlags];
-  static raw_ostream &logFlags(raw_ostream &, uint8_t Flags);
+  static uint32_t OptionsFlags[NumFlags];
+  static raw_ostream &logFlags(raw_ostream &, uint16_t Flags);
   virtual raw_ostream &log(raw_ostream &OS) const {
     return OS << "Base menu definition\n";
   }
@@ -314,10 +327,10 @@ public:
 class MenuItem : public MenuDefinition {
   StringRef Name;
   uint32_t Id;
-  uint8_t Flags;
+  uint16_t Flags;
 
 public:
-  MenuItem(StringRef Caption, uint32_t ItemId, uint8_t ItemFlags)
+  MenuItem(StringRef Caption, uint32_t ItemId, uint16_t ItemFlags)
       : Name(Caption), Id(ItemId), Flags(ItemFlags) {}
   raw_ostream &log(raw_ostream &) const override;
 };
@@ -327,37 +340,35 @@ public:
 // Ref: msdn.microsoft.com/en-us/library/windows/desktop/aa381030(v=vs.85).aspx
 class PopupItem : public MenuDefinition {
   StringRef Name;
-  uint8_t Flags;
+  uint16_t Flags;
   MenuDefinitionList SubItems;
 
 public:
-  PopupItem(StringRef Caption, uint8_t ItemFlags,
+  PopupItem(StringRef Caption, uint16_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;
+class MenuResource : public OptStatementsRCResource {
   MenuDefinitionList Elements;
 
 public:
   MenuResource(OptionalStmtList &&OptStmts, MenuDefinitionList &&Items)
-      : OptStatements(std::move(OptStmts)), Elements(std::move(Items)) {}
+      : OptStatementsRCResource(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;
+class StringTableResource : public OptStatementsRCResource {
   std::vector<std::pair<uint32_t, StringRef>> Table;
 
 public:
-  StringTableResource(OptionalStmtList &&OptStmts)
-      : OptStatements(std::move(OptStmts)) {}
+  using OptStatementsRCResource::OptStatementsRCResource;
   void addString(uint32_t ID, StringRef String) {
     Table.emplace_back(ID, String);
   }
@@ -395,9 +406,8 @@ public:
 // Single dialog definition. We don't create distinct classes for DIALOG and
 // DIALOGEX because of their being too similar to each other. We only have a
 // flag determining the type of the dialog box.
-class DialogResource : public RCResource {
+class DialogResource : public OptStatementsRCResource {
   uint32_t X, Y, Width, Height, HelpID;
-  OptionalStmtList OptStatements;
   std::vector<Control> Controls;
   bool IsExtended;
 
@@ -405,8 +415,9 @@ public:
   DialogResource(uint32_t PosX, uint32_t PosY, uint32_t DlgWidth,
                  uint32_t DlgHeight, uint32_t DlgHelpID,
                  OptionalStmtList &&OptStmts, bool IsDialogEx)
-      : X(PosX), Y(PosY), Width(DlgWidth), Height(DlgHeight), HelpID(DlgHelpID),
-        OptStatements(std::move(OptStmts)), IsExtended(IsDialogEx) {}
+      : OptStatementsRCResource(std::move(OptStmts)), X(PosX), Y(PosY),
+        Width(DlgWidth), Height(DlgHeight), HelpID(DlgHelpID),
+        IsExtended(IsDialogEx) {}
 
   void addControl(Control &&Ctl) { Controls.push_back(std::move(Ctl)); }