[llvm-rc] Have the tokenizer discard comments

[zturner]

Summary

A user reported that llvm-rc did not work correctly when there were comments in the file. This patch aims to fix that.

[rnk]

lgtm, but isn't this the beginning of a C pre-processor? Long term we might want to link this as a library into clang and make clang-cl -c foo.rc produce a .res file.

[amccarth]

llvm/tools/llvm-rc/ResourceScriptToken.cpp

149

Does this happen after line concatenation? If a line ends with a backslash just before the newline, then the "single-line" comment includes the next "line" as well.

llvm/test/tools/llvm-rc/Inputs/tokens.rc

 1 + 2 - 3214L & 0x120894 032173 2|&~+(-7){0xabcdef 0xABCDEFl} Begin End
 He11o LLVM
 
 "RC string test.",L"Another RC string test.'&{",42,100
 
+Block Comment Ident /*block /* // comment */ ifier
 
+Line Comment // Identifier /*
+
+/* Multi line
+   block
+   comment */
+
+Multiple /* comments */ on /* a */ single // line
 
          ":))"

llvm/test/tools/llvm-rc/tokenizer.test

 ; CHECK-NEXT:  Identifier: LLVM
 ; CHECK-NEXT:  String: "RC string test."
 ; CHECK-NEXT:  Comma: ,
 ; CHECK-NEXT:  String: L"Another RC string test.'&{"
 ; CHECK-NEXT:  Comma: ,
 ; CHECK-NEXT:  Int: 42; int value = 42
 ; CHECK-NEXT:  Comma: ,
 ; CHECK-NEXT:  Int: 100; int value = 100
+; CHECK-NEXT:  Identifier: Block
+; CHECK-NEXT:  Identifier: Comment
+; CHECK-NEXT:  Identifier: Ident
+; CHECK-NEXT:  Identifier: ifier
+; CHECK-NEXT:  Identifier: Line
+; CHECK-NEXT:  Identifier: Comment
+; CHECK-NEXT:  Identifier: Multiple
+; CHECK-NEXT:  Identifier: on
+; CHECK-NEXT:  Identifier: single
 ; CHECK-NEXT:  String: ":))"

llvm/tools/llvm-rc/ResourceScriptToken.cpp

private:
   // is hexadecimal). As some kind of separator should come after the
   // integer, we can consume the integer until a non-alphanumeric
   // character.
   bool canStartInt() const;
   bool canContinueInt() const;
 
   bool canStartString() const;
 
+  // Check if tokenizer can start reading a single line comment (e.g. a comment
+  // that begins with '//')
+  bool canStartLineComment() const;
+
+  // Check if tokenizer can start or finish reading a block comment (e.g. a
+  // comment that begins with '/*' and ends with '*/')
+  bool canStartBlockComment() const;
+
+  // Throw away all remaining characters on the current line.
+  void skipCurrentLine();
+
   bool streamEof() const;
 
   // Classify the token that is about to be read from the current position.
   Kind classifyCurrentToken() const;
 
   // Process the Kind::Identifier token - check if it is
   // an identifier describing a block start or end.
   void processIdentifier(RCToken &token) const;
 
   StringRef Data;
   size_t DataLength, Pos;
 };
 
