smeenai added inline comments.

================
Comment at: lib/Sema/SemaTemplate.cpp:7710
+        (Context.getTargetInfo().getCXXABI().isMicrosoft() ||
+         Context.getTargetInfo().getTriple().isWindowsItaniumEnvironment())) {
+      // In the MS ABI, an explicit instantiation definition can add a dll
----------------
hans wrote:
> smeenai wrote:
> > hans wrote:
> > > Why the isWindowsItaniumEnvironment check? I'd expect checking for the MS 
> > > ABI is sufficient?
> > windows-itanium in general tries to stick to MSVC semantics for 
> > dllexport/import annotations (unlike Cygwin and MinGW which kinda do their 
> > own thing). This is consistent with the conditional for the previous case 
> > (lines 7691 to 7693 in this diff).
> Oh I see, this seems to be a new thing, starting with e.g. r284288.
> 
> Seems fine then, but I'm a little worried that we're adding another variable 
> into the matrix here. IIRC, we key dll attribute behaviour off 
> `getCXXABI().isMicrosoft()` in lots of places.
You're right; the current situation isn't ideal. I can clean that up in a 
follow-up.


https://reviews.llvm.org/D26657



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to