[PATCH] D46507: [llvm-rc] Add support for all missing dialog controls

[Adrian McCarthy via Phabricator via llvm-commits]

amccarth added inline comments.


================
Comment at: tools/llvm-rc/ResourceScriptParser.cpp:484
+    RETURN_IF_ERROR(consumeType(Kind::Comma));
+    ASSIGN_OR_RETURN(Args, readIntsWithCommas(5, 7));
+
----------------
This is OK for now, but in the future I think you'll need special code for `style` and `exstyle` because those are really expressions that don't work exactly like regular C-style bitwise operations.

So I'm not sure how much `readIntsWithCommas` is buying versus reading the arguments into well-named variables like `X`, `Y`, `Width`, `Height`, etc., which would then make reading the Control c'tor calls a lot easier and would highlight the difference between the `CONTROL` syntax versus the syntax for the pre-defined control types.

About a third of this function is special case handling for `CONTROL` when the only differences are two little tweaks to the syntax (position of `style` and the explicit `class` parameter).  It feels like a lot of near-duplicate code (e.g., the TakeOptArg lambda has to be defined twice) to handle the tweaks.


https://reviews.llvm.org/D46507