On Fri, May 3, 2013 at 10:02 AM, Reid Kleckner <span dir="ltr"><<a href="mailto:rnk@google.com" target="_blank">rnk@google.com</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">
Cool!  There's two instances of trailing whitespace you'll want to fix.<br>
<br>
-  if (!isa<FunctionDecl>(D)) {<br>
+  // Microsoft mode expects the naked attribute to only be applied to a<br>
+  // function definition, or else it causes an error.  In non-Microsoft mode,<br>
+  // the attribute is allowed to be attached to a function definition or is<br>
+  // otherwise warned about.<br>
+  //<br>
+  // Because the attribute is handled before the function body is parsed, try<br>
+  // to use the Declarator to determine whether this is a function definition<br>
+  // or not.<br>
+  bool IsFunctionDecl = isa<FunctionDecl>(D);<br>
+  if (S.LangOpts.MicrosoftMode &&<br>
+      (!IsFunctionDecl || (PD && !PD->isFunctionDefinition()))) {<br>
<br>
I'm trying to find a way to avoid passing the Declarator through to<br>
here, but I assume you've looked and I can't find anything either...<br>
All of the definition-related checks rely on Body which isn't set<br>
until the body parsing is finished.<br>
<br>
If it were a TagDecl we could say 'D->isBeingDefined()', but for<br>
functions it looks like we haven't needed that.  Maybe it's worth<br>
adding a IsDefinition or BeingDefined bit to FunctionDecl?  Would<br>
anyone else find that useful or is it a bad idea?<br></blockquote><div><br></div><div>This seems reasonable to me, although I'm not sure how generally useful it would be.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

+    S.Diag(Attr.getLoc(), diag::err_attribute_wrong_decl_type)<br>
+      << Attr.getName() << ExpectedFunctionDefinition;<br>
+    return;<br>
+  } else if (!IsFunctionDecl) {<br>
     S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type)<br>
       << Attr.getName() << ExpectedFunction;<br>
     return;<br>
@@ -4699,7 +4715,8 @@<br>
<div><div class="h5"><br>
<br>
On Fri, May 3, 2013 at 9:09 AM, Aaron Ballman <<a href="mailto:aaron@aaronballman.com">aaron@aaronballman.com</a>> wrote:<br>
> MSVC handles the naked attribute differently than clang does.<br>
> Specifically, in MSVC the __declspec(naked) must attach to a function<br>
> *definition*, and failure to do so will result in an error.<br>
><br>
> This patch brings clang's behavior more in line with MSVC's for<br>
> __declspec(naked) in MS compatibility mode.  This does not modify the<br>
> behavior of the codegen for naked functions (as is being discussed<br>
> elsewhere).<br>
><br>
> ~Aaron<br>
><br>
</div></div>> _______________________________________________<br>
> cfe-commits mailing list<br>
> <a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
><br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br>