This improves the current llvm-rc parser by the ability of parsing ACCELERATORS statement.
Details
Details
Diff Detail
Diff Detail
- Repository
- rL LLVM
Event Timeline
llvm/tools/llvm-rc/ResourceScriptParser.cpp | ||
---|---|---|
127 ↗ | (On Diff #111734) | no else after return |
llvm/tools/llvm-rc/ResourceScriptParser.cpp | ||
---|---|---|
127 ↗ | (On Diff #111734) | 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? |
llvm/tools/llvm-rc/ResourceScriptParser.cpp | ||
---|---|---|
127 ↗ | (On Diff #111734) | 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. |
llvm/tools/llvm-rc/ResourceScriptParser.h | ||
---|---|---|
106 ↗ | (On Diff #112442) | I'd recommend ArrayRef<StringRef> here. Then callers of parseFlags don't need to pass NumFlags separately. |
Comment Actions
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 ↗ | (On Diff #112442) | if (!FlagResult->equals_lower(FlagDesc[FlagId])) continue; |
217–219 ↗ | (On Diff #112442) | How about std::string ExpectedList = llvm::join(FlagDesc, "/"); |