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

Tue Apr 25 14:12:12 PDT 2017

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.
> microsoft.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.

>
>
>> 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/0fc70c87/attachment-0001.html>