<div dir="ltr">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).<div>
<br></div><div>[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.]</div>
</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Sep 10, 2013 at 3:52 PM, Aaron Ballman <span dir="ltr"><<a href="mailto:aaron@aaronballman.com" target="_blank">aaron@aaronballman.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Since there were some reasonable substantial changes to handle<br>
explicit template arguments, I am resubmitting for a final look-over.<br>
<br>
Thanks!<br>
<span class="HOEnZb"><font color="#888888"><br>
~Aaron<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
On Tue, Sep 10, 2013 at 4:59 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>> wrote:<br>
> Your -Wgcc-compat check for DRE->hasQualifier() should also check for<br>
> DRE->hasExplicitTemplateArgs(). With that and a test for it, LGTM.<br>
><br>
><br>
> On Mon, Sep 9, 2013 at 5:47 AM, Aaron Ballman <<a href="mailto:aaron@aaronballman.com">aaron@aaronballman.com</a>><br>
> wrote:<br>
>><br>
>> Ping<br>
>><br>
>> On Wed, Sep 4, 2013 at 3:08 PM, Aaron Ballman <<a href="mailto:aaron@aaronballman.com">aaron@aaronballman.com</a>><br>
>> wrote:<br>
>> > On Wed, Sep 4, 2013 at 1:35 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>><br>
>> > wrote:<br>
>> >> This goes beyond what GCC supports: there, the attribute must be given<br>
>> >> a<br>
>> >> simple identifier. I think this extension is reasonable, but please add<br>
>> >> a<br>
>> >> -Wgcc-compat warning for the cases that GCC doesn't allow. I also<br>
>> >> wonder<br>
>> >> whether there's any reason to restrict this to a DeclRefExpr, or<br>
>> >> whether we<br>
>> >> should just allow any expression of the right type. (If we make this<br>
>> >> more<br>
>> >> permissive, we'll need to document when the expression is evaluated)<br>
>> ><br>
>> > I've added another test file for ensuring we fire the -Wgcc-compat<br>
>> > warnings as expected.  As for extending it to any expression of the<br>
>> > right type, that is a bit further than I was looking to take this (I<br>
>> > mostly wanted to get another case of unresolved identifiers out of the<br>
>> > way for other attribute work).<br>
>> ><br>
>> > Thanks!<br>
>> ><br>
>> > ~Aaron<br>
>> ><br>
>> >><br>
>> >> On 4 Sep 2013 07:47, "Aaron Ballman" <<a href="mailto:aaron@aaronballman.com">aaron@aaronballman.com</a>> wrote:<br>
>> >>><br>
>> >>> The cleanup attribute was using an unresolved, simple identifier as<br>
>> >>> its sole argument.  However, while processing the attribute, we would<br>
>> >>> attempt to look up the simple identifier, flag its usage, etc as<br>
>> >>> though it were a resolved identifier.  This patch removes the custom<br>
>> >>> logic from SemaDeclAttr.cpp and simply uses a resolved identifier<br>
>> >>> (DeclRefExpr) for the argument.  I've added some extra test cases<br>
>> >>> since this expands what can be used as an argument to cleanup.<br>
>> >>><br>
>> >>> ~Aaron<br>
><br>
><br>
</div></div></blockquote></div><br></div>