[PATCH] Sema: Treat dllimport globals without explicit storage class as extern

[Richard Smith]

Some trivial things:

+  for (const AttributeList* L = AttrList; L; L = L->getNext())

* goes on the right.


+  for (unsigned i = 0, e = PD.getNumTypeObjects(); i != e; ++i) {

'i' and 'e' should be capitalized.

You don't have any tests for the attribute being in the declarator chunks,
rather than in the decl specifiers. Please add such tests.


+  // Ensure that dllimport globals without explicit storage class are
treated as
+  // extern. The storage class is set above using parsed attributes. Now
we can
+  // check the VarDecl itself.
+  if (const DLLImportAttr *DA = NewVD->getAttr<DLLImportAttr>()) {
+    assert(DA->isInherited() || NewVD->isStaticDataMember() ||
+           NewVD->getStorageClass() != SC_None);
+  }

Move the getAttr into the assert, so we trivially remove the whole thing in
an NDEBUG build.


On Thu, Mar 20, 2014 at 4:41 PM, Nico Rieck <nico.rieck at gmail.com> wrote:

> On 05.03.2014 19:43, Richard Smith wrote:
> > If we don't adjust the DeclContext, I don't think we'll reject ill-formed
> > code such as this:
> >
> > float x;
> > void f() {
> >    __declspec(dllimport) int x;
> > }
>
> This test fails with the original patch, but this problem can be solved
> by setting the storage class a bit earlier even without setting the
> correct DeclContext.


Setting the storage class earlier (as this patch does) is causing is to set
the correct DeclContext. So that's why everything's working =)


> I've restructured the patch to look at the parsed attributes to get the
> correct DeclContext. Another option I explored was using setDeclContext
> on the VarDecl. This passes all tests but I'm more reluctant to do this.


Thanks, the approach in the revised patch seems fine to me.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140320/5b3ffc41/attachment.html>