Thanks, landed in that form in r224790 / r224791. On Tue, Dec 23, 2014 at 1:16 PM, Richard Smith <[email protected]> wrote:
> On Mon, Dec 22, 2014 at 8:40 AM, Nico Weber <[email protected]> wrote: > >> On Mon, Dec 22, 2014 at 1:38 AM, Hans Wennborg <[email protected]> wrote: >> >>> On Sun, Dec 21, 2014 at 5:24 PM, Nico Weber <[email protected]> wrote: >>> > That's much nicer, thanks. I couldn't figure out how to alias a >>> clang-cl >>> > option to a cc1 option, so I added 4 lines to explicitly translate >>> > OPT__SLASH_Zc_trigraphs to -ftrigraphs, but it's still much simpler. >>> >>> Hmm, that's a little annoying, but I guess it makes sense.. the >>> clang-cl flags are handled as Driver flags so aliasing to a CC1 >>> doesn't work. >>> >>> Perhaps we should just stick to the existing -trigraphs and add a >>> -no-trigraphs driver flag? lgtm either way I guess. >>> >> >> Richard, are you okay with adding a -no-trigraphs gcc-driver-level flag? >> gcc doesn't have this, but several people on the internet are asking for >> this feature (I searched for `gcc "no-trigraphs"`). I'd like a way to build >> with -std=c++11 (i.e. not gnu++11) but still not enable trigraphs myself, >> too. >> > > I'd prefer to add -ftrigraphs and -fno-trigraphs to the driver, and make > -trigraphs an alias for -ftrigraphs. Other than that, I think this is a > good idea. > > >> > I also used the same technique to map /Zc:strictStrings to >>> > -Werror=c++11-compat-deprecated-writable-strings while here. >>> >>> Nice. >>> >>> > (Disabling trigraphs in -fms-compatibility mode for the gcc driver >>> makes >>> > sense to me too, but I think that should happen in a different patch.) >>> >>> > Property changes on: test/Driver/cl-zc.cpp >>> > ___________________________________________________________________ >>> > Added: svn:eol-style >>> > ## -0,0 +1 ## >>> > +LF >>> >>> Is this intentional? >>> >>> >>> > On Sun, Dec 21, 2014 at 4:36 AM, Hans Wennborg <[email protected]> >>> wrote: >>> >> >>> >> I would have broken out /Zc:trigraphs[-] into two separate CLFlag >>> >> options instead. >>> >> >>> >> That way we don't need parser code for it, the flags can have >>> >> individual help texts, and they can just be aliases to the cc1 flags. >>> >> >>> >> Also, instead of passing -fno-trigraphs each time, maybe we should >>> >> change the default in CompilerInvocation based on the language >>> >> options. It seems we currently disable them in gnu mode for example. >>> >> Maybe we should do the same if -fms-extensions or -fms-compatibility >>> >> if enabled? >>> >> >>> >> On Sat, Dec 20, 2014 at 11:16 AM, Nico Weber <[email protected]> >>> wrote: >>> >> > Thanks! All done. >>> >> > >>> >> > On Fri, Dec 19, 2014 at 6:47 PM, Richard Smith < >>> [email protected]> >>> >> > wrote: >>> >> >> >>> >> >> On Fri, Dec 19, 2014 at 6:08 PM, Nico Weber <[email protected]> >>> >> >> wrote: >>> >> >>> >>> >> >>> Forgot to say: Fixes PR21974. >>> >> >>> >>> >> >>> On Fri, Dec 19, 2014 at 6:07 PM, Nico Weber <[email protected]> >>> >> >>> wrote: >>> >> >>>> >>> >> >>>> Hi, >>> >> >>>> >>> >> >>>> the attached patch disables trigraphs by default in clang-cl >>> mode. To >>> >> >>>> do >>> >> >>>> so, I'm renaming the cc1 trigraph flag to -ftrigraphs, and I'm >>> adding >>> >> >>>> a >>> >> >>>> -fno-trigraphs flag. (This requires updating a handful of cc1 >>> tests, >>> >> >>>> and >>> >> >>>> translating -trigraphs to -ftrigraphs in the driver.) >>> >> >>>> >>> >> >>>> The driver grows parsing logic for /Zc:, everything other than >>> >> >>>> trigraphs >>> >> >>>> is ignored for now. (We probably want to implement at least >>> >> >>>> /Zc:inline at >>> >> >>>> some point, which is why I added parsing code for this instead of >>> >> >>>> always >>> >> >>>> sending -fno-trigraphs to cc1 in clang-cl mode.) >>> >> >> >>> >> >> >>> >> >> -def trigraphs : Flag<["-", "--"], "trigraphs">, >>> Flags<[CC1Option]>, >>> >> >> +def trigraphs : Flag<["-", "--"], "trigraphs">, >>> Flags<[DriverOption]>, >>> >> >> >>> >> >> I don't think DriverOption is right here; that means we don't >>> forward >>> >> >> the >>> >> >> flag to a gcc invocation. Now, we never invoke GCC for C-like >>> languages >>> >> >> any >>> >> >> more, but if we did, this would be wrong (and for all I know, it's >>> >> >> wrong for >>> >> >> Fortran...). You should be able to just drop the Flags. >>> >> >> >>> >> >> >>> >> >> if (Arg *A = Args.getLastArg(options::OPT_std_EQ, >>> >> >> options::OPT_ansi, >>> >> >> - options::OPT_trigraphs)) >>> >> >> - if (A != Std) >>> >> >> + options::OPT_trigraphs)) { >>> >> >> + if (A->getOption().matches(options::OPT_trigraphs)) >>> >> >> + CmdArgs.push_back("-ftrigraphs"); >>> >> >> + else if (A != Std) >>> >> >> A->render(Args, CmdArgs); >>> >> >> >>> >> >> Std here is Args.getLastArg(options::OPT_std_EQ, >>> options::OPT_ansi), so >>> >> >> your 'else if' clause is unreachable. Something like this would be >>> >> >> simpler >>> >> >> and more obvious: >>> >> >> >>> >> >> // If -trigraphs appears after the language standard flag, honor >>> it. >>> >> >> if (Args.getLastArg(options::OPT_std_EQ, options::OPT_ansi, >>> >> >> options::OPT_trigraphs) != Std) >>> >> >> CmdArgs.push_back("-ftrigraphs"); >>> >> >> >>> >> >> You could even sink this below the 'if (Std)' block to avoid >>> >> >> duplicating >>> >> >> it between the cases where we do and don't have a Std flag. >>> >> >> >>> >> >>>> Ok? >>> >> >>>> >>> >> >>>> Nico >>> >> >>> >>> >> >>> >>> >> >>> >>> >> >>> _______________________________________________ >>> >> >>> cfe-commits mailing list >>> >> >>> [email protected] >>> >> >>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >>> >> >>> >>> >> > >>> >> > >>> >> > _______________________________________________ >>> >> > cfe-commits mailing list >>> >> > [email protected] >>> >> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >>> >> > >>> > >>> > >>> >> >> >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
