This extends llvm-rc parsing tool by MENU resource (msdn.microsoft.com/en-us/library/windows/desktop/aa381025(v=vs.85).aspx). As for now, MENUEX (msdn.microsoft.com/en-us/library/windows/desktop/aa381023(v=vs.85).aspx) seems unnecessary.
Details
Details
Diff Detail
Diff Detail
- Repository
- rL LLVM
Event Timeline
Comment Actions
Small bugfixes (compilation problems on gcc and missing newlines at the end of the files).
llvm/tools/llvm-rc/ResourceScriptParser.cpp | ||
---|---|---|
316 ↗ | (On Diff #111751) | I've never seen this before. Is there a reason to using a comma instead of just putting as a separate statement? |
320 ↗ | (On Diff #111751) | do the early continue for better readability, and remove the else block. i.e. while (....) { ...... if (IsMenuItem && isNextTokenKind(Kind::Identifier)) { ..... continue; } ASSIGN_OR_RETURN(CaptionResult, readString()); ....... } |
323 ↗ | (On Diff #111751) | doesn't ASSIGN_OR_RETURN go here? |
344 ↗ | (On Diff #111751) | can use same early continue pattern here. |
393 ↗ | (On Diff #111751) | Sorry this isn't part of this revision, but this should have a better name, like just "createError()" |
llvm/tools/llvm-rc/ResourceScriptParser.cpp | ||
---|---|---|
393 ↗ | (On Diff #111751) | OK, might be. As this name is scattered around the file, I suggest that I refactor it as soon as all the parser parts are accepted. |
Comment Actions
Can you rebase this and the other patches and I'll take another look?
llvm/test/tools/llvm-rc/parser.test | ||
---|---|---|
120 ↗ | (On Diff #112223) | The same comment re: pipes applies here. |
llvm/tools/llvm-rc/ResourceScriptParser.cpp | ||
---|---|---|
343 ↗ | (On Diff #112444) | ArrayRef will help here too. |