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

Hans Wennborg hans at chromium.org
Tue Jul 16 10:50:24 PDT 2013


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-flavor.patch
Type: application/octet-stream
Size: 25231 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130716/ddd8f521/attachment.obj>


More information about the cfe-commits mailing list