<div dir="ltr"><div>Some trivial things:</div><div><br></div>+  for (const AttributeList* L = AttrList; L; L = L->getNext())<br><div><br></div><div>* goes on the right.</div><div><br></div><div><br></div><div>+  for (unsigned i = 0, e = PD.getNumTypeObjects(); i != e; ++i) {<br>
</div><div><br></div><div>'i' and 'e' should be capitalized.</div><div><br></div><div>You don't have any tests for the attribute being in the declarator chunks, rather than in the decl specifiers. Please add such tests.</div>
<div class="gmail_extra"><br></div><div class="gmail_extra"><br></div><div class="gmail_extra"><div class="gmail_extra">+  // Ensure that dllimport globals without explicit storage class are treated as</div><div class="gmail_extra">
+  // extern. The storage class is set above using parsed attributes. Now we can</div><div class="gmail_extra">+  // check the VarDecl itself.</div><div class="gmail_extra">+  if (const DLLImportAttr *DA = NewVD->getAttr<DLLImportAttr>()) {</div>
<div class="gmail_extra"><div class="gmail_extra">+    assert(DA->isInherited() || NewVD->isStaticDataMember() ||</div><div class="gmail_extra">+           NewVD->getStorageClass() != SC_None);</div><div class="gmail_extra">
+  }</div><div class="gmail_extra"><br></div></div><div class="gmail_extra">Move the getAttr into the assert, so we trivially remove the whole thing in an NDEBUG build.</div><div class="gmail_extra"><br></div><div class="gmail_extra">
<br></div><div class="gmail_quote">On Thu, Mar 20, 2014 at 4:41 PM, Nico Rieck <span dir="ltr"><<a href="mailto:nico.rieck@gmail.com" target="_blank">nico.rieck@gmail.com</a>></span> wrote:<br><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">
<div class="">On 05.03.2014 19:43, Richard Smith wrote:<br>
> If we don't adjust the DeclContext, I don't think we'll reject ill-formed<br>
> code such as this:<br>
><br>
> float x;<br>
> void f() {<br>
>    __declspec(dllimport) int x;<br>
> }<br>
<br>
</div>This test fails with the original patch, but this problem can be solved<br>
by setting the storage class a bit earlier even without setting the<br>
correct DeclContext.</blockquote><div><br></div><div>Setting the storage class earlier (as this patch does) is causing is to set the correct DeclContext. So that's why everything's working =)</div><div> </div><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">
I've restructured the patch to look at the parsed attributes to get the<br>
correct DeclContext. Another option I explored was using setDeclContext<br>
on the VarDecl. This passes all tests but I'm more reluctant to do this.</blockquote><div><br></div><div>Thanks, the approach in the revised patch seems fine to me.</div></div></div></div>