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 <[email protected]> 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 <[email protected]> > 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 <[email protected]> >> 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 <[email protected]> >> > wrote: >> >> >> >> Ping >> >> >> >> On Wed, Sep 4, 2013 at 3:08 PM, Aaron Ballman <[email protected]> >> >> wrote: >> >> > On Wed, Sep 4, 2013 at 1:35 PM, Richard Smith <[email protected]> >> >> > 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" <[email protected]> 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 >> > >> > > > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
