[PATCH] D26657: [Sema] Respect DLL attributes more faithfully

Hans Wennborg via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Dec 5 09:12:14 PST 2016


hans accepted this revision.
hans added a comment.
This revision is now accepted and ready to land.

lgtm



================
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
----------------
smeenai wrote:
> 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.
Sounds good.


https://reviews.llvm.org/D26657





More information about the cfe-commits mailing list