aganea added inline comments.
================ Comment at: llvm/tools/llvm-rc/llvm-rc.cpp:300 + +std::string unescape(StringRef S) { + std::string Out; ---------------- mstorsjo wrote: > aganea wrote: > > I would also need this function in D43002 (see unescapeSlashes), do you > > think we can move it to `sys::path` or any other "support" lib? > This does things differently than your function - this converts `\\\"` into > `\"` while your version leaves `\\"` if I read it correctly. > > (I'm also considering a different, platform specific unescaping strategy - > that'd be closer to how GNU windres behaves, but makes for a more > inconsistent tool. Keeping the logic here makes it easier to tweak if needed.) I think `unescapeSlashes` in D43002 was only a quick hack to allow copy-pasting a command-line file name from LIT outputs into the VS debugger options dialog. If the file name contains double quotes I would expect it to be converted as you do, but double quotes cannot happen in file names on Windows. In theory one could use the `S_OBJNAME` and `LF_BUILDINFO` records to repro a clang-cl build command on Linux, but that seems improbable? (since the feature is meant to be used on Windows) ================ Comment at: llvm/tools/llvm-rc/llvm-rc.cpp:567 + if (InputArgs.hasArg(OPT_lang_id)) { + if (InputArgs.getLastArgValue(OPT_lang_id).getAsInteger(16, Opts.LangId)) + fatalError("Invalid language id: " + ---------------- mstorsjo wrote: > aganea wrote: > > There was a latent issue here - unrelated to the moving of code around - I > > don't know if you want to fix it now or after? That is, `s/16/0/`. > Hmm, as far as I can see, both rc.exe and GNU windres interpret values as > hexadecimal - if I run e.g. `rc.exe -l 40 test.rc`, where `40` is ambiguous, > I get language id 64. Ok, we only have stuff like `rc.exe /l 0x0409` in our build scripts, the 0x in front isn't consumed, I though it was mandatory. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100756/new/ https://reviews.llvm.org/D100756 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits