[PATCH] Cleanup attribute using resolved identifiers

[Richard Smith]

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
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130910/994fe74e/attachment.html>