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