[llvm-rc] Add ACCELERATORS parsing ability. [3/8]

[mnbvmar]

Summary

This improves the current llvm-rc parser by the ability of parsing ACCELERATORS statement.

[ecbeckmann]

llvm/tools/llvm-rc/ResourceScriptParser.cpp

127

no else after return

[ecbeckmann]

llvm/tools/llvm-rc/ResourceScriptParser.cpp

127

Actually it looks like IntOrString has a constructor taking a Token, but currently isn't used anywhere and has no implementation. Is this intended and if so you could use this constructor instead of having if block?

[mnbvmar]

llvm/tools/llvm-rc/ResourceScriptParser.cpp

127

No, this was not intended. I remember writing it to support this method and the one below (readTypeOrName). I'll fix it in a moment.

[mnbvmar]

Uses IntOrString(Token) instead of if-s.

[mnbvmar]

Use pipes instead of temporary files.

[rnk]

llvm/tools/llvm-rc/ResourceScriptParser.h

106

I'd recommend ArrayRef<StringRef> here. Then callers of parseFlags don't need to pass NumFlags separately.

[zturner]

Just the below style changes along with rnk@'s suggestion about ArrayRef<StringRef> and this is good.

llvm/tools/llvm-rc/ResourceScriptParser.cpp

206–214

if (!FlagResult->equals_lower(FlagDesc[FlagId]))
  continue;

217–219

How about

std::string ExpectedList = llvm::join(FlagDesc, "/");

[mnbvmar]

Small style fixes.

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

+1 ACCELERATORS {
+  5, 7, ALT, CONTROL, HELLO, WORLD
+}

llvm/test/tools/llvm-rc/Inputs/parser-accelerators-bad-int-or-string.rc

+1 ACCELERATORS {
+  NotIntOrString, 7
+}

llvm/test/tools/llvm-rc/Inputs/parser-accelerators-no-comma-2.rc

+1 ACCELERATORS {
+  "^C" 10
+}

llvm/test/tools/llvm-rc/Inputs/parser-accelerators-no-comma.rc

+1 ACCELERATORS {
+  5, 10, ASCII CONTROL
+}

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

 {
   1 "hello"
   2 "world"
 }
 STRINGTABLE BEGIN END
 
 500 HTML "index.html"
 Name Cursor "hello.ico"
+
+12 ACCELERATORS
+VERSION 5000
+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
+}

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

 ; RUN: llvm-rc /V %p/Inputs/parser-correct-everything.rc | FileCheck %s --check-prefix PGOOD
 
 ; PGOOD:  Icon (meh): "hello.bmp"
 ; PGOOD-NEXT:  Icon (Icon): "Icon"
 ; PGOOD-NEXT:  Language: 5, Sublanguage: 12
 ; PGOOD-NEXT:  StringTable:
 ; PGOOD-NEXT:    Option: Language: 1, Sublanguage: 1
 ; PGOOD-NEXT:    Option: Characteristics: 500
 ; PGOOD-NEXT:    Option: Language: 3, Sublanguage: 4
 ; PGOOD-NEXT:    Option: Version: 14
 ; PGOOD-NEXT:    1 => "hello"
 ; PGOOD-NEXT:    2 => "world"
 ; PGOOD-NEXT:  StringTable:
 ; PGOOD-NEXT:  HTML (500): "index.html"
 ; PGOOD-NEXT:  Cursor (Name): "hello.ico"
+; PGOOD-NEXT:  Accelerators (12):
+; PGOOD-NEXT:    Option: Version: 5000
+; 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
 
 
 ; RUN: not llvm-rc /V %p/Inputs/parser-stringtable-no-string.rc 2>&1 | FileCheck %s --check-prefix PSTRINGTABLE1
 
 ; PSTRINGTABLE1:  llvm-rc: Error parsing file: expected string, got }
 
 
 ; RUN: not llvm-rc /V %p/Inputs/parser-stringtable-weird-option.rc 2>&1 | FileCheck %s --check-prefix PSTRINGTABLE2

 ; RUN: not llvm-rc /V %p/Inputs/parser-html-bad-string.rc 2>&1 | FileCheck %s --check-prefix PHTML1
 
 ; PHTML1:  llvm-rc: Error parsing file: expected string, got ThisPassesInTheOriginalToolButDocSaysItShouldBeQuoted
 
 
 ; RUN: not llvm-rc /V %p/Inputs/parser-html-extra-comma.rc 2>&1 | FileCheck %s --check-prefix PHTML2
 
 ; PHTML2:  llvm-rc: Error parsing file: expected string, got ,
