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

[Hans Wennborg]

Thanks!

I actually forgot to rename -ccc-mode to --driver-flavor in one place.
Also, we should "claim" the option to avoid the "unused during
compilation" warning. Attaching new patch that fixes that.

Does anyone have more comments on this? OK to commit?

Thanks,
Hans

On Thu, Jul 18, 2013 at 10:12 AM, Reid Kleckner <rnk at google.com> wrote:
> 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 --------------
A non-text attachment was scrubbed...
Name: driver-flavor2.patch
Type: application/octet-stream
Size: 25704 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130718/8f3fbf93/attachment.obj>