On Mon, Dec 2, 2013 at 2:25 PM, Richard Smith <[email protected]> wrote:
> On Mon, Dec 2, 2013 at 1:28 PM, Aaron Ballman <[email protected]>wrote: > >> The nothrow and const attributes have a curious construct that does >> not appear with any other attributes. Both of them check whether the >> attribute has already been applied, and if it has, it silently fails >> to reapply the attribute. Eg) >> >> if (NoThrowAttr *Existing = D->getAttr<NoThrowAttr>()) { >> if (Existing->getLocation().isInvalid()) >> Existing->setRange(Attr.getRange()); >> } else { >> D->addAttr(::new (S.Context) >> NoThrowAttr(Attr.getRange(), S.Context, >> Attr.getAttributeSpellingListIndex())); >> } >> >> While this makes some sense, two things in particular are worrisome: >> >> 1) We lose syntactic information because we are losing the as-written >> attribute information. This happens relatively frequently, but any >> time we can cut that down would be good. >> 2) This is not consistently applied across attributes. >> >> I am wondering whether it is reasonable to remove the check for the >> existing attribute from the nothrow and const attributes, and simply >> apply multiple times. Since neither of these attributes accept >> arguments, it seems like this would be a harmless action. However, I >> may not have the full picture; this is the original commit message: >> >> dgregor 6/15/2011 1:45:11 AM >> Don't add redundant FormatAttr, ConstAttr, or NoThrowAttr attributes, >> either imlicitly (for builtins) or explicitly (due to multiple >> specification of the same attributes). Fixes <rdar://problem/9612060>. >> >> (Note, I am not suggesting any modifications for FormatAttr.) >> >> Thanks! > > > Here's the testcase from that change: > > +int printf(const char * restrict, ...) __attribute__((__format__ > (__printf__, 1, 2))); > + > +void rdar9612060(void) { > + printf("%s", 2); // expected-warning{{conversion specifies type 'char > *' but the argument has type 'int'}} > +} > > It seems the problem that this change was trying to avoid was where an > attribute is implicitly added because the function is a builtin, and then > explicitly added. > (In case it's not obvious, if we get two FormatAttrs on the same FunctionDecl, we'll emit two warnings instead of one.) > I suspect this is redundant when initially building the attribute -- > mergeDeclAttributes will not propagate the attribute from the > implicitly-declared builtin onto the explicitly-declared function. >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
