[PATCH] D32435: clang-cl: Add support for /permissive-

Tue Apr 25 14:28:31 PDT 2017

On Tue, Apr 25, 2017 at 2:12 PM, Nico Weber <thakis at chromium.org> wrote:

> On Tue, Apr 25, 2017 at 4:14 PM, David Majnemer <david.majnemer at gmail.com>
> wrote:
>
>>
>>
>> On Tue, Apr 25, 2017 at 11:42 AM, Nico Weber <thakis at chromium.org> wrote:
>>
>>> On Tue, Apr 25, 2017 at 2:06 PM, David Majnemer <
>>> david.majnemer at gmail.com> wrote:
>>>
>>>>
>>>>
>>>> On Tue, Apr 25, 2017 at 8:46 AM, Nico Weber <thakis at chromium.org>
>>>> wrote:
>>>>
>>>>> On Mon, Apr 24, 2017 at 10:00 PM, David Majnemer <
>>>>> david.majnemer at gmail.com> wrote:
>>>>>
>>>>>>
>>>>>>
>>>>>> On Mon, Apr 24, 2017 at 11:56 AM, Nico Weber <thakis at chromium.org>
>>>>>> wrote:
>>>>>>
>>>>>>> "Opting into the conforming mode, /permissive-, during the series of
>>>>>>> VS 2017 update is a commitment to keeping your code base clean and to
>>>>>>> fixing non-conforming constructs we fix conformance issues in Visual C++."
>>>>>>> [...] "By contrast /permissive- offers a useful conformance mode where
>>>>>>> input C++ code is interpreted according to ISO C++ rules but also allows
>>>>>>> conforming extensions necessary to compile C++ on targets supported by
>>>>>>> Visual C++."
>>>>>>>
>>>>>>> I guess the second quote agrees with your interpretation.
>>>>>>>
>>>>>>> We already diag most of the things they already mention. The one
>>>>>>> thing we don't diag by default is Wmicrosoft-enum-forward-reference
>>>>>>> since that's only an Extension and not an ExtWarn. We don't expose
>>>>>>> -pedantic from clang-cl, so this seemed like a somewhat natural mapping to
>>>>>>> me.
>>>>>>>
>>>>>>> Should /permissive- map to -Wmicrosoft instead and turn on the parts
>>>>>>> of -Wmicrosoft that are Extensions?
>>>>>>>
>>>>>>
>>>>>> Did you mean on or off?
>>>>>>
>>>>>
>>>>> On.
>>>>>
>>>>>
>>>>>> I think that their intent is that things like __declspec remain OK.
>>>>>>
>>>>>
>>>>> Nothing in -Wmicrosoft warns on __declspec.
>>>>>
>>>>>
>>>>>> They want to diagnose non-conforming extensions like crazy template
>>>>>> stuff, bogus typedef syntax, bad main function definitions, etc.
>>>>>>
>>>>>
>>>>> Right. The only thing it currently makes cl warn on that clang-cl
>>>>> doesn't warn on by default is Wmicrosoft-enum-forward-reference,
>>>>> which is an Extension warning, not an ExtWarn. So mapping /permissive- to
>>>>> -Wmicrosoft would make clang-cl diagnose forward-declared enums like it
>>>>> does with 2017 cl.
>>>>>
>>>>
>>>> Ok, sounds like it diagnoses the same sorts of things. They diagnose as
>>>> error though, I think we should too. What about -fdelayed-template-parsing?
>>>> Shouldn't that be disabled?
>>>>
>>>
>>> CL has added a /Zc:twoPhase for that (not yet released anywhere), and
>>> Hans added support for that to clang-cl a while ago. Some blog post (maybe
>>> the one I linked to?) says that they're thinking of possibly
>>> enabling /Zc:twoPhase when /permissive- is passed, but at the moment it's
>>> independent. (In part because /permissive- ships in VC2017 and /Zc:twoPhase
>>> hasn't been shipped yet).
>>>
>>
>> Ok.
>>
>>
>>>
>>> What's the advantage of making it an error?
>>>
>>
>> All the diagnostics they show in https://blogs.msdn.microsof
>> t.com/vcblog/2016/11/16/permissive-switch/ diagnose as errors.
>>
>
> But why should we match that? Having them be warnings seems like a better
> user experience to me.
>

I think the thinking behind the flag is that they want folks to be able to
make their code more standards compliant. I imagine that anybody who turns
the flag on understands what they are getting themselves into.

IMO, the biggest reason why it should match their diagnostic level is
because builds could otherwise break if a project has /permissive- but
clang-cl allows a build to continue while cl would not.

>
>
>>
>>
>>> If it's a warning, you can pass -Werror separately if you want. And
>>> SFINAE'ing on this seems like asking for trouble.
>>>
>>
>> I said nothing about permitting SFINAE on these errors.
>>
>>
>>>
>>>
>>>>
>>>>
>>>>>
>>>>>
>>>>>>
>>>>>>
>>>>>>> Should we just ignore /permissive- and possibly make some of our
>>>>>>> -Wmicrosoft Extensions ExtWarns instead?
>>>>>>>
>>>>>>> On Mon, Apr 24, 2017 at 2:10 PM, David Majnemer <
>>>>>>> david.majnemer at gmail.com> wrote:
>>>>>>>
>>>>>>>> -pedantic means "Issue all the warnings demanded by strict ISO C
>>>>>>>> and ISO C++; reject all programs that use forbidden extensions, and some
>>>>>>>> other programs that do not follow ISO C and ISO C++."
>>>>>>>> I believe it is more akin to -fno-ms-compatibility as it disables
>>>>>>>> compatibility hacks.
>>>>>>>>
>>>>>>>> On Mon, Apr 24, 2017 at 11:02 AM, Nico Weber <thakis at chromium.org>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>> It does sound pretty similar to me from the blog post. I think
>>>>>>>>> this is a decent place to start from.
>>>>>>>>>
>>>>>>>>> On Apr 24, 2017 11:55 AM, "David Majnemer via Phabricator via
>>>>>>>>> cfe-commits" <cfe-commits at lists.llvm.org> wrote:
>>>>>>>>>
>>>>>>>>>> majnemer requested changes to this revision.
>>>>>>>>>> majnemer added a comment.
>>>>>>>>>> This revision now requires changes to proceed.
>>>>>>>>>>
>>>>>>>>>> I don't think this is correct. GDR (of Microsoft) says the
>>>>>>>>>> behavior is different: https://www.reddit.com/r/cpp/comm
>>>>>>>>>> <https://www.reddit.com/r/cpp/comments/5dh7j5/visual_c_introduces_permissive_for_conformance/da5fxjj/>
>>>>>>>>>>               LOG(INFO) << "n_window_index: " << n_window_index;
>>>>>>>>>> ents/5dh7j5/visual_c_introduces_permissive_for_conformance/d
>>>>>>>>>> a5fxjj/
>>>>>>>>>> <https://www.reddit.com/r/cpp/comments/5dh7j5/visual_c_introduces_permissive_for_conformance/da5fxjj/>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> https://reviews.llvm.org/D32435
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> _______________________________________________
>>>>>>>>>> cfe-commits mailing list
>>>>>>>>>> cfe-commits at lists.llvm.org
>>>>>>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170425/6acd0f11/attachment.html>