[PATCH] clang-cl: implement /fallback mode

Reid Kleckner rnk at google.com
Thu Sep 19 11:18:13 PDT 2013


  LGTM


================
Comment at: lib/Driver/Tools.cpp:6721
@@ +6720,3 @@
+
+  // FIXME: If we've put clang-cl as cl.exe on the path, then we might have a problem now :/
+  const char *Exec =
----------------
Hans Wennborg wrote:
> Reid Kleckner wrote:
> > That seems problematic.  You could use sys::fs::equivalent() to skip a path if it's the current executable, but we'd have to sink that into GetProgramPath().
> On the good side, we're not passing /fallback through to the second invocation, so we don't get stuck forever :)
> 
> On the bad side, I don't think checking sys::fs::equivalent() would work, because the cl.exe on the PATH could be a copy of clang-cl.exe, as in the case of the current VS integration :/
> 
> It seems that automatically looking for the "true" cl.exe could get pretty messy.
> 
> What do you think about having an option, like /fallbackcommand <foo>? Ideally, the VS integration could set this automagically, and it would also be useful for testing (we could set this to "echo" or something and test the actual execution of the fallback).
I agree it could get messy, but I think there's an important common case, which is where the currently running compiler is the cl.exe on PATH, and the next cl.exe on PATH is MSVC.  That's the set up we use for VS project files, so it's pretty important for users.  I'm fine handling that as a followup, though.

================
Comment at: lib/Driver/Tools.cpp:6687
@@ +6686,3 @@
+  // Flags for which clang-cl have an alias.
+  // FIXME: Could we avoid this manual reverse translation?
+
----------------
Hans Wennborg wrote:
> Reid Kleckner wrote:
> > Can you iterate over Args.getArgString(I) up to getNumInputArgStrings(), or does that fail?
> We could do that, but we don't want to just pass through the flags verbatim - we'd need to look at the option and decide whether to include it or not, and then we get into parsing the options and it could get a bit messy. As we expose more "core" clang flags we also need to take interactions with those into account.
> 
> I like the current approach of "canonicalizing" a new cl.exe command-line based on the options as clang-cl has understood them. My only worry is keeping this in sync with the supported options. At least it's easily testable.
Oh, right, we need to filter out the core options.  I think it could be done by looping over all the args and filtering out options without the CLOption flag, but that would lose the canonicalization.  I'd prefer to kill the duplication and lose the canonicalization, but I don't feel strongly about it.


http://llvm-reviews.chandlerc.com/D1711



More information about the cfe-commits mailing list