[Patch] Turn Driver::CCCIsCXX and CCCIsCPP into an enum, add -cxx-mode= option
Reid Kleckner
rnk at google.com
Tue Jul 16 06:42:45 PDT 2013
>
> diff --git a/include/clang/Driver/Driver.h b/include/clang/Driver/Driver.h
> index 1459e0d..cd114b5 100644
> --- a/include/clang/Driver/Driver.h
> +++ b/include/clang/Driver/Driver.h
> @@ -51,6 +51,13 @@ class Driver {
>
> DiagnosticsEngine &Diags;
>
> + enum CCCMode {
> + GCCMode,
> + GXXMode,
> + CPPMode,
> + InvalidMode
> + } Mode;
> +
> public:
> // Diag - Forwarding function for diagnostics.
> DiagnosticBuilder Diag(unsigned DiagID) const {
> @@ -117,10 +124,10 @@ public:
> InputList;
>
> /// Whether the driver should follow g++ like behavior.
> - unsigned CCCIsCXX : 1;
> + bool CCCIsCXX() const { return Mode == GXXMode; }
>
> /// Whether the driver is just the preprocessor.
> - unsigned CCCIsCPP : 1;
> + bool CCCIsCPP() const { return Mode == CPPMode; }
>
> /// Echo commands while executing (in -v style).
> unsigned CCCEcho : 1;
> @@ -236,6 +243,9 @@ public:
> /// @name Driver Steps
> /// @{
>
> + /// ParseCCCMode - Look for and handle the -ccc-mode= option in Args.
> + void ParseCCCMode(ArrayRef<const char *> Args);
> +
> /// ParseArgStrings - Parse the given list of strings into an
> /// ArgList.
> llvm::opt::InputArgList *ParseArgStrings(ArrayRef<const char *> Args);
> diff --git a/include/clang/Driver/Options.td
> b/include/clang/Driver/Options.td
> index 7e7e111..7834ed7 100644
> --- a/include/clang/Driver/Options.td
> +++ b/include/clang/Driver/Options.td
> @@ -112,8 +112,8 @@ def ccc_debug_Group : OptionGroup<"<clang
> debug/development internal options>">,
> Group<ccc_Group>, HelpText<"DEBUG/DEVELOPMENT OPTIONS">;
>
> class CCCDriverOpt : Group<ccc_driver_Group>, Flags<[DriverOption,
> HelpHidden]>;
> -def ccc_cxx : Flag<["-"], "ccc-cxx">, CCCDriverOpt,
> - HelpText<"Act as a C++ driver">;
> +def ccc_mode : Joined<["-"], "ccc-mode=">, CCCDriverOpt,
> + HelpText<"Set the ccc mode to either 'gcc', 'g++' or 'cpp'">;
>
-ccc creates a nice namespace for clang, but I don't think it's at all
intuitive and it's now a misnomer (clang "c" compiler) and I'm not sure if
we want to promote it.
There was also some discussion about preferring "--" prefixes for new long
options, so I'd go with that.
lld calls this the driver "flavor" rather than mode. It would be nice to
stay consistent on terminology, if folks are OK with flavor over mode.
Putting it together, I was basically thinking of --driver-flavor=foo, but
we should get more buy in.
> def ccc_echo : Flag<["-"], "ccc-echo">, CCCDriverOpt,
> HelpText<"Echo commands before running them">;
> def ccc_gcc_name : Separate<["-"], "ccc-gcc-name">, CCCDriverOpt,
> diff --git a/lib/Driver/Driver.cpp b/lib/Driver/Driver.cpp
> index 1daabcb..8b01668 100644
> --- a/lib/Driver/Driver.cpp
> +++ b/lib/Driver/Driver.cpp
> @@ -21,6 +21,7 @@
> #include "llvm/ADT/ArrayRef.h"
> #include "llvm/ADT/OwningPtr.h"
> #include "llvm/ADT/StringSet.h"
> +#include "llvm/ADT/StringSwitch.h"
> #include "llvm/Option/Arg.h"
> #include "llvm/Option/ArgList.h"
> #include "llvm/Option/OptTable.h"
> @@ -53,8 +54,8 @@ Driver::Driver(StringRef ClangExecutable,
> DefaultImageName(DefaultImageName),
> DriverTitle("clang LLVM compiler"),
> CCPrintOptionsFilename(0), CCPrintHeadersFilename(0),
> - CCLogDiagnosticsFilename(0), CCCIsCXX(false),
> - CCCIsCPP(false),CCCEcho(false), CCCPrintBindings(false),
> + CCLogDiagnosticsFilename(0), Mode(GCCMode),
> + CCCEcho(false), CCCPrintBindings(false),
> CCPrintOptions(false), CCPrintHeaders(false), CCLogDiagnostics(false),
> CCGenDiagnostics(false), CCCGenericGCCName(""),
> CheckInputsExist(true),
> CCCUsePCH(true), SuppressMissingInputWarning(false) {
> @@ -81,6 +82,29 @@ Driver::~Driver() {
> delete I->second;
> }
>
> +void Driver::ParseCCCMode(ArrayRef<const char *> Args) {
> + const std::string CCCOptName =
> + getOpts().getOption(options::OPT_ccc_mode).getPrefixedName();
> +
> + for (size_t I = 0, E = Args.size(); I != E; ++I) {
> + const StringRef Arg = Args[I];
> + if (!Arg.startswith(CCCOptName))
> + continue;
> +
> + const StringRef Value = Arg.drop_front(CCCOptName.size());
> + CCCMode M = llvm::StringSwitch<CCCMode>(Value)
> + .Case("gcc", GCCMode)
> + .Case("g++", GXXMode)
> + .Case("cpp", CPPMode)
> + .Default(InvalidMode);
>
I bet you can get rid of InvalidMode by making the switch type unsigned and
returning ~0U in Default or something.
> +
> + if (M != InvalidMode)
> + Mode = M;
> + else
> + Diag(diag::err_drv_unsupported_option_argument) << CCCOptName <<
> Value;
> + }
> +}
> +
>
.. snip
> diff --git a/test/Driver/ccc-as-cpp.c b/test/Driver/ccc-as-cpp.c
> index feead51..6c00433 100644
> --- a/test/Driver/ccc-as-cpp.c
> +++ b/test/Driver/ccc-as-cpp.c
> @@ -1,6 +1,3 @@
> -// REQUIRES: shell
> -// RUN: ln -sf %clang %T/clang-cpp
> -
> // PR13529: Don't crash.
> -// RUN: %T/clang-cpp -lfoo -M %s 2>&1 | FileCheck
> --check-prefix=CHECK-PR13529 %s
> +// RUN: %clang_cpp -lfoo -M %s 2>&1 | FileCheck
> --check-prefix=CHECK-PR13529 %s
>
Nice.
On Mon, Jul 15, 2013 at 9:28 PM, Hans Wennborg <hans at chromium.org> wrote:
> On Mon, Jul 15, 2013 at 6:11 PM, David Blaikie <dblaikie at gmail.com> wrote:
> > (not claiming ownership of this CR, but a few drive-by comments)
> >
> > You could consider using a StringSwitch in Driver::ParseCCCMode.
>
> Done. It's a little annoying that I need an invalid value for the
> default case, but on the other hand the StringSwitch is nice.
>
> > Is there any reason you need to remove support for "clangxx" (that's
> > causing you to need to update those hexagon tests?)?
>
> I'm not, I'm switching those hexagon tests over to use clangxx instead
> of -ccc-cxx which I'm removing.
>
> > Would you mind updating immediate-options.c to use FileCheck rather
> > than multiple invocations with grep?
>
> Sure. I'd like to do that in a separate patch, though.
>
> Thanks for the comments! New patch attached.
>
> - Hans
>
>
> > On Mon, Jul 15, 2013 at 4:17 PM, Hans Wennborg <hans at chromium.org>
> wrote:
> >> Hi all,
> >>
> >> This patch is a follow-up to the discussion in Reid's cl.exe
> >> compatible driver proposal [1].
> >>
> >> Clang currently looks at argv[0] to switch between running in
> >> gcc/g++/cpp mode. This mode is represented in Driver by CCCIsCXX and
> >> CCCIsCPP. Since there is no overlap between the three modes, I have
> >> turned those two variables into an enum instead. The plan is to later
> >> extend this enum with a "cl.exe mode".
> >>
> >> The patch also adds a new command line option, -ccc-mode, to set the
> >> mode. This replaces the current -ccc-cxx option (and also makes it
> >> easier to test the cpp mode). This option is special: because the mode
> >> can affect option parsing (the cl.exe mode will add new options), the
> >> -ccc-mode needs to be parsed early.
> >>
> >> Please take a look!
> >>
> >> Thanks,
> >> Hans
> >>
> >> [1]. http://lists.cs.uiuc.edu/pipermail/cfe-dev/2013-June/030439.html
> >>
> >> _______________________________________________
> >> cfe-commits mailing list
> >> cfe-commits at cs.uiuc.edu
> >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> >>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130716/44bd8d73/attachment.html>
More information about the cfe-commits
mailing list