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

[Alexandre Ganea via Phabricator via llvm-commits]

aganea added a comment.

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:

  // foo.rc
  #include "resource.h"
  IDI_MY_ICON       ICON           "my.ico"

with

  // resource.h
  #define IDI_MY_ICON               101

---

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}
  +



================
Comment at: llvm/tools/llvm-rc/Opts.td:43
 
-def codepage : JS<"C", "Set the codepage used for input strings.">;
+def no_cpp : F<"no-cpp", "Don't try to preprocess the input file.">;
+
----------------
I know `-no-cpp` is short, but in my mind 'cpp' sounds like C++ not C preprocessor. I'm wondering if `-no-preprocess`  wouldn't be clearer?


================
Comment at: llvm/tools/llvm-rc/llvm-rc.cpp:133
+
+  SmallVector<StringRef, 8> Args = {Clang, "-target", PreprocTriple,
+                                    "-E",  "-xc",     "-DRC_INVOKED",
----------------
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.


================
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.
----------------
Couldn't we fold the child process with `-fintegrated-cc1`? Life would be much better without temp files.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100755