[cfe-commits] r149587 - in /cfe/trunk: lib/AST/ASTContext.cpp test/CodeGen/inline.c test/CodeGen/inline2.c

Anton Yartsev anton.yartsev at gmail.com
Mon Feb 6 16:06:59 PST 2012


On 04.02.2012 5:35, Anton Yartsev wrote:
> On 03.02.2012 23:57, Eli Friedman wrote:
>> On Fri, Feb 3, 2012 at 11:34 AM, Anton 
>> Yartsev<anton.yartsev at gmail.com>  wrote:
>>> On 03.02.2012 2:34, Eli Friedman wrote:
>>>> On Wed, Feb 1, 2012 at 9:33 PM, Eli Friedman<eli.friedman at gmail.com>
>>>>   wrote:
>>>>> On Wed, Feb 1, 2012 at 9:13 PM, Anton 
>>>>> Yartsev<anton.yartsev at gmail.com>
>>>>>   wrote:
>>>>>> Author: ayartsev
>>>>>> Date: Wed Feb  1 23:13:59 2012
>>>>>> New Revision: 149587
>>>>>>
>>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=149587&view=rev
>>>>>> Log:
>>>>>> Fix for PR10657 (http://llvm.org/bugs/show_bug.cgi?id=10657)
>>>>>> extern inline case considered
>>>>>>
>>>>>> Modified:
>>>>>>     cfe/trunk/lib/AST/ASTContext.cpp
>>>>>>     cfe/trunk/test/CodeGen/inline.c
>>>>>>     cfe/trunk/test/CodeGen/inline2.c
>>>>> The changes you are making to the tests are clearly wrong.  Please
>>>>> revert, and we can discuss the correct way to fix your testcase.
>>>> I just took a quick look; a correct patch would probably involve
>>>> changing FunctionDecl::doesDeclarationForceExternallyVisibleDefinition
>>>> .
>>>>
>>>> -Eli
>>> Previous commit was awful, sorry. Attached is an adequate patch. Ok to
>>> commit?
>> Better... comments below:
>>
>> -// No PR#, but this once crashed clang in C99 mode due to buggy 
>> extern inline
>> -// redeclaration detection.
>> -void test7() { }
>> +// PR10657
>> +extern __inline void test7() {}
>>   void test7();
>>
>> Please make your new test a separate test; we want as much coverage as
>> possible here.
>>
>> Just to double-check, does your patch work correctly with the 
>> following test?
>>
>> void test();
>> __inline void test() {}
>>   void test();
>>
>> -Eli
> Did you mean "extern __inline"?
> The patch works fine - no crush, the definition is generated.
> Updated the test, new patch attached
>
Can I commit the patch?

-- 
Anton




More information about the cfe-commits mailing list