+
+
+; RUN: not llvm-rc /V %p/Inputs/parser-accelerators-bad-flag.rc 2>&1 | FileCheck %s --check-prefix PACCELERATORS1
+
+; PACCELERATORS1:  llvm-rc: Error parsing file: expected ASCII/VIRTKEY/NOINVERT/ALT/SHIFT/CONTROL, got HELLO
+
+
+; RUN: not llvm-rc /V %p/Inputs/parser-accelerators-bad-int-or-string.rc 2>&1 | FileCheck %s --check-prefix PACCELERATORS2
+
+; PACCELERATORS2:  llvm-rc: Error parsing file: expected int or string, got NotIntOrString
+
+
+; RUN: not llvm-rc /V %p/Inputs/parser-accelerators-no-comma.rc 2>&1 | FileCheck %s --check-prefix PACCELERATORS3
+
+; PACCELERATORS3:  llvm-rc: Error parsing file: expected int or string, got CONTROL
+
+
+; RUN: not llvm-rc /V %p/Inputs/parser-accelerators-no-comma-2.rc 2>&1 | FileCheck %s --check-prefix PACCELERATORS4
+
+; PACCELERATORS4:  llvm-rc: Error parsing file: expected ',', got 10

llvm/tools/llvm-rc/ResourceScriptParser.h

private:
   const RCToken &look() const;
   // Read the current token and advance the state by one token.
   const RCToken &read();
   // Advance the state by one token, discarding the current token.
   void consume();
 
   // The following methods try to read a single token, check if it has the
   // correct type and then parse it.
-  Expected<uint32_t> readInt();           // Parse an integer.
-  Expected<StringRef> readString();       // Parse a string.
-  Expected<StringRef> readIdentifier();   // Parse an identifier.
-  Expected<IntOrString> readTypeOrName(); // Parse an integer or an identifier.
+  Expected<uint32_t> readInt();            // Parse an integer.
+  Expected<StringRef> readString();        // Parse a string.
+  Expected<StringRef> readIdentifier();    // Parse an identifier.
+  Expected<IntOrString> readIntOrString(); // Parse an integer or a string.
+  Expected<IntOrString> readTypeOrName();  // Parse an integer or an identifier.
 
   // Advance the state by one, discarding the current token.
   // If the discarded token had an incorrect type, fail.
   Error consumeType(Kind TokenKind);
 
   // Check the current token type. If it's TokenKind, discard it.
   // Return true if the parser consumed this token successfully.
   bool consumeOptionalType(Kind TokenKind);
 
   // Read at least MinCount, and at most MaxCount integers separated by
   // commas. The parser stops reading after fetching MaxCount integers
   // or after an error occurs. Whenever the parser reads a comma, it
   // expects an integer to follow.
   Expected<SmallVector<uint32_t, 8>> readIntsWithCommas(size_t MinCount,
                                                         size_t MaxCount);
 
+  // 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.
+  // 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);
+
   // 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
   // before the main block with the contents of the resource.
   // Usually, resources use a basic set of optional statements:
   //    CHARACTERISTICS, LANGUAGE, VERSION
   // However, DIALOG and DIALOGEX extend this list by the following items:
   //    CAPTION, CLASS, EXSTYLE, FONT, MENU, STYLE
   // UseExtendedStatements flag (off by default) allows the parser to read
   // the additional types of statements.
   //
   // Ref (to the list of all optional statements):
   //    msdn.microsoft.com/en-us/library/windows/desktop/aa381002(v=vs.85).aspx
   Expected<OptionalStmtList>
   parseOptionalStatements(bool UseExtendedStatements = false);
 
   // Read a single optional statement.
   Expected<std::unique_ptr<OptionalStmt>>
   parseSingleOptionalStatement(bool UseExtendedStatements = false);
 
   // Top-level resource parsers.
   ParseType parseLanguageResource();
