[PATCH] Removing the notion of an attribute parameter

[Aaron Ballman]

On Fri, Aug 30, 2013 at 8:06 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> Can you confirm my understanding:
>  * for attributes that don't have an 'identifier, expression...' form, we
> use getArgAsExpr and get an assert if the argument is not an expression.

Correct -- getArgAsExpr uses PorinterUnion::get internally, which asserts.

>  * for attributes that might have an 'identifier, expression...' form, the
> parser will sometimes give us an 'expression...' form instead, so those
> cases always check isArgExpr.

Yes, I believe I've hit all of the places that need an isArgFoo check.
 I added some test cases where I felt there wasn't good enough
coverage, too.

> (Given that, it seems surprising to me that alias, weakref, and the
> 'handleAttrWithMessage' need the isArgExpr check. But it looks like that's
> because we have some confusion over whether a StringArgument represents a
> string literal or an identifier, and I don't want this patch blocked on
> sorting that out!)

the isArgExpr check is for the case where the user does put in an
identifier and we want to diagnose it.  Eg)

__attribute__((alias(bad_thing)))

vs

__attribute__((alias("bad_thing")))

> You have "else if" after an "if" that returns in a few places in
> SemaDeclAttr. We prefer to drop the 'else' in this case. Otherwise, LGTM.

Okay, I can fix those up.  Assuming those are corrected, are you fine
with me committing?

Thanks!

~Aaron