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

[Nico Weber via Phabricator via llvm-commits]

thakis added inline comments.


================
Comment at: llvm/tools/llvm-rc/llvm-rc.cpp:154
+      Args.push_back("-U");
+      break;
+    }
----------------
mstorsjo wrote:
> thakis wrote:
> > Here's our chromium wrapper: https://source.chromium.org/chromium/chromium/src/+/master:build/toolchain/win/rc/rc.py
> > 
> > On Windows, /winsysroot support and possibly -imsvc support would be nice too.
> Those sound useful - but I think I'd prefer to defer adding support for more nonstandard preprocessing options to a later patch.
> 
> What do you think of a generic `--preprocessor-arg` like in the windres frontend, which might be useful for @aganea?
We don't need a general `--preprocessor-arg` as long as common args work.

Oh, on that note: https://source.chromium.org/chromium/chromium/src/+/master:build/toolchain/win/rc/rc.py also has `/showIncludes` support which is necessary to make ninja re-build .rc files if either

* an included .h file is changed (needs preprocessor output)
* an included resource file (.ico or similar) is changed (needs llvm-rc support)

Don't know if llvm-rc has support for the latter part. If it doesn't, that'd be nice to add, and some test that checks that both parts work would be nice. (Neither in this patch; just thinking about what we'd need to switch.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100755