[Patch] Turn Driver::CCCIsCXX and CCCIsCPP into an enum, add -cxx-mode= option

[Reid Kleckner]

LGTM

Sorry, this fell out of my inbox.


On Tue, Jul 16, 2013 at 1:50 PM, Hans Wennborg <hans at chromium.org> wrote:

> Thanks for the review!
>
> On Tue, Jul 16, 2013 at 6:42 AM, Reid Kleckner <rnk at google.com> wrote:
> >> +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.
>
> --driver-flavor=foo sounds good to me.
>
> >> +    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.
>
> Done.
>
> New patch attached.
>
>  - Hans
>
> > 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130718/7d5fdd9f/attachment.html>