[cfe-commits] r120872 - in /cfe/trunk: lib/Sema/SemaDeclAttr.cpp test/Sema/warn-unused-function.c

[Argyrios Kyrtzidis]

On Dec 6, 2010, at 10:33 AM, John McCall wrote:

> 
> On Dec 6, 2010, at 10:25 AM, Argyrios Kyrtzidis wrote:
> 
>> 
>> On Dec 6, 2010, at 10:12 AM, John McCall wrote:
>> 
>>> 
>>> On Dec 6, 2010, at 10:06 AM, Argyrios Kyrtzidis wrote:
>>> 
>>>> On Dec 5, 2010, at 8:33 PM, John McCall wrote:
>>>> 
>>>>> 
>>>>> On Dec 3, 2010, at 5:12 PM, Argyrios Kyrtzidis wrote:
>>>>> 
>>>>>> Author: akirtzidis
>>>>>> Date: Fri Dec  3 19:12:11 2010
>>>>>> New Revision: 120872
>>>>>> 
>>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=120872&view=rev
>>>>>> Log:
>>>>>> Mark functions referenced by 'cleanup' attribute as used. Fixes rdar://8728293
>>>>> 
>>>>>> 
>>>>>> Modified:
>>>>>> cfe/trunk/lib/Sema/SemaDeclAttr.cpp
>>>>>> cfe/trunk/test/Sema/warn-unused-function.c
>>>>>> 
>>>>>> Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
>>>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=120872&r1=120871&r2=120872&view=diff
>>>>>> ==============================================================================
>>>>>> --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
>>>>>> +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Fri Dec  3 19:12:11 2010
>>>>>> @@ -1450,6 +1450,7 @@
>>>>>> }
>>>>>> 
>>>>>> d->addAttr(::new (S.Context) CleanupAttr(Attr.getLoc(), S.Context, FD));
>>>>>> +  FD->setUsed();
>>>>>> }
>>>>> 
>>>>> Technically, you can use __attribute__((cleanup)) inside a template.  This should use MarkDeclarationReferenced, and then there should be special instantiation logic for the attribute.
>>>> 
>>>> Hmm, is this really the right approach ? If it's referenced from a template which is not instantiated, it's not used from the codegen perspective but how about from the user's perspective:
>>>> 
>>>> -We emit a warning that the function 'foo' is unused.
>>>> -User removes function 'foo'
>>>> -Now we emit an error that 'foo' is not found.
>>> 
>>> Oh, you mean if a function is referenced from a template definition that's never instantiated?  Yeah, that's a valid user complaint.  That's not specific to attribute((cleanup)), though;  any reference without dependent ADL will have that issue.
>> 
>> For this specific case, the lookup is always non-dependent, I think the right user perspective is that the 'foo' is referenced if the attribute references it even in uninstantiated templates.
> 
> It could be dependent if we implemented overloading on it. :)
> 
> I'm not sure why __attribute__((cleanup("foo"))) is different from, say, an explicit call to (foo)(x), or &foo.  In all of these cases, from the user's perspective the function is used, because their code probably won't compile if you remove the declaration.  So you should still use MarkDeclarationReferenced, and if we want to teach MarkDeclarationReferenced about this user notion of a dependent use, we've got one place to change.

Sounds good, thanks.

> 
> John.