]> granicus.if.org Git - clang/commitdiff
Don't let virtual calls and dynamic casts call Sema::MarkVTableUsed().
authorNico Weber <nicolasweber@gmx.de>
Mon, 26 Jan 2015 06:23:36 +0000 (06:23 +0000)
committerNico Weber <nicolasweber@gmx.de>
Mon, 26 Jan 2015 06:23:36 +0000 (06:23 +0000)
clang currently calls MarkVTableUsed() for classes that get their virtual
methods called or that participate in a dynamic_cast. This is unnecessary,
since CodeGen only emits vtables when it generates constructor, destructor, and
vtt code. (*)

Note that Sema::MarkVTableUsed() doesn't cause the emission of a vtable.
Its main user-visible effect is that it instantiates virtual member functions
of template classes, to make sure that if codegen decides to write a vtable
all the entries in the vtable are defined.

While this shouldn't change the behavior of codegen (other than being faster),
it does make clang more permissive: virtual methods of templates (in particular
destructors) end up being instantiated less often. In particular, classes that
have members that are smart pointers to incomplete types will now get their
implicit virtual destructor instantiated less frequently. For example, this
used to not compile but does now compile:

    template <typename T> struct OwnPtr {
      ~OwnPtr() { static_assert((sizeof(T) > 0), "TypeMustBeComplete"); }
    };
    class ScriptLoader;
    struct Base { virtual ~Base(); };
    struct Sub : public Base {
      virtual void someFun() const {}
      OwnPtr<ScriptLoader> m_loader;
    };
    void f(Sub *s) { s->someFun(); }

The more permissive behavior matches both gcc (where this is not often
observable, since in practice most things with virtual methods have a key
function, and Sema::DefineUsedVTables() skips vtables for classes with key
functions) and cl (which is my motivation for this change) – this fixes
PR20337.  See this issue and the review thread for some discussions about
optimizations.

This is similar to r213109 in spirit. r225761 was a prerequisite for this
change.

Various tests relied on "a->f()" marking a's vtable as used (in the sema
sense), switch these to just construct a on the stack. This forces
instantiation of the implicit constructor, which will mark the vtable as used.

(*) The exception is -fapple-kext mode: In this mode, qualified calls to
virtual functions (`a->Base::f()`) still go through the vtable, and since the
vtable pointer off this doesn't point to Base's vtable, this needs to reference
Base's vtable directly. To keep this working, keep referencing the vtable for
virtual calls in apple kext mode.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@227073 91177308-0d34-0410-b5e6-96231b3b80d8

12 files changed:
include/clang/Sema/Sema.h
lib/Sema/Sema.cpp
lib/Sema/SemaCast.cpp
lib/Sema/SemaDeclCXX.cpp
lib/Sema/SemaExpr.cpp
lib/Sema/SemaInit.cpp
test/CXX/special/class.dtor/p9.cpp
test/CodeGenCXX/key-function-vtable.cpp
test/SemaCXX/devirtualize-vtable-marking.cpp
test/SemaCXX/microsoft-dtor-lookup.cpp
test/SemaCXX/warn-weak-vtables.cpp
test/SemaTemplate/virtual-member-functions.cpp

index a53eda41e1f8e1e82cef168c3ec97e5b4fef87eb..f6d185e491684d6b285905213286f0891d07e144 100644 (file)
@@ -5182,8 +5182,6 @@ public:
   // FIXME: I don't like this name.
   void BuildBasePathArray(const CXXBasePaths &Paths, CXXCastPath &BasePath);
 
