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

[Eli Friedman]

On Mon, Feb 6, 2012 at 4:06 PM, Anton Yartsev <anton.yartsev at gmail.com> wrote:
> 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?

The patch wasn't quite right... I committed a revised version in r149963.

-Eli