[patch] Delay __fastcall attribute checking utnil after merging declarations

[Reid Kleckner]

lgtm


On Thu, Jul 31, 2014 at 9:45 AM, Nico Weber <thakis at chromium.org> wrote:

> Thanks!
>
> On Tue, Jul 29, 2014 at 10:40 AM, Reid Kleckner <rnk at google.com> wrote:
>
>> +  // Diagnose the use of X86 fastcall on unprototyped functions.
>>
>> We should also diagnose unprototyped stdcall functions, but I'm not sure
>> if that would be too disruptive.  Currently we accept this and always call
>> _f at 0:
>> void __stdcall f();
>> int main() {
>>   f(1);
>>   f(1, 2);
>>   f(1, 2, 3);
>> }
>>
>> MSVC rejects with "t.c(5) : error C2708: 'f' : actual parameters length
>> in bytes differs from previous call or reference".
>>
>
> I added a TODO for adding this and reshuffled the if to make this easy to
> add. I'll give it a try once this patch is in.
>
>
>> +  QualType NewQType = Context.getCanonicalType(NewFD->getType());
>> +  const FunctionType *NewType = cast<FunctionType>(NewQType);
>> +  FunctionType::ExtInfo NewTypeInfo = NewType->getExtInfo();
>> +  if (NewTypeInfo.getCC() == CC_X86FastCall) {
>> +    if (isa<FunctionNoProtoType>(NewType)) {
>> +      Diag(NewFD->getLocation(), diag::err_cconv_knr)
>> +          << FunctionType::getNameForCallConv(CC_X86FastCall);
>> +    }
>> +
>> +    // Also diagnose fastcall with regparm.
>>
>> Before you moved this, there was other code to handle CC_X86FastCall and
>> AT_Regparm nearby.  I think we should either move all regparm+fastcall
>> diagnosis to after redeclaration merging or leave it where it is.
>>
>
> I moved the regparm code back to where it was.
>
>
>>
>> +    if (NewType->getHasRegParm()) {
>> +      Diag(NewFD->getLocation(), diag::err_attributes_are_not_compatible)
>> +          << "regparm" <<
>> FunctionType::getNameForCallConv(CC_X86FastCall);
>> +    }
>> +  }
>>
>>
>>
>> On Sat, Jul 26, 2014 at 3:05 PM, Nico Weber <thakis at chromium.org> wrote:
>>
>>> Hi,
>>>
>>> the attached patch delays semantic analysis of __fastcall until after
>>> MergeFunctionDecl() has been called, PR20386.
>>>
>>> The motivation is code like this in a .c file:
>>>
>>> void __fastcall CrcGenerateTable(void);
>>> void __fastcall CrcGenerateTable() {}
>>>
>>> Without this patch, __fastcall is analyzed before the definition of
>>> CrcGenerateTable() is merged with the declaration, so Sema thinks the
>>> function doesn't have a prototype. Since the error is only emitted on
>>> FunctionNoProto functions, and functions in C++ or functions with more than
>>> 0 arguments have a proto, this is only an issue for functions with 0
>>> parameters in C files. See the bug for some more notes.
>>>
>>> (A side effect of moving the diagnostic is that the diagnostic now
>>> points at the function instead of the attribute, and that the attribute is
>>> no longer marked invalid. Neither seems like a problem.)
>>>
>>> Nico
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140731/aab979c0/attachment.html>