On Tue, Apr 25, 2017 at 5:28 PM, David Majnemer <david.majne...@gmail.com> wrote:
> > > On Tue, Apr 25, 2017 at 2:12 PM, Nico Weber <tha...@chromium.org> wrote: > >> On Tue, Apr 25, 2017 at 4:14 PM, David Majnemer <david.majne...@gmail.com >> > wrote: >> >>> >>> >>> On Tue, Apr 25, 2017 at 11:42 AM, Nico Weber <tha...@chromium.org> >>> wrote: >>> >>>> On Tue, Apr 25, 2017 at 2:06 PM, David Majnemer < >>>> david.majne...@gmail.com> wrote: >>>> >>>>> >>>>> >>>>> On Tue, Apr 25, 2017 at 8:46 AM, Nico Weber <tha...@chromium.org> >>>>> wrote: >>>>> >>>>>> On Mon, Apr 24, 2017 at 10:00 PM, David Majnemer < >>>>>> david.majne...@gmail.com> wrote: >>>>>> >>>>>>> >>>>>>> >>>>>>> On Mon, Apr 24, 2017 at 11:56 AM, Nico Weber <tha...@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. > But you can just pass -Werror if you care about that, disable things you don't like with -Wno-, etc. If we make /permissive- turn on some ill-defined set of warnigns as errors, we now need to keep that list of diagnostics closely synced up to what cl does, and it makes things harder for both users and us, without a clear benefit. > > >> >> >>> >>> >>>> 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.majne...@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 <tha...@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@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@lists.llvm.org >>>>>>>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >>>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >> >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits