mstorsjo marked an inline comment as done.
mstorsjo added inline comments.
================
Comment at: llvm/tools/llvm-rc/llvm-rc.cpp:300
+
+std::string unescape(StringRef S) {
+ std::string Out;
----------------
aganea wrote:
> 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)
Hmm, right. In any case - for that patch, I guess one would have to specify
which unescaping one expects to do.
This one here is (currently) meant to be unix shell unescaping, but it could
also be unix/windows shell unescaping based on what platform it runs on.
================
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: " +
----------------
aganea wrote:
> 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.
Ah, I see. I guess that's just good form to be explicit as user, as it indeed
is quite ambiguous otherwise.
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
