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