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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
