[PATCH] D100755: [llvm-rc] [3/4] Run clang to preprocess input files

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added a comment.

In D100755#2701608 <https://reviews.llvm.org/D100755#2701608>, @aganea wrote:

> Thanks for adding this Martin!
>
> I'd really like to see a simple test running from the Clang side as well! Like calling llvm-rc with a file that requires preprocessing:

That sounds like a good idea, as there's indeed a gap in testing at that spot right now.

What would be the most appropriate place under clang/test for it?

> ---
>
> Also just for reference purposes, I'm pasting the little script I'm currently using to replace rc.exe on Linux. I've put it in the same folder as clang-cl and llvm-rc and now I'm able to use the same cmd-line as for rc.exe (it goes through a temp file which isn't ideal):
>
>   diff --git a/llvm/tools/llvm-rc/rc.sh b/llvm/tools/llvm-rc/rc.sh
>   new file mode 100755
>   index 000000000000..4a16f020aad8
>   --- /dev/null
>   +++ b/llvm/tools/llvm-rc/rc.sh
>   @@ -0,0 +1,34 @@
>   +#!/bin/bash
>   +
>   +INCLUDES=
>   +for flag in "$@" 
>   +do
>   +        if [[ $flag =~ /Fo(.+) ]] ;
>   +	then
>   +		OUTFILE=${BASH_REMATCH[1]}
>   +		OUTFLAG=$flag
>   +	fi
>   +	if [[ $flag =~ .+\.rc ]] ;
>   +	then
>   +		RCFILE=$flag
>   +		RCPATH=$(dirname "${flag}")
>   +	fi
>   +	if [[ $flag =~ /I ]] ;
>   +	then
>   +		INCLUDES="$INCLUDES $flag"
>   +	fi
>   +	if [[ $flag == /nologo ]] ;
>   +	then
>   +		NOLOGO=$flag
>   +	fi
>   +done
>   +if [[ $* =~ (/l[[:space:]]0x[[:alnum:]]+) ]] ;
>   +then
>   +	LANGUAGE="${BASH_REMATCH[1]}"
>   +fi
>   +
>   +SCRIPTPATH=$(dirname "$0")
>   +
>   +$SCRIPTPATH/clang-cl /E ${RCFILE} ${INCLUDES} -nostdinc -Wno-nonportable-include-path > "${OUTFILE}.i"
>   +$SCRIPTPATH/llvm-rc "${OUTFILE}.i" ${OUTFLAG} ${LANGUAGE} ${NOLOGO} /I${RCPATH}
>   +

Can the same things be emulated with the plain `rc.exe` tool too? I guess `-nostdinc` maps to the `/X` option (i.e. ignoring the `INCLUDE` variable), but I'm not sure if there's an official way of passing arbitrary preprocessor args with rc.exe (as rc.exe doesn't launch an out of process preprocessor, but it uses a built-in one). (With the windres interface, there's `--preprocessor-arg` though.)



================
Comment at: llvm/tools/llvm-rc/llvm-rc.cpp:133
+
+  SmallVector<StringRef, 8> Args = {Clang, "-target", PreprocTriple,
+                                    "-E",  "-xc",     "-DRC_INVOKED",
----------------
aganea wrote:
> I'm wondering if you could add `--driver-mode=cl` ? We're using platform includes and/or VC includes in our .res files. How would that work out?
> 
> For example, we're including these:
> ```
> C:\.nuget\dd\vs2019_buildtools.16.9.1\tools\VC\Tools\MSVC\14.28.29910\atlmfc\include\afxres.h
> C:\.nuget\dd\win-sdk.10.19041.1\tools\10\Include\10.0.19041.0\um\winresrc.h
> ```
> In our case they are consumed from nuget packages and thus we're adding `-nostdinc` as well to the clang-cl invocation, before calling llvm-rc with the output. But I'm more concerned about any options or #define(s) that the CL driver defines, that stock clang.exe does not? You wouldn't have to fiddle with the triple if you did that I think.
I did indeed consider calling literally `clang-cl` first, but to reduce differences with D100756, I went with plain clang. Running `clang -target *-windows-msvc` does retain most aspects of the clang-cl specific behaviours (including using the `INCLUDE` env var for picking up include directories) - off hand I don't think of anything in this use case where the distinction between `clang -target *-windows-msvc` and `--driver-mode=cl` matters. But I guess one could add `--driver-mode=cl` if you really want to, it shouldn't make things much harder for D100756.


================
Comment at: llvm/tools/llvm-rc/llvm-rc.cpp:167
+  }
+  // The llvm Support classes don't handle reading from stdout of a child
+  // process; otherwise we could avoid using a temp file.
----------------
aganea wrote:
> Couldn't we fold the child process with `-fintegrated-cc1`? Life would be much better without temp files.
It's not about child processes within clang, it's simply that the LLVM Support library's `Execute*` functions don't support redirecting stdin/stdout/stderr to a pipe and reading it at all.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100755/new/

https://reviews.llvm.org/D100755