nothrow and const Attribtues

[Richard Smith]

On Mon, Dec 2, 2013 at 1:28 PM, Aaron Ballman <aaron at aaronballman.com>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. 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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131202/383c95e2/attachment.html>