+void Tokenizer::skipCurrentLine() {
+  Pos = Data.find_first_of("\r\n", Pos);
+  Pos = Data.find_first_not_of("\r\n", Pos);
+
+  if (Pos == StringRef::npos)
+    Pos = DataLength;
+}
+
 Expected<std::vector<RCToken>> Tokenizer::run() {
   Pos = 0;
   std::vector<RCToken> Result;
 
   // Consume an optional UTF-8 Byte Order Mark.
   if (willNowRead("\xef\xbb\xbf"))
     advance(3);
 
   while (!streamEof()) {
     if (!skipWhitespaces())
       break;
 
     Kind TokenKind = classifyCurrentToken();
     if (TokenKind == Kind::Invalid)
       return getStringError("Invalid token found at position " + Twine(Pos));
 
     const size_t TokenStart = Pos;
     if (Error TokenError = consumeToken(TokenKind))
       return std::move(TokenError);
 
+    // Comments are just deleted, don't bother saving them.
+    if (TokenKind == Kind::LineComment || TokenKind == Kind::StartComment)
+      continue;
+
     RCToken Token(TokenKind, Data.take_front(Pos).drop_front(TokenStart));
     if (TokenKind == Kind::Identifier) {
       processIdentifier(Token);
     } else if (TokenKind == Kind::Int) {
       uint32_t TokenInt;
       if (!rcGetAsInteger(Token.value(), TokenInt)) {
         // The integer has incorrect format or cannot be represented in
         // a 32-bit integer.

 #define TOKEN(Name)
 #define SHORT_TOKEN(Name, Ch) case Kind::Name:
 #include "ResourceScriptTokenList.h"
 #undef TOKEN
 #undef SHORT_TOKEN
     advance();
     return Error::success();
 
+  case Kind::LineComment:
+    advance(2);
+    skipCurrentLine();
+    return Error::success();
+
+  case Kind::StartComment: {
+    advance(2);
+    auto EndPos = Data.find("*/", Pos);
+    if (EndPos == StringRef::npos)
+      return getStringError(
+          "Unclosed multi-line comment beginning at position " + Twine(Pos));
+    advance(EndPos - Pos);
+    advance(2);
+    return Error::success();
+  }
   case Kind::Identifier:
     while (!streamEof() && canContinueIdentifier())
       advance();
     return Error::success();
 
   case Kind::Int:
     while (!streamEof() && canContinueInt())
       advance();
bool Tokenizer::canContinueIdentifier() const {
   return std::isalnum(CurChar) || CurChar == '_';
 }
 
 bool Tokenizer::canStartInt() const {
   assert(!streamEof());
   return std::isdigit(Data[Pos]);
 }
 
+bool Tokenizer::canStartBlockComment() const {
+  assert(!streamEof());
+  return Data.drop_front(Pos).startswith("/*");
+}
+
+bool Tokenizer::canStartLineComment() const {
+  assert(!streamEof());
+  return Data.drop_front(Pos).startswith("//");
+}
+
 bool Tokenizer::canContinueInt() const {
   assert(!streamEof());
   return std::isalnum(Data[Pos]);
 }
 
 bool Tokenizer::canStartString() const {
   return willNowRead("\"") || willNowRead("L\"") || willNowRead("l\"");
 }
 
 bool Tokenizer::streamEof() const { return Pos == DataLength; }
 
 Kind Tokenizer::classifyCurrentToken() const {
+  if (canStartBlockComment())
+    return Kind::StartComment;
+  if (canStartLineComment())
+    return Kind::LineComment;
+
   if (canStartInt())
     return Kind::Int;
   if (canStartString())
     return Kind::String;
   // BEGIN and END are at this point of lexing recognized as identifiers.
   if (canStartIdentifier())
     return Kind::Identifier;
 

llvm/tools/llvm-rc/ResourceScriptTokenList.h

 //===---------------------------------------------------------------------===//
 
 
 // Long tokens. They might consist of more than one character.
 TOKEN(Invalid)      // Invalid token. Should not occur in a valid script.
 TOKEN(Int)          // Integer (decimal, octal or hexadecimal).
 TOKEN(String)       // String value.
 TOKEN(Identifier)   // Script identifier (resource name or type).
+TOKEN(LineComment)  // Beginning of single-line comment.
+TOKEN(StartComment) // Beginning of multi-line comment.
 
 // Short tokens. They usually consist of exactly one character.
 // The definitions are of the form SHORT_TOKEN(TokenName, TokenChar).
 // TokenChar is the one-character token representation occuring in the correct
 // .rc scripts.
 SHORT_TOKEN(BlockBegin, '{')   // Start of the script block; can also be BEGIN.
 SHORT_TOKEN(BlockEnd, '}')     // End of the block; can also be END.
 SHORT_TOKEN(Comma, ',')        // Comma - resource arguments separator.
 SHORT_TOKEN(Plus, '+')         // Addition operator.
 SHORT_TOKEN(Minus, '-')        // Subtraction operator.
 SHORT_TOKEN(Pipe, '|')         // Bitwise-OR operator.
 SHORT_TOKEN(Amp, '&')          // Bitwise-AND operator.
 SHORT_TOKEN(Tilde, '~')        // Bitwise-NOT operator.
 SHORT_TOKEN(LeftParen, '(')    // Left parenthesis in the script expressions.
 SHORT_TOKEN(RightParen, ')')   // Right parenthesis.