[PATCH] Cleanup attribute using resolved identifiers

[Aaron Ballman]

Thanks for the review -- I've resolved the duplicate diagnostic and
committed in r190476.

It's quite possible we're missing an access check for the function.
I'll put that on my list of things to explore.  :-)

~Aaron

On Tue, Sep 10, 2013 at 8:24 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> It looks like you'll produce duplicate diagnostics if
> ResolveSingleFunctionTemplateSpecialization fails (one for the resolution
> failure, and another saying "'cleanup' attribute is not a function).
>
> [I also suspect we're missing an access check for the function. I'm vaguely
> wondering whether we should recast the check as an attempt to
> copy-initialize a function pointer of the appropriate type from the
> expression. But I think this patch is doing enough things already.]
>
>
> On Tue, Sep 10, 2013 at 3:52 PM, Aaron Ballman <aaron at aaronballman.com>
> wrote:
>>
>> Since there were some reasonable substantial changes to handle
>> explicit template arguments, I am resubmitting for a final look-over.
>>
>> Thanks!
>>
>> ~Aaron
>>
>> On Tue, Sep 10, 2013 at 4:59 PM, Richard Smith <richard at metafoo.co.uk>
>> wrote:
>> > Your -Wgcc-compat check for DRE->hasQualifier() should also check for
>> > DRE->hasExplicitTemplateArgs(). With that and a test for it, LGTM.
>> >
>> >
>> > On Mon, Sep 9, 2013 at 5:47 AM, Aaron Ballman <aaron at aaronballman.com>
>> > wrote:
>> >>
>> >> Ping
>> >>
>> >> On Wed, Sep 4, 2013 at 3:08 PM, Aaron Ballman <aaron at aaronballman.com>
>> >> wrote:
>> >> > On Wed, Sep 4, 2013 at 1:35 PM, Richard Smith <richard at metafoo.co.uk>
>> >> > wrote:
>> >> >> This goes beyond what GCC supports: there, the attribute must be
>> >> >> given
>> >> >> a
>> >> >> simple identifier. I think this extension is reasonable, but please
>> >> >> add
>> >> >> a
>> >> >> -Wgcc-compat warning for the cases that GCC doesn't allow. I also
>> >> >> wonder
>> >> >> whether there's any reason to restrict this to a DeclRefExpr, or
>> >> >> whether we
>> >> >> should just allow any expression of the right type. (If we make this
>> >> >> more
>> >> >> permissive, we'll need to document when the expression is evaluated)
>> >> >
>> >> > I've added another test file for ensuring we fire the -Wgcc-compat
>> >> > warnings as expected.  As for extending it to any expression of the
>> >> > right type, that is a bit further than I was looking to take this (I
>> >> > mostly wanted to get another case of unresolved identifiers out of
>> >> > the
>> >> > way for other attribute work).
>> >> >
>> >> > Thanks!
>> >> >
>> >> > ~Aaron
>> >> >
>> >> >>
>> >> >> On 4 Sep 2013 07:47, "Aaron Ballman" <aaron at aaronballman.com> wrote:
>> >> >>>
>> >> >>> The cleanup attribute was using an unresolved, simple identifier as
>> >> >>> its sole argument.  However, while processing the attribute, we
>> >> >>> would
>> >> >>> attempt to look up the simple identifier, flag its usage, etc as
>> >> >>> though it were a resolved identifier.  This patch removes the
>> >> >>> custom
>> >> >>> logic from SemaDeclAttr.cpp and simply uses a resolved identifier
>> >> >>> (DeclRefExpr) for the argument.  I've added some extra test cases
>> >> >>> since this expands what can be used as an argument to cleanup.
>> >> >>>
>> >> >>> ~Aaron
>> >
>> >
>
>