[PATCH] D105621: [llvm-rc] Make commas in user data structs optional

[Adrian McCarthy via Phabricator via llvm-commits]

amccarth added a comment.

Two small requests, then LGTM.



================
Comment at: llvm/test/tools/llvm-rc/Inputs/tag-user.rc:5
 501 RCDATA {
-  1, 2, 3, 4, 5, "data", L"wide data", 0xABCD, 0xABCDEF01L
+  1, 2, 3, 4, 5, "da" "ta" L"wide", L" data", 0xABCD 0xABCDEF01L,
 }
----------------
Could you add an extra comma between a couple tokens here?  The 500/500 example has it, which is a true user-defined "resource type."  This one is an RCDATA which should obey the same rules.  The implementation is shared, so this should be fine, but if RCDATA is ever treated differently, we'd still want this to work.

BTW:  I'm not sure splitting `"data"` into two strings with no comma actually exercises your change.  Since RC (theoretically) works with the output from the preprocessor, I'd expect the preprocessor to concatenate those into one token.  (Not a big deal.)




================
Comment at: llvm/tools/llvm-rc/ResourceScriptParser.cpp:516
+
+    // There can be zero or more commas after each token (but not before
+    // the first one).
----------------
I would have guessed zero-or-one comma, but you're right.  RC.EXE allows any  number of commas.


================
Comment at: llvm/tools/llvm-rc/ResourceScriptParser.cpp:518
+    // the first one).
+    while (consumeOptionalType(Kind::Comma));
   }
----------------
The prevailing style for empty loop bodies in LLVM appears to be empty braces `{}`, which I believe will also satisfy the lint check.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105621/new/

https://reviews.llvm.org/D105621