[Lldb-commits] [PATCH] D68980: [LLDB] [test] Allow specifying the full arch for msvc/clang-cl in build.py

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Having proper "target" support is one way to go, though I am disappointed to see it go this way, particularly as nobody really knows why invoking clang-cl directly should not work, and so the main motivation seems to be a fear of potentially breaking something, without understanding what that "something" is, or whether it even exists. The reason I am disappointed is because this script is a fairly big departure from the way most lit tests in llvm work, and it wasn't ever really fully embraced by the lldb community (as can be seen by the amount of %clangxx tests that could be using it, but aren't). And while I do think it has some benefits, I don't think it should be used as a replacement for all clang-cl invocations. If it turns out that there having a script wrapping clang-cl is a necessity (which I doubt), then I'd rather see a different python script (possibly reusing parts of this one) which just sets up the necessary environment, but otherwise forwards arguments to clang-cl verbatim. That would enable one to invoke clang-cl with any fancy argument he wants (and this could be hidden under %clang-cl), while leaving this script with the job of figuring out how to build a working host executable with a random compiler.

Anyway, if we're going down this path, then I think it should be done properly:

- having "host" mean "x64" is just wrong. It should be the "host"
- "32" and "64" sound like they could be useful, so they can stay if they are needed, but they should mean "the 32-bit flavour of 'host'", not "i386".
- anything else specifies an exact target, and it should probably be a full triple, not just the architecture
- using an unsupported builder/target (gcc) should be an error


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

https://reviews.llvm.org/D68980