[PATCH] D22034: [MSVC][DLL] use local vftable mangling only exported classes with virtual destructor

Reid Kleckner via cfe-commits cfe-commits at lists.llvm.org
Tue Jul 19 07:05:49 PDT 2016


rnk added inline comments.

================
Comment at: test/CodeGenCXX/dllimport-rtti.cpp:7
@@ -6,3 +6,1 @@
 } s;
-// MSVC: [[VF_S:.*]] = private unnamed_addr constant [2 x i8*]
-// MSVC-DAG: @"\01??_SS@@6B@" = unnamed_addr alias i8*, getelementptr inbounds ([2 x i8*], [2 x i8*]* [[VF_S]], i32 0, i32 1)
----------------
DmitryPolukhin wrote:
> rnk wrote:
> > I would've expected this to remain the same, since the implicit default ctor of 'S' is constexpr by default in C++14. It seems a lot better to emit a local vftable here and get static initialization for 's' than dynamic initialization.
> The context of evaluation of the whole expression is not constexpr so this case can be done both ways. But implemented approach is how MSVC behaves in this case. MSVC has very predictable behavior when local vftable is used - only when class has virtual d-tor. Current Clang behavior is also very consistent - always use local vftable. But this patch makes it hard to predict which table will be used - it becomes use context sensitive instead of depending only on class declaration. Therefore different translation units could use different tables and it could cause strange artifacts. Using dllimported classes in pure constexpr case in my experience is very rare case so it was kind of fine but implicit constructors much more common case.
> 
> Also thinking more about my patch I realized that fix in MayBeEmittedEagerly doesn't work if dllimported class is a member of non-imported class so actual fix would require traversing for all base classes and members for the type.
> 
> So my proposal is to keep things as is for now and abandon this patch if you have no objection.
Sounds good, we also thought this was a reasonable compromise position last time we touched this.


https://reviews.llvm.org/D22034





More information about the cfe-commits mailing list