<div dir="ltr"><div>Can you confirm my understanding:<br></div><div> * for attributes that don't have an 'identifier, expression...' form, we use getArgAsExpr and get an assert if the argument is not an expression.</div>
<div> * 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.</div><div><br></div><div>
(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!)</div>
<div><br></div><div>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.</div></div><div class="gmail_extra">
<br><br><div class="gmail_quote">On Fri, Aug 30, 2013 at 3:57 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">
This patch consolidates the notion of a GNU attribute parameter with<br>
the argument list.  This makes for considerably easier error reporting<br>
in the long run since the attribute arguments specified in the td file<br>
match the arguments passed in via the AttributeList.<br>
<br>
It accomplishes this by using a PointerUnion of identifier information<br>
and expressions.  While the patch is pretty large, the vast majority<br>
of the changes come from using this discriminated union where we used<br>
to look for a parameter or an expression specifically.<br>
<span class="HOEnZb"><font color="#888888"><br>
~Aaron<br>
</font></span></blockquote></div><br></div>