+  ParseType parseAcceleratorsResource();
   ParseType parseCursorResource();
   ParseType parseIconResource();
   ParseType parseHTMLResource();
   ParseType parseStringTableResource();
 
   // Optional statement parsers.
   ParseOptionType parseLanguageStmt();
   ParseOptionType parseCharacteristicsStmt();

llvm/tools/llvm-rc/ResourceScriptParser.cpp

RCParser::ParseType RCParser::parseSingleResource() {
 
   // If it's not an unnamed resource, what we've just read is a name. Now,
   // read resource type;
   ASSIGN_OR_RETURN(TypeToken, readTypeOrName());
 
   ParseType Result = std::unique_ptr<RCResource>();
   (void)!Result;
 
-  if (TypeToken->equalsLower("CURSOR"))
+  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
     return getExpectedError("resource type", /* IsAlreadyRead = */ true);
 

 }
 
 Expected<StringRef> RCParser::readIdentifier() {
   if (!isNextTokenKind(Kind::Identifier))
     return getExpectedError("identifier");
   return read().value();
 }
 
+Expected<IntOrString> RCParser::readIntOrString() {
+  if (!isNextTokenKind(Kind::Int) && !isNextTokenKind(Kind::String))
+    return getExpectedError("int or string");
+  return IntOrString(read());
+}
+
 Expected<IntOrString> RCParser::readTypeOrName() {
   // We suggest that the correct resource name or type should be either an
   // identifier or an integer. The original RC tool is much more liberal.
   if (!isNextTokenKind(Kind::Identifier) && !isNextTokenKind(Kind::Int))
     return getExpectedError("int or identifier");
-
-  const RCToken &Tok = read();
-  if (Tok.kind() == Kind::Int)
-    return IntOrString(Tok.intValue());
-  else
-    return IntOrString(Tok.value());
+  return IntOrString(read());
 }
 
 Error RCParser::consumeType(Kind TokenKind) {
   if (isNextTokenKind(TokenKind)) {
     consume();
     return Error::success();
   }
 
if (auto IntResult = readInt())
       Result.push_back(*IntResult);
     else
       return FailureHandler(IntResult.takeError());
   }
 
   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.");
