<div dir="ltr"><div class="gmail_extra">On Mon, Dec 2, 2013 at 2:25 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div class="h5">On Mon, Dec 2, 2013 at 1:28 PM, Aaron Ballman <span dir="ltr"><<a href="mailto:aaron@aaronballman.com" target="_blank">aaron@aaronballman.com</a>></span> wrote:<br>
</div></div><div class="gmail_extra"><div class="gmail_quote"><div><div class="h5">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">The nothrow and const attributes have a curious construct that does<br>


not appear with any other attributes. Both of them check whether the<br>
attribute has already been applied, and if it has, it silently fails<br>
to reapply the attribute.  Eg)<br>
<br>
  if (NoThrowAttr *Existing = D->getAttr<NoThrowAttr>()) {<br>
    if (Existing->getLocation().isInvalid())<br>
      Existing->setRange(Attr.getRange());<br>
  } else {<br>
    D->addAttr(::new (S.Context)<br>
               NoThrowAttr(Attr.getRange(), S.Context,<br>
                           Attr.getAttributeSpellingListIndex()));<br>
  }<br>
<br>
While this makes some sense, two things in particular are worrisome:<br>
<br>
1) We lose syntactic information because we are losing the as-written<br>
attribute information. This happens relatively frequently, but any<br>
time we can cut that down would be good.<br>
2) This is not consistently applied across attributes.<br>
<br>
I am wondering whether it is reasonable to remove the check for the<br>
existing attribute from the nothrow and const attributes, and simply<br>
apply multiple times. Since neither of these attributes accept<br>
arguments, it seems like this would be a harmless action. However, I<br>
may not have the full picture; this is the original commit message:<br>
<br>
dgregor  6/15/2011 1:45:11 AM<br>
Don't add redundant FormatAttr, ConstAttr, or NoThrowAttr attributes,<br>
either imlicitly (for builtins) or explicitly (due to multiple<br>
specification of the same attributes). Fixes <rdar://problem/9612060>.<br>
<br>
(Note, I am not suggesting any modifications for FormatAttr.)<br>
<br>
Thanks!</blockquote><div><br></div></div></div><div>Here's the testcase from that change:</div><div><br></div><div><div>+int printf(const char * restrict, ...) __attribute__((__format__ (__printf__, 1, 2)));</div><div>
+</div><div>
+void rdar9612060(void) {</div><div>+  printf("%s", 2); // expected-warning{{conversion specifies type 'char *' but the argument has type 'int'}}</div><div>+}</div></div><div><br></div><div>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.</div>
</div></div></div></blockquote><div><br></div><div>(In case it's not obvious, if we get two FormatAttrs on the same FunctionDecl, we'll emit two warnings instead of one.)</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>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.</div>

</div></div></div>
</blockquote></div><br></div></div>