[cfe-commits] [PATCH] #import directive in MS compatibility mode

[Douglas Gregor]

LGTM!

Sent from my iPhone

On Mar 17, 2012, at 5:25 PM, Aaron Ballman <aaron at aaronballman.com> wrote:

> On Sat, Mar 17, 2012 at 7:14 PM, Douglas Gregor <dgregor at apple.com> wrote:
>> 
>> On Mar 17, 2012, at 5:11 PM, Aaron Ballman wrote:
>> 
>>> On Sat, Mar 17, 2012 at 7:03 PM, Douglas Gregor <dgregor at apple.com> wrote:
>>>> 
>>>> On Mar 17, 2012, at 5:00 PM, Aaron Ballman wrote:
>>>> 
>>>>> On Thu, Mar 15, 2012 at 7:29 AM, Aaron Ballman <aaron at aaronballman.com> wrote:
>>>>>> On Wed, Mar 14, 2012 at 11:25 PM, J B <jayblanchard.cpp at gmail.com> 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>