+  assert(!FlagDesc.empty());
+
+  uint32_t Result = 0;
+  while (isNextTokenKind(Kind::Comma)) {
+    consume();
+    ASSIGN_OR_RETURN(FlagResult, readIdentifier());
+    bool FoundFlag = false;
+
+    for (size_t FlagId = 0; FlagId < FlagDesc.size(); ++FlagId) {
+      if (!FlagResult->equals_lower(FlagDesc[FlagId]))
+        continue;
+
+      Result |= (1U << FlagId);
+      FoundFlag = true;
+      break;
+    }
+
+    if (!FoundFlag)
+      return getExpectedError(join(FlagDesc, "/"), true);
+  }
+
+  return Result;
+}
+
 // As for now, we ignore the extended set of statements.
 Expected<OptionalStmtList> RCParser::parseOptionalStatements(bool IsExtended) {
   OptionalStmtList Result;
 
   // The last statement is always followed by the start of the block.
   while (!isNextTokenKind(Kind::BlockBegin)) {
     ASSIGN_OR_RETURN(SingleParse, parseSingleOptionalStatement(IsExtended));
     Result.addStmt(std::move(*SingleParse));

 }
 
 RCParser::ParseType RCParser::parseLanguageResource() {
   // Read LANGUAGE as an optional statement. If it's read correctly, we can
   // upcast it to RCResource.
   return parseLanguageStmt();
 }
 
+RCParser::ParseType RCParser::parseAcceleratorsResource() {
+  ASSIGN_OR_RETURN(OptStatements, parseOptionalStatements());
+  RETURN_IF_ERROR(consumeType(Kind::BlockBegin));
+
+  auto Accels = make_unique<AcceleratorsResource>(std::move(*OptStatements));
+
+  while (!consumeOptionalType(Kind::BlockEnd)) {
+    ASSIGN_OR_RETURN(EventResult, readIntOrString());
+    RETURN_IF_ERROR(consumeType(Kind::Comma));
+    ASSIGN_OR_RETURN(IDResult, readInt());
+    ASSIGN_OR_RETURN(FlagsResult,
+                     parseFlags(AcceleratorsResource::Accelerator::OptionsStr));
+    Accels->addAccelerator(*EventResult, *IDResult, *FlagsResult);
+  }
+
+  return std::move(Accels);
+}
+
 RCParser::ParseType RCParser::parseCursorResource() {
   ASSIGN_OR_RETURN(Arg, readString());
   return make_unique<CursorResource>(*Arg);
 }
 
 RCParser::ParseType RCParser::parseIconResource() {
   ASSIGN_OR_RETURN(Arg, readString());
   return make_unique<IconResource>(*Arg);

llvm/tools/llvm-rc/ResourceScriptStmt.h

 // A class holding a name - either an integer or a reference to the string.
 class IntOrString {
 private:
   union Data {
     uint32_t Int;
     StringRef String;
     Data(uint32_t Value) : Int(Value) {}
     Data(const StringRef Value) : String(Value) {}
-    Data(const RCToken &Token);
+    Data(const RCToken &Token) {
+      if (Token.kind() == RCToken::Kind::Int)
+        Int = Token.intValue();
+      else
+        String = Token.value();
+    }
   } Data;
   bool IsInt;
 
 public:
   IntOrString() : IntOrString(0) {}
   IntOrString(uint32_t Value) : Data(Value), IsInt(1) {}
   IntOrString(StringRef Value) : Data(Value), IsInt(0) {}
   IntOrString(const RCToken &Token)
class LanguageResource : public OptionalStmt {
   uint32_t Lang, SubLang;
 
 public:
   LanguageResource(uint32_t LangId, uint32_t SubLangId)
       : Lang(LangId), SubLang(SubLangId) {}
   raw_ostream &log(raw_ostream &) const override;
 };
 
+// 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 {
+public:
+  class Accelerator {
+  public:
+    IntOrString Event;
+    uint32_t Id;
+    uint8_t Flags;
+
+    enum Options {
+      ASCII = (1 << 0),
+      VIRTKEY = (1 << 1),
+      NOINVERT = (1 << 2),
+      ALT = (1 << 3),
+      SHIFT = (1 << 4),
+      CONTROL = (1 << 5)
+    };
+
+    static constexpr size_t NumFlags = 6;
+    static StringRef OptionsStr[NumFlags];
+  };
+
+  AcceleratorsResource(OptionalStmtList &&OptStmts)
+      : OptStatements(std::move(OptStmts)) {}
+  void addAccelerator(IntOrString Event, uint32_t Id, uint8_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.
 //
 // Ref: msdn.microsoft.com/en-us/library/windows/desktop/aa380920(v=vs.85).aspx
 class CursorResource : public RCResource {
   StringRef CursorLoc;
 
 public:
   CursorResource(StringRef Location) : CursorLoc(Location) {}

llvm/tools/llvm-rc/ResourceScriptStmt.cpp

raw_ostream &OptionalStmtList::log(raw_ostream &OS) const {
   }
   return OS;
 }
 
 raw_ostream &LanguageResource::log(raw_ostream &OS) const {
   return OS << "Language: " << Lang << ", Sublanguage: " << SubLang << "\n";
 }
 
+StringRef AcceleratorsResource::Accelerator::OptionsStr
+    [AcceleratorsResource::Accelerator::NumFlags] = {
+        "ASCII", "VIRTKEY", "NOINVERT", "ALT", "SHIFT", "CONTROL"};
+
+raw_ostream &AcceleratorsResource::log(raw_ostream &OS) const {
+  OS << "Accelerators (" << ResName << "): \n";
+  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))
+        OS << " " << Accelerator::OptionsStr[i];
+    OS << "\n";
+  }
+  return OS;
+}
+
 raw_ostream &CursorResource::log(raw_ostream &OS) const {
   return OS << "Cursor (" << ResName << "): " << CursorLoc << "\n";
 }
 
 raw_ostream &IconResource::log(raw_ostream &OS) const {
   return OS << "Icon (" << ResName << "): " << IconLoc << "\n";
 }