[PATCH] D20608: clang-cl: Treat dllimport explicit template instantiation definitions as declarations (PR27810, PR27811)

[Nico Weber via cfe-commits]

thakis added a comment.

Looks great, thanks! A few minor questions below.

I verified that this has the same effect as my brute-force patch I tried locally.

Do we have test coverage for `template class __declspec(dllexport) codecvt<char>;` somewhere already?


================
Comment at: lib/Sema/SemaTemplate.cpp:7382
@@ +7381,3 @@
+      if (A->getKind() == AttributeList::AT_DLLExport)
+        DLLImport = false;
+    }
----------------
If there are multiple dllexports and dllimports on an explicit instantiation, cl.exe just silently picks the last one?

================
Comment at: lib/Sema/SemaTemplate.cpp:7467
@@ +7466,3 @@
+    // The new specialization might add a dllimport attribute.
+    HasNoEffect = false;
+  }
----------------
HasNoEffect is read two times before it's updated here. Is it intentional that you only change it after it's been read twice? If so, maybe add a comment why, since it looks not intentional else. (And make sure there's test coverage for setting it at the right time)


http://reviews.llvm.org/D20608