-  bool BasePathInvolvesVirtualBase(const CXXCastPath &BasePath);
-
   bool CheckDerivedToBaseConversion(QualType Derived, QualType Base,
                                     SourceLocation Loc, SourceRange Range,
                                     CXXCastPath *BasePath = nullptr,
index b9aaf1616d68f9cf94939b99ae590be231c75c90..ded6c303af85190c4106c75786984ba5f646dc88 100644 (file)
@@ -337,18 +337,6 @@ ExprResult Sema::ImpCastExprToType(Expr *E, QualType Ty,
   if (ExprTy == TypeTy)
     return E;
 
-  // If this is a derived-to-base cast to a through a virtual base, we
-  // need a vtable.
-  if (Kind == CK_DerivedToBase &&
-      BasePathInvolvesVirtualBase(*BasePath)) {
-    QualType T = E->getType();
-    if (const PointerType *Pointer = T->getAs<PointerType>())
-      T = Pointer->getPointeeType();
-    if (const RecordType *RecordTy = T->getAs<RecordType>())
-      MarkVTableUsed(E->getLocStart(),
-                     cast<CXXRecordDecl>(RecordTy->getDecl()));
-  }
-
   if (ImplicitCastExpr *ImpCast = dyn_cast<ImplicitCastExpr>(E)) {
     if (ImpCast->getCastKind() == Kind && (!BasePath || BasePath->empty())) {
       ImpCast->setType(Ty);
index a4c2d9b51c2632e67f6d4e0fbfcc7124c76f5fc0..3e06a6477d640725a4f57495cf52647e835442a7 100644 (file)
@@ -665,12 +665,6 @@ void CastOperation::CheckDynamicCast() {
     }
 
     Kind = CK_DerivedToBase;
-
-    // If we are casting to or through a virtual base class, we need a
-    // vtable.
-    if (Self.BasePathInvolvesVirtualBase(BasePath))
-      Self.MarkVTableUsed(OpRange.getBegin(), 
-                          cast<CXXRecordDecl>(SrcRecord->getDecl()));
     return;
   }
 
@@ -682,8 +676,6 @@ void CastOperation::CheckDynamicCast() {
       << SrcPointee.getUnqualifiedType() << SrcExpr.get()->getSourceRange();
     SrcExpr = ExprError();
   }
-  Self.MarkVTableUsed(OpRange.getBegin(), 
-                      cast<CXXRecordDecl>(SrcRecord->getDecl()));
 
   // dynamic_cast is not available with -fno-rtti.
   // As an exception, dynamic_cast to void* is available because it doesn't
index 1b9ccee9dbee38dd42d7769f290245df7e511058..79732c354cad23682f87a63b0d7416631de050e3 100644 (file)
@@ -1745,18 +1745,6 @@ void Sema::BuildBasePathArray(const CXXBasePaths &Paths,
     BasePathArray.push_back(const_cast<CXXBaseSpecifier*>(Path[I].Base));
 }
 
-/// \brief Determine whether the given base path includes a virtual
-/// base class.
-bool Sema::BasePathInvolvesVirtualBase(const CXXCastPath &BasePath) {
-  for (CXXCastPath::const_iterator B = BasePath.begin(), 
-                                BEnd = BasePath.end();
-       B != BEnd; ++B)
-    if ((*B)->isVirtual())
-      return true;
-
-  return false;
-}
-
 /// CheckDerivedToBaseConversion - Check whether the Derived-to-Base
 /// conversion (where Derived and Base are class types) is
 /// well-formed, meaning that the conversion is unambiguous (and
index 4114b5671363c0c370458dc3d152a404c7b40a86..8b9950847f5b2aeaa17d05c611164a94d5584fb5 100644 (file)
@@ -11621,7 +11621,7 @@ void Sema::MarkFunctionReferenced(SourceLocation Loc, FunctionDecl *Func,
     Destructor = cast<CXXDestructorDecl>(Destructor->getFirstDecl());
     if (Destructor->isDefaulted() && !Destructor->isDeleted())
       DefineImplicitDestructor(Loc, Destructor);
-    if (Destructor->isVirtual())
+    if (Destructor->isVirtual() && getLangOpts().AppleKext)
       MarkVTableUsed(Loc, Destructor->getParent());
   } else if (CXXMethodDecl *MethodDecl = dyn_cast<CXXMethodDecl>(Func)) {
     if (MethodDecl->isOverloadedOperator() &&
@@ -11641,7 +11641,7 @@ void Sema::MarkFunctionReferenced(SourceLocation Loc, FunctionDecl *Func,
         DefineImplicitLambdaToBlockPointerConversion(Loc, Conversion);
       else
         DefineImplicitLambdaToFunctionPointerConversion(Loc, Conversion);
-    } else if (MethodDecl->isVirtual())
+    } else if (MethodDecl->isVirtual() && getLangOpts().AppleKext)
       MarkVTableUsed(Loc, MethodDecl->getParent());
   }
 
index e7a8e9cacde1508e29f5c12032583edc1306d0a0..9aca373d97b7232efccf66f6c9f6faf887229b28 100644 (file)
@@ -5880,15 +5880,6 @@ InitializationSequence::Perform(Sema &S,
                                          &BasePath, IgnoreBaseAccess))
         return ExprError();
 
-      if (S.BasePathInvolvesVirtualBase(BasePath)) {
-        QualType T = SourceType;
-        if (const PointerType *Pointer = T->getAs<PointerType>())
-          T = Pointer->getPointeeType();
-        if (const RecordType *RecordTy = T->getAs<RecordType>())
-          S.MarkVTableUsed(CurInit.get()->getLocStart(),
-                           cast<CXXRecordDecl>(RecordTy->getDecl()));
-      }
-
       ExprValueKind VK =
           Step->Kind == SK_CastDerivedToBaseLValue ?
               VK_LValue :
index a03fcdb24952ba71b487db65241b89558fab0195..42a4236a4a079ea3acf2d6f248fe0b07071ac806 100644 (file)
@@ -89,4 +89,11 @@ namespace test3 {
     virtual ~B() {}
     static void operator delete(void*);
   };
+
+  void f() {
+#ifdef MSABI
+    // expected-note@+2 {{implicit default constructor for 'test3::B' first required here}}
+#endif
+    B use_vtable;
+  }
 }
index c443b5277e449d3508a985d90e48d66ffb1a6a60..80ce497505dcfb84d22cd32655575d7e1abdccb8 100644 (file)
@@ -41,7 +41,7 @@ struct X1 : X0 {
 
 inline void X1::f() { }
 
-void use_X1(X1 *x1) { x1->f(); }
+void use_X1() { X1 x1; }
 
 // FIXME: The checks are extremely difficult to get right when the globals
 // aren't alphabetized
index fc3e8ce7704c49c953216990724adc208a7aff08..1b32182c4f2ef6ce4188a2d7aa0cfc009009f58e 100644 (file)
@@ -1,29 +1,32 @@
 // RUN: %clang_cc1 -verify -std=c++11 %s
-
+// expected-no-diagnostics
 template <typename T> struct OwnPtr {
   T *p;
   ~OwnPtr() {
-    // expected-error@+1 {{invalid application of 'sizeof'}}
     static_assert(sizeof(T) > 0, "incomplete T");
     delete p;
   }
 };
 
 namespace use_vtable_for_vcall {
-struct Incomplete; // expected-note {{forward declaration}}
+struct Incomplete;
 struct A {
   virtual ~A() {}
   virtual void m() {}
 };
-struct B : A { // expected-note {{in instantiation}}
+struct B : A {
   B();
   virtual void m() { }
   virtual void m2() { static_cast<A *>(this)->m(); }
   OwnPtr<Incomplete> m_sqlError;
 };
 
-B *f() {
-  return new B();
+void f() {
+  // Since B's constructor is declared out of line, nothing in this file
+  // references a vtable, so the destructor doesn't get built.
+  A *b = new B();
+  b->m();
+  delete b;
 }
 }
 
index 412749f707ee4735be0e65b61eb6e12426c6ba7e..312598e286398282a32c38aa79c875f147afc892 100644 (file)
@@ -23,8 +23,9 @@ struct VC : A, B {
   virtual ~VC(); // expected-error {{member 'operator delete' found in multiple base classes of different types}}
 };
 
-void f(VC vc) {
+void f() {
   // This marks VC's vtable used.
+  VC vc;
 }
 
 }
index 671ff297cfa15d97e2b639095be6ba702528faba..d30665387153144e925c5eac72271841a410ca48 100644 (file)
@@ -20,15 +20,14 @@ void f() {
     virtual void f() { }
   };
 
-  A *a;
-  a->f();
+  A a;
 }
 
 // Use the vtables
-void uses(A &a, B<int> &b, C &c) {
-  a.f();
-  b.f();
-  c.f();
+void uses_abc() {
+  A a;
+  B<int> b;
+  C c;
 }
 
 // <rdar://problem/9979458>
@@ -52,10 +51,9 @@ public:
 
 Parent::~Parent() {}
 
-void uses(Parent &p, Derived &d, VeryDerived &vd) {
-  p.getFoo();
-  d.getFoo();
-  vd.getFoo();
+void uses_derived() {
+  Derived d;
+  VeryDerived vd;
 }
 
 template<typename T> struct TemplVirt {
@@ -72,8 +70,8 @@ template<> struct TemplVirt<long> { // expected-warning{{'TemplVirt<long>' has n
   virtual void f() {}
 };
 
-void uses(TemplVirt<float>& f, TemplVirt<bool>& b, TemplVirt<long>& l) {
-  f.f();
-  b.f();
-  l.f();
+void uses_templ() {
+  TemplVirt<float> f;
+  TemplVirt<bool> b;
+  TemplVirt<long> l;
 }
index c2fc2634edd631f5ca0a8b2845515cd46126ed56..4044f9e513db03cf08d513a0d1ba8ca026304387 100644 (file)
@@ -110,12 +110,12 @@ namespace PR7114 {
 namespace DynamicCast {
   struct Y {};
   template<typename T> struct X : virtual Y {
-    virtual void foo() { T x; } // expected-error {{variable has incomplete type 'void'}}
+    virtual void foo() { T x; }
   };
   template<typename T> struct X2 : virtual Y {
     virtual void foo() { T x; }
   };
-  Y* f(X<void>* x) { return dynamic_cast<Y*>(x); } // expected-note {{in instantiation of member function 'DynamicCast::X<void>::foo' requested here}}
+  Y* f(X<void>* x) { return dynamic_cast<Y*>(x); }
   Y* f2(X<void>* x) { return dynamic_cast<Y*>(x); }
 }