[patch] Delay __fastcall attribute checking utnil after merging declarations

[Nico Weber]

r214408, thanks!


On Thu, Jul 31, 2014 at 10:12 AM, Reid Kleckner <rnk at google.com> wrote:

> 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/fdc98d3f/attachment.html>