LGTM! Sent from my iPhone
On Mar 17, 2012, at 5:25 PM, Aaron Ballman <[email protected]> wrote: > On Sat, Mar 17, 2012 at 7:14 PM, Douglas Gregor <[email protected]> wrote: >> >> On Mar 17, 2012, at 5:11 PM, Aaron Ballman wrote: >> >>> On Sat, Mar 17, 2012 at 7:03 PM, Douglas Gregor <[email protected]> wrote: >>>> >>>> On Mar 17, 2012, at 5:00 PM, Aaron Ballman wrote: >>>> >>>>> On Thu, Mar 15, 2012 at 7:29 AM, Aaron Ballman <[email protected]> >>>>> wrote: >>>>>> On Wed, Mar 14, 2012 at 11:25 PM, J B <[email protected]> wrote: >>>>>>> The reason I recommend an error is because the code will error out later >>>>>>> without the .tlh (where it is requested via attributes or absence of). >>>>>>> The >>>>>>> errors later will be a direct result of the failure of #import to >>>>>>> function >>>>>>> properly. >>>>>> >>>>>> That's a valid reason to error in my book. I'll go down that route. >>>>>> Thanks for the insights! >>>>> >>>>> This is the revised patch based on feedback from everyone. It causes >>>>> a (shorter) error to happen when using #import in MS mode, and does >>>>> not perform the actual include. It also eats the optional attributes >>>>> so that parsing can continue. >>>>> >>>>> Again, this will fix PR 10727 >>>> >>>> +def err_pp_import_directive_ms : Error< >>>> + "#import does not convert a type library to C++ classes in Microsoft >>>> Mode">, >>>> + InGroup<Microsoft>; >>>> >>>> This should really just say that this Microsoft feature is not implemented. >>>> >>>> - if (!LangOpts.ObjC1) // #import is standard for ObjC. >>>> + if (LangOpts.MicrosoftMode) >>>> + return HandleMicrosoftImportDirective(ImportTok); >>>> + else if (!LangOpts.ObjC1) // #import is standard for ObjC. >>>> Diag(ImportTok, diag::ext_pp_import_directive); >>>> >>>> I think we should do this the other way: if LangOpts.ObjC1, we treat it as >>>> an Objective-C #import. Otherwise, we either error (if in Microsoft mode) >>>> or give an extension warning (not in Microsoft mode). >>> >>> Good points -- rectified in this patch. >> >> >> Last complaint, but otherwise LGTM: >> >> +def err_pp_import_directive_ms : Error< >> + "this Microsoft feature is not implemented">, >> + InGroup<Microsoft>; >> + >> >> How about "#import of type library is an unsupported Microsoft feature" > > Sounds good to me > > ~Aaron > <MicrosoftImport.patch> _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
