[PATCH] D100756: [llvm-rc] [4/4] Add a GNU windres-like frontend to llvm-rc

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D100756#3088035 , @nickdesaulniers 
wrote:

> thanks for this patch @mstorsjo ! This is help AOSP LLVM move our windows 
> builds of LLVM away from MinGW to LLVM! 
> https://android-review.googlesource.com/c/toolchain/llvm_android/+/1867380/

Thanks for letting me know - that’s nice to hear that it benefits you too!

Btw, a little nitpick about naming - MinGW is the SDK/environment that you’re 
still using, but you moved from GNU binutils windres to llvm windres - it’s 
just as little/much mingw as before. :P


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

[PATCH] D100756: [llvm-rc] [4/4] Add a GNU windres-like frontend to llvm-rc

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

thanks for this patch @mstorsjo ! This is help AOSP LLVM move our windows 
builds of LLVM away from MinGW to LLVM! 
https://android-review.googlesource.com/c/toolchain/llvm_android/+/1867380/


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

[PATCH] D100756: [llvm-rc] [4/4] Add a GNU windres-like frontend to llvm-rc

[Martin Storsjö via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf8de9aaef2f4: [llvm-rc] Add a GNU windres-like frontend to 
llvm-rc (authored by mstorsjo).

Changed prior to commit:
  https://reviews.llvm.org/D100756?vs=339263=340607#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100756

Files:
  clang/test/CMakeLists.txt
  clang/test/Preprocessor/Inputs/llvm-windres.h
  clang/test/Preprocessor/llvm-windres.rc
  llvm/test/CMakeLists.txt
  llvm/test/lit.cfg.py
  llvm/test/tools/llvm-rc/codepage.test
  llvm/test/tools/llvm-rc/language.test
  llvm/test/tools/llvm-rc/windres-format.test
  llvm/test/tools/llvm-rc/windres-prefix.test
  llvm/test/tools/llvm-rc/windres-preproc.test
  llvm/test/tools/llvm-rc/windres-target.test
  llvm/test/tools/llvm-rc/windres-version.test
  llvm/tools/llvm-rc/CMakeLists.txt
  llvm/tools/llvm-rc/WindresOpts.td
  llvm/tools/llvm-rc/llvm-rc.cpp

Index: llvm/tools/llvm-rc/llvm-rc.cpp
===
--- llvm/tools/llvm-rc/llvm-rc.cpp
+++ llvm/tools/llvm-rc/llvm-rc.cpp
@@ -18,8 +18,10 @@
 #include "ResourceScriptToken.h"
 
 #include "llvm/ADT/Triple.h"
+#include "llvm/Object/WindowsResource.h"
 #include "llvm/Option/Arg.h"
 #include "llvm/Option/ArgList.h"
+#include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/FileUtilities.h"
@@ -75,8 +77,39 @@
   RcOptTable() : OptTable(InfoTable, /* IgnoreCase = */ true) {}
 };
 
+enum Windres_ID {
+  WINDRES_INVALID = 0, // This is not a correct option ID.
+#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
+   HELPTEXT, METAVAR, VALUES)  \
+  WINDRES_##ID,
+#include "WindresOpts.inc"
+#undef OPTION
+};
+
+#define PREFIX(NAME, VALUE) const char *const WINDRES_##NAME[] = VALUE;
+#include "WindresOpts.inc"
+#undef PREFIX
+
+static const opt::OptTable::Info WindresInfoTable[] = {
+#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
+   HELPTEXT, METAVAR, VALUES)  \
+  {\
+  WINDRES_##PREFIX, NAME, HELPTEXT,\
+  METAVAR,  WINDRES_##ID, opt::Option::KIND##Class,\
+  PARAM,FLAGS,WINDRES_##GROUP, \
+  WINDRES_##ALIAS,  ALIASARGS,VALUES},
+#include "WindresOpts.inc"
+#undef OPTION
+};
+
+class WindresOptTable : public opt::OptTable {
+public:
+  WindresOptTable() : OptTable(WindresInfoTable, /* IgnoreCase = */ false) {}
+};
+
 static ExitOnError ExitOnErr;
 static FileRemover TempPreprocFile;
+static FileRemover TempResFile;
 
 LLVM_ATTRIBUTE_NORETURN static void fatalError(const Twine ) {
   errs() << Message << "\n";
@@ -112,9 +145,8 @@
   return Path;
 }
 
-std::string getClangClTriple() {
-  Triple T(sys::getDefaultTargetTriple());
-  switch (T.getArch()) {
+Triple::ArchType getDefaultArch(Triple::ArchType Arch) {
+  switch (Arch) {
   case Triple::x86:
   case Triple::x86_64:
   case Triple::arm:
@@ -122,13 +154,17 @@
   case Triple::aarch64:
 // These work properly with the clang driver, setting the expected
 // defines such as _WIN32 etc.
-break;
+return Arch;
   default:
 // Other archs aren't set up for use with windows as target OS, (clang
 // doesn't define e.g. _WIN32 etc), so set a reasonable default arch.
-T.setArch(Triple::x86_64);
-break;
+return Triple::x86_64;
   }
+}
+
+std::string getClangClTriple() {
+  Triple T(sys::getDefaultTargetTriple());
+  T.setArch(getDefaultArch(T.getArch()));
   T.setOS(Triple::Win32);
   T.setVendor(Triple::PC);
   T.setEnvironment(Triple::MSVC);
@@ -136,10 +172,44 @@
   return T.str();
 }
 
-bool preprocess(StringRef Src, StringRef Dst, opt::InputArgList ,
+std::string getMingwTriple() {
+  Triple T(sys::getDefaultTargetTriple());
+  T.setArch(getDefaultArch(T.getArch()));
+  if (T.isWindowsGNUEnvironment())
+return T.str();
+  // Write out the literal form of the vendor/env here, instead of
+  // constructing them with enum values (which end up with them in
+  // normalized form). The literal form of the triple can matter for
+  // finding include files.
+  return (Twine(T.getArchName()) + "-w64-mingw32").str();
+}
+
+enum Format { Rc, Res, Coff, Unknown };
+
+struct RcOptions {
+  bool Preprocess = true;
+  bool PrintCmdAndExit = false;
+  std::string Triple;
+  std::vector PreprocessCmd;
+  std::vector PreprocessArgs;
+
+  std::string InputFile;
+  Format InputFormat = Rc;
+  std::string OutputFile;
+  Format OutputFormat = Res;
+
+  bool BeVerbose = false;
+  WriterParams Params;
+  bool AppendNull = false;
+  

[PATCH] D100756: [llvm-rc] [4/4] Add a GNU windres-like frontend to llvm-rc

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D100756#2717331 , @amccarth wrote:

> There's a lot going on here, but I don't see anything wrong.  Thanks for the 
> completeness of the tests and the comments, as that helps a lot in 
> understanding what's going on here.

Thanks!

With this in the tree, it'll be easier to reason about further requests to the 
preprocessing, when both use cases are available at once.




Comment at: llvm/tools/llvm-rc/llvm-rc.cpp:295
+ .Case(".o", Coff)
+ .Default(Unknown);
+  if (F != Unknown)

amccarth wrote:
> ".obj"?
Good point, I'll add that too.


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

[PATCH] D100756: [llvm-rc] [4/4] Add a GNU windres-like frontend to llvm-rc

[Adrian McCarthy via Phabricator via cfe-commits]

amccarth accepted this revision.
amccarth added a comment.
This revision is now accepted and ready to land.

There's a lot going on here, but I don't see anything wrong.  Thanks for the 
completeness of the tests and the comments, as that helps a lot in 
understanding what's going on here.




Comment at: llvm/tools/llvm-rc/llvm-rc.cpp:295
+ .Case(".o", Coff)
+ .Default(Unknown);
+  if (F != Unknown)

".obj"?


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

[PATCH] D100756: [llvm-rc] [4/4] Add a GNU windres-like frontend to llvm-rc

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo marked an inline comment as done.
mstorsjo added a comment.

Any further comments on this one, or is tolerable in this form? I'd prefer to 
not move the unescaping to shared code for now (as the exact definition of what 
it should do is a bit open).


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

[PATCH] D100756: [llvm-rc] [4/4] Add a GNU windres-like frontend to llvm-rc

[Martin Storsjö via Phabricator via cfe-commits]

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

[PATCH] D100756: [llvm-rc] [4/4] Add a GNU windres-like frontend to llvm-rc

[Alexandre Ganea via Phabricator via cfe-commits]

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

[PATCH] D100756: [llvm-rc] [4/4] Add a GNU windres-like frontend to llvm-rc

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo updated this revision to Diff 339263.
mstorsjo added a comment.

Updated the capitalization of one variable


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100756

Files:
  clang/test/CMakeLists.txt
  clang/test/Preprocessor/Inputs/llvm-windres.h
  clang/test/Preprocessor/llvm-windres.rc
  llvm/test/CMakeLists.txt
  llvm/test/lit.cfg.py
  llvm/test/tools/llvm-rc/codepage.test
  llvm/test/tools/llvm-rc/language.test
  llvm/test/tools/llvm-rc/windres-format.test
  llvm/test/tools/llvm-rc/windres-prefix.test
  llvm/test/tools/llvm-rc/windres-preproc.test
  llvm/test/tools/llvm-rc/windres-target.test
  llvm/test/tools/llvm-rc/windres-version.test
  llvm/tools/llvm-rc/CMakeLists.txt
  llvm/tools/llvm-rc/WindresOpts.td
  llvm/tools/llvm-rc/llvm-rc.cpp

Index: llvm/tools/llvm-rc/llvm-rc.cpp
===
--- llvm/tools/llvm-rc/llvm-rc.cpp
+++ llvm/tools/llvm-rc/llvm-rc.cpp
@@ -18,8 +18,10 @@
 #include "ResourceScriptToken.h"
 
 #include "llvm/ADT/Triple.h"
+#include "llvm/Object/WindowsResource.h"
 #include "llvm/Option/Arg.h"
 #include "llvm/Option/ArgList.h"
+#include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/FileUtilities.h"
@@ -75,8 +77,39 @@
   RcOptTable() : OptTable(InfoTable, /* IgnoreCase = */ true) {}
 };
 
+enum Windres_ID {
+  WINDRES_INVALID = 0, // This is not a correct option ID.
+#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
+   HELPTEXT, METAVAR, VALUES)  \
+  WINDRES_##ID,
+#include "WindresOpts.inc"
+#undef OPTION
+};
+
+#define PREFIX(NAME, VALUE) const char *const WINDRES_##NAME[] = VALUE;
+#include "WindresOpts.inc"
+#undef PREFIX
+
+static const opt::OptTable::Info WindresInfoTable[] = {
+#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
+   HELPTEXT, METAVAR, VALUES)  \
+  {\
+  WINDRES_##PREFIX, NAME, HELPTEXT,\
+  METAVAR,  WINDRES_##ID, opt::Option::KIND##Class,\
+  PARAM,FLAGS,WINDRES_##GROUP, \
+  WINDRES_##ALIAS,  ALIASARGS,VALUES},
+#include "WindresOpts.inc"
+#undef OPTION
+};
+
+class WindresOptTable : public opt::OptTable {
+public:
+  WindresOptTable() : OptTable(WindresInfoTable, /* IgnoreCase = */ false) {}
+};
+
 static ExitOnError ExitOnErr;
 static FileRemover TempPreprocFile;
+static FileRemover TempResFile;
 
 LLVM_ATTRIBUTE_NORETURN static void fatalError(const Twine ) {
   errs() << Message << "\n";
@@ -112,9 +145,8 @@
   return Path;
 }
 
-std::string getClangClTriple() {
-  Triple T(sys::getDefaultTargetTriple());
-  switch (T.getArch()) {
+Triple::ArchType getDefaultArch(Triple::ArchType Arch) {
+  switch (Arch) {
   case Triple::x86:
   case Triple::x86_64:
   case Triple::arm:
@@ -122,13 +154,17 @@
   case Triple::aarch64:
 // These work properly with the clang driver, setting the expected
 // defines such as _WIN32 etc.
-break;
+return Arch;
   default:
 // Other archs aren't set up for use with windows as target OS, (clang
 // doesn't define e.g. _WIN32 etc), so set a reasonable default arch.
-T.setArch(Triple::x86_64);
-break;
+return Triple::x86_64;
   }
+}
+
+std::string getClangClTriple() {
+  Triple T(sys::getDefaultTargetTriple());
+  T.setArch(getDefaultArch(T.getArch()));
   T.setOS(Triple::Win32);
   T.setVendor(Triple::PC);
   T.setEnvironment(Triple::MSVC);
@@ -136,10 +172,44 @@
   return T.str();
 }
 
-bool preprocess(StringRef Src, StringRef Dst, opt::InputArgList ,
+std::string getMingwTriple() {
+  Triple T(sys::getDefaultTargetTriple());
+  T.setArch(getDefaultArch(T.getArch()));
+  if (T.isWindowsGNUEnvironment())
+return T.str();
+  // Write out the literal form of the vendor/env here, instead of
+  // constructing them with enum values (which end up with them in
+  // normalized form). The literal form of the triple can matter for
+  // finding include files.
+  return (Twine(T.getArchName()) + "-w64-mingw32").str();
+}
+
+enum Format { Rc, Res, Coff, Unknown };
+
+struct RcOptions {
+  bool Preprocess = true;
+  bool PrintCmdAndExit = false;
+  std::string Triple;
+  std::vector PreprocessCmd;
+  std::vector PreprocessArgs;
+
+  std::string InputFile;
+  Format InputFormat = Rc;
+  std::string OutputFile;
+  Format OutputFormat = Res;
+
+  bool BeVerbose = false;
+  WriterParams Params;
+  bool AppendNull = false;
+  bool IsDryRun = false;
+  // Set the default language; choose en-US arbitrarily.
+  unsigned LangId = (/*PrimaryLangId*/ 0x09) | (/*SubLangId*/ 0x01 << 10);
+};
+
+bool preprocess(StringRef Src, StringRef 

[PATCH] D100756: [llvm-rc] [4/4] Add a GNU windres-like frontend to llvm-rc

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added inline comments.



Comment at: llvm/tools/llvm-rc/llvm-rc.cpp:259
 
-} // anonymous namespace
+static bool consume_back_lower(StringRef , const char *str) {
+  if (!S.endswith_lower(str))

aganea wrote:
> `s/str/Str/`
Thanks, will fix.



Comment at: llvm/tools/llvm-rc/llvm-rc.cpp:300
+
+std::string unescape(StringRef S) {
+  std::string Out;

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.)



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:
> 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.


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

[PATCH] D100756: [llvm-rc] [4/4] Add a GNU windres-like frontend to llvm-rc

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added inline comments.



Comment at: llvm/tools/llvm-rc/llvm-rc.cpp:259
 
-} // anonymous namespace
+static bool consume_back_lower(StringRef , const char *str) {
+  if (!S.endswith_lower(str))

`s/str/Str/`



Comment at: llvm/tools/llvm-rc/llvm-rc.cpp:300
+
+std::string unescape(StringRef S) {
+  std::string Out;

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?



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: " +

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/`.


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

[PATCH] D100756: [llvm-rc] [4/4] Add a GNU windres-like frontend to llvm-rc

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo updated this revision to Diff 339158.
mstorsjo added a comment.

Rebased, updated with fixes making the full preprocessing test pass on windows, 
including fixes for the default arch when running on arches that aren't 
normally supported as windows targets.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100756

Files:
  clang/test/CMakeLists.txt
  clang/test/Preprocessor/Inputs/llvm-windres.h
  clang/test/Preprocessor/llvm-windres.rc
  llvm/test/CMakeLists.txt
  llvm/test/lit.cfg.py
  llvm/test/tools/llvm-rc/codepage.test
  llvm/test/tools/llvm-rc/language.test
  llvm/test/tools/llvm-rc/windres-format.test
  llvm/test/tools/llvm-rc/windres-prefix.test
  llvm/test/tools/llvm-rc/windres-preproc.test
  llvm/test/tools/llvm-rc/windres-target.test
  llvm/test/tools/llvm-rc/windres-version.test
  llvm/tools/llvm-rc/CMakeLists.txt
  llvm/tools/llvm-rc/WindresOpts.td
  llvm/tools/llvm-rc/llvm-rc.cpp

Index: llvm/tools/llvm-rc/llvm-rc.cpp
===
--- llvm/tools/llvm-rc/llvm-rc.cpp
+++ llvm/tools/llvm-rc/llvm-rc.cpp
@@ -18,8 +18,10 @@
 #include "ResourceScriptToken.h"
 
 #include "llvm/ADT/Triple.h"
+#include "llvm/Object/WindowsResource.h"
 #include "llvm/Option/Arg.h"
 #include "llvm/Option/ArgList.h"
+#include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/FileUtilities.h"
@@ -75,8 +77,39 @@
   RcOptTable() : OptTable(InfoTable, /* IgnoreCase = */ true) {}
 };
 
+enum Windres_ID {
+  WINDRES_INVALID = 0, // This is not a correct option ID.
+#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
+   HELPTEXT, METAVAR, VALUES)  \
+  WINDRES_##ID,
+#include "WindresOpts.inc"
+#undef OPTION
+};
+
+#define PREFIX(NAME, VALUE) const char *const WINDRES_##NAME[] = VALUE;
+#include "WindresOpts.inc"
+#undef PREFIX
+
+static const opt::OptTable::Info WindresInfoTable[] = {
+#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
+   HELPTEXT, METAVAR, VALUES)  \
+  {\
+  WINDRES_##PREFIX, NAME, HELPTEXT,\
+  METAVAR,  WINDRES_##ID, opt::Option::KIND##Class,\
+  PARAM,FLAGS,WINDRES_##GROUP, \
+  WINDRES_##ALIAS,  ALIASARGS,VALUES},
+#include "WindresOpts.inc"
+#undef OPTION
+};
+
+class WindresOptTable : public opt::OptTable {
+public:
+  WindresOptTable() : OptTable(WindresInfoTable, /* IgnoreCase = */ false) {}
+};
+
 static ExitOnError ExitOnErr;
 static FileRemover TempPreprocFile;
+static FileRemover TempResFile;
 
 LLVM_ATTRIBUTE_NORETURN static void fatalError(const Twine ) {
   errs() << Message << "\n";
@@ -112,9 +145,8 @@
   return Path;
 }
 
-std::string getClangClTriple() {
-  Triple T(sys::getDefaultTargetTriple());
-  switch (T.getArch()) {
+Triple::ArchType getDefaultArch(Triple::ArchType Arch) {
+  switch (Arch) {
   case Triple::x86:
   case Triple::x86_64:
   case Triple::arm:
@@ -122,13 +154,17 @@
   case Triple::aarch64:
 // These work properly with the clang driver, setting the expected
 // defines such as _WIN32 etc.
-break;
+return Arch;
   default:
 // Other archs aren't set up for use with windows as target OS, (clang
 // doesn't define e.g. _WIN32 etc), so set a reasonable default arch.
-T.setArch(Triple::x86_64);
-break;
+return Triple::x86_64;
   }
+}
+
+std::string getClangClTriple() {
+  Triple T(sys::getDefaultTargetTriple());
+  T.setArch(getDefaultArch(T.getArch()));
   T.setOS(Triple::Win32);
   T.setVendor(Triple::PC);
   T.setEnvironment(Triple::MSVC);
@@ -136,10 +172,44 @@
   return T.str();
 }
 
-bool preprocess(StringRef Src, StringRef Dst, opt::InputArgList ,
+std::string getMingwTriple() {
+  Triple T(sys::getDefaultTargetTriple());
+  T.setArch(getDefaultArch(T.getArch()));
+  if (T.isWindowsGNUEnvironment())
+return T.str();
+  // Write out the literal form of the vendor/env here, instead of
+  // constructing them with enum values (which end up with them in
+  // normalized form). The literal form of the triple can matter for
+  // finding include files.
+  return (Twine(T.getArchName()) + "-w64-mingw32").str();
+}
+
+enum Format { Rc, Res, Coff, Unknown };
+
+struct RcOptions {
+  bool Preprocess = true;
+  bool PrintCmdAndExit = false;
+  std::string Triple;
+  std::vector PreprocessCmd;
+  std::vector PreprocessArgs;
+
+  std::string InputFile;
+  Format InputFormat = Rc;
+  std::string OutputFile;
+  Format OutputFormat = Res;
+
+  bool BeVerbose = false;
+  WriterParams Params;
+  bool AppendNull = false;
+  bool IsDryRun = false;
+  // Set the default language; 

[PATCH] D100756: [llvm-rc] [4/4] Add a GNU windres-like frontend to llvm-rc

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo updated this revision to Diff 338978.
mstorsjo added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Rebased on top of the updated preceding patch. Adjusted the logic for picking th
e default triple, to preserve the exact spelling of the default triple, if 
isWindowsGNUEnvironment() is true. Added a preprocessing test under clang.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100756

Files:
  clang/test/CMakeLists.txt
  clang/test/Preprocessor/Inputs/llvm-windres.h
  clang/test/Preprocessor/llvm-windres.rc
  llvm/test/CMakeLists.txt
  llvm/test/lit.cfg.py
  llvm/test/tools/llvm-rc/codepage.test
  llvm/test/tools/llvm-rc/language.test
  llvm/test/tools/llvm-rc/windres-format.test
  llvm/test/tools/llvm-rc/windres-prefix.test
  llvm/test/tools/llvm-rc/windres-preproc.test
  llvm/test/tools/llvm-rc/windres-target.test
  llvm/test/tools/llvm-rc/windres-version.test
  llvm/tools/llvm-rc/CMakeLists.txt
  llvm/tools/llvm-rc/WindresOpts.td
  llvm/tools/llvm-rc/llvm-rc.cpp

Index: llvm/tools/llvm-rc/llvm-rc.cpp
===
--- llvm/tools/llvm-rc/llvm-rc.cpp
+++ llvm/tools/llvm-rc/llvm-rc.cpp
@@ -18,8 +18,10 @@
 #include "ResourceScriptToken.h"
 
 #include "llvm/ADT/Triple.h"
+#include "llvm/Object/WindowsResource.h"
 #include "llvm/Option/Arg.h"
 #include "llvm/Option/ArgList.h"
+#include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/FileUtilities.h"
@@ -75,8 +77,39 @@
   RcOptTable() : OptTable(InfoTable, /* IgnoreCase = */ true) {}
 };
 
+enum Windres_ID {
+  WINDRES_INVALID = 0, // This is not a correct option ID.
+#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
+   HELPTEXT, METAVAR, VALUES)  \
+  WINDRES_##ID,
+#include "WindresOpts.inc"
+#undef OPTION
+};
+
+#define PREFIX(NAME, VALUE) const char *const WINDRES_##NAME[] = VALUE;
+#include "WindresOpts.inc"
+#undef PREFIX
+
+static const opt::OptTable::Info WindresInfoTable[] = {
+#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
+   HELPTEXT, METAVAR, VALUES)  \
+  {\
+  WINDRES_##PREFIX, NAME, HELPTEXT,\
+  METAVAR,  WINDRES_##ID, opt::Option::KIND##Class,\
+  PARAM,FLAGS,WINDRES_##GROUP, \
+  WINDRES_##ALIAS,  ALIASARGS,VALUES},
+#include "WindresOpts.inc"
+#undef OPTION
+};
+
+class WindresOptTable : public opt::OptTable {
+public:
+  WindresOptTable() : OptTable(WindresInfoTable, /* IgnoreCase = */ false) {}
+};
+
 static ExitOnError ExitOnErr;
 static FileRemover TempPreprocFile;
+static FileRemover TempResFile;
 
 LLVM_ATTRIBUTE_NORETURN static void fatalError(const Twine ) {
   errs() << Message << "\n";
@@ -121,10 +154,43 @@
   return T.str();
 }
 
-bool preprocess(StringRef Src, StringRef Dst, opt::InputArgList ,
+std::string getMingwTriple() {
+  Triple T(sys::getDefaultTargetTriple());
+  if (T.isWindowsGNUEnvironment())
+return T.str();
+  // Write out the literal form of the vendor/env here, instead of
+  // constructing them with enum values (which end up with them in
+  // normalized form). The literal form of the triple can matter for
+  // finding include files.
+  return (Twine(T.getArchName()) + "-w64-mingw32").str();
+}
+
+enum Format { Rc, Res, Coff, Unknown };
+
+struct RcOptions {
+  bool Preprocess = true;
+  bool PrintCmdAndExit = false;
+  std::string Triple;
+  std::vector PreprocessCmd;
+  std::vector PreprocessArgs;
+
+  std::string InputFile;
+  Format InputFormat = Rc;
+  std::string OutputFile;
+  Format OutputFormat = Res;
+
+  bool BeVerbose = false;
+  WriterParams Params;
+  bool AppendNull = false;
+  bool IsDryRun = false;
+  // Set the default language; choose en-US arbitrarily.
+  unsigned LangId = (/*PrimaryLangId*/ 0x09) | (/*SubLangId*/ 0x01 << 10);
+};
+
+bool preprocess(StringRef Src, StringRef Dst, const RcOptions ,
 const char *Argv0) {
   std::string Clang;
-  if (InputArgs.hasArg(OPT__HASH_HASH_HASH)) {
+  if (Opts.PrintCmdAndExit) {
 Clang = "clang";
   } else {
 ErrorOr ClangOrErr = findClang(Argv0);
@@ -139,40 +205,27 @@
   return false;
 }
   }
-  std::string PreprocTriple = getClangClTriple();
 
   SmallVector Args = {
-  Clang, "--driver-mode=gcc", "-target", PreprocTriple, "-E",
-  "-xc", "-DRC_INVOKED",  Src,   "-o",  Dst};
-  if (InputArgs.hasArg(OPT_noinclude)) {
-#ifdef _WIN32
-::_putenv("INCLUDE=");
-#else
-::unsetenv("INCLUDE");
-#endif
-  }
-  for (const auto *Arg :
-   InputArgs.filtered(OPT_includepath,