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

[Eli Friedman]

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