Check that classes with non-trivial destructors have finalization support.

Uses clang's notion of when a class has a 'trivial destructor' to determine when garbage-collected types need finalization.

BUG=334149
NOTRY=true

Review URL: https://codereview.chromium.org/192933002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@257072 0039d316-1c4b-4281-b951-d872f2087c98

20 files changed, 491 insertions, 70 deletions

diff --git a/tools/clang/blink_gc_plugin/BlinkGCPlugin.cpp b/tools/clang/blink_gc_plugin/BlinkGCPlugin.cpp
index 15ce1ae..73cf38f 100644
--- a/tools/clang/blink_gc_plugin/BlinkGCPlugin.cpp
+++ b/tools/clang/blink_gc_plugin/BlinkGCPlugin.cpp
@@ -41,8 +41,8 @@ const char kClassContainsInvalidFields[] =
 const char kClassContainsGCRoot[] =
     "[blink-gc] Class %0 contains GC root in field %1.";
 
-const char kFinalizerInNonFinalizedClass[] =
-    "[blink-gc] Non-finalized class %0 has a user-declared finalizer %1.";
+const char kClassRequiresFinalization[] =
+    "[blink-gc] Class %0 requires finalization.";
 
 const char kFinalizerAccessesFinalizedField[] =
     "[blink-gc] Finalizer %0 accesses potentially finalized field %1.";
@@ -65,6 +65,15 @@ const char kFieldContainsGCRoot[] =
 const char kFinalizedFieldNote[] =
     "[blink-gc] Potentially finalized field %0 declared here:";
 
+const char kUserDeclaredDestructorNote[] =
+    "[blink-gc] User-declared destructor declared here:";
+
+const char kBaseRequiresFinalizationNote[] =
+    "[blink-gc] Base class %0 requiring finalization declared here:";
+
+const char kFieldRequiresFinalizationNote[] =
+    "[blink-gc] Field %0 requiring finalization declared here:";
+
 struct BlinkGCPluginOptions {
   BlinkGCPluginOptions() : enable_oilpan(false) {}
   bool enable_oilpan;
@@ -409,8 +418,8 @@ class BlinkGCPluginConsumer : public ASTConsumer {
                                     kClassContainsInvalidFields);
     diag_class_contains_gc_root_ =
         diagnostic_.getCustomDiagID(getErrorLevel(), kClassContainsGCRoot);
-    diag_finalizer_in_nonfinalized_class_ = diagnostic_.getCustomDiagID(
-        getErrorLevel(), kFinalizerInNonFinalizedClass);
+    diag_class_requires_finalization_ = diagnostic_.getCustomDiagID(
+        getErrorLevel(), kClassRequiresFinalization);
     diag_finalizer_accesses_finalized_field_ = diagnostic_.getCustomDiagID(
         getErrorLevel(), kFinalizerAccessesFinalizedField);
 
@@ -429,6 +438,12 @@ class BlinkGCPluginConsumer : public ASTConsumer {
         DiagnosticsEngine::Note, kFieldContainsGCRoot);
     diag_finalized_field_note_ = diagnostic_.getCustomDiagID(
         DiagnosticsEngine::Note, kFinalizedFieldNote);
+    diag_user_declared_destructor_note_ = diagnostic_.getCustomDiagID(
+        DiagnosticsEngine::Note, kUserDeclaredDestructorNote);
+    diag_base_requires_finalization_note_ = diagnostic_.getCustomDiagID(
+        DiagnosticsEngine::Note, kBaseRequiresFinalizationNote);
+    diag_field_requires_finalization_note_ = diagnostic_.getCustomDiagID(
+        DiagnosticsEngine::Note, kFieldRequiresFinalizationNote);
   }
 
   virtual void HandleTranslationUnit(ASTContext& context) {
@@ -490,37 +505,56 @@ class BlinkGCPluginConsumer : public ASTConsumer {
     }
 
     if (info->IsGCDerived()) {
-      {
-        CheckGCRootsVisitor visitor;
-        if (visitor.ContainsGCRoots(info))
-          ReportClassContainsGCRoots(info, &visitor.gc_roots());
-      }
+      CheckGCRootsVisitor visitor;
+      if (visitor.ContainsGCRoots(info))
+        ReportClassContainsGCRoots(info, &visitor.gc_roots());
 
-      // TODO: check for non-user defined and non-trivial destructors too.
-      // TODO: support overridden finalize().
-      if (CXXDestructorDecl* dtor = info->record()->getDestructor()) {
-        if (dtor->isUserProvided() && !info->IsGCFinalized()) {
-          // Don't report if using transition types and the body is empty.
-          if (!options_.enable_oilpan) {
-            ReportFinalizerInNonFinalizedClass(info, dtor);
-          } else {
-            if (dtor->hasBody()) {
-              CompoundStmt* stmt = cast<CompoundStmt>(dtor->getBody());
-              if (stmt && !stmt->body_empty())
-                ReportFinalizerInNonFinalizedClass(info, dtor);
-            }
-          }
-        }
+      if (info->NeedsFinalization())
+        CheckFinalization(info);
+    }
+  }
 
-        if (dtor->hasBody()) {
-          CheckFinalizerVisitor visitor(&cache_);
-          visitor.TraverseCXXMethodDecl(dtor);
-          if (!visitor.finalized_fields().empty()) {
-            ReportFinalizerAccessesFinalizedFields(dtor,
-                                                   &visitor.finalized_fields());
-          }
+  void CheckFinalization(RecordInfo* info) {
+    // TODO: Should we collect destructors similar to trace methods?
+    // TODO: Check overridden finalize().
+    CXXDestructorDecl* dtor = info->record()->getDestructor();
+
+    // For finalized classes, check the finalization method if possible.
+    if (info->IsGCFinalized()) {
+      if (dtor && dtor->hasBody()) {
+        CheckFinalizerVisitor visitor(&cache_);
+        visitor.TraverseCXXMethodDecl(dtor);
+        if (!visitor.finalized_fields().empty()) {
+          ReportFinalizerAccessesFinalizedFields(
+              dtor, &visitor.finalized_fields());
         }
       }
+      return;
+    }
+
+    // Don't require finalization of a mixin that has not yet been "mixed in".
+    if (info->IsUnmixedGCMixin())
+      return;
+
+    // Report the finalization error, and proceed to print possible causes for
+    // the finalization requirement.
+    ReportClassRequiresFinalization(info);
+
+    if (dtor && dtor->isUserProvided())
+      NoteUserDeclaredDestructor(dtor);
+
+    for (RecordInfo::Bases::iterator it = info->GetBases().begin();
+         it != info->GetBases().end();
+         ++it) {
+      if (it->second.info()->NeedsFinalization())
+        NoteBaseRequiresFinalization(&it->second);
+    }
+
+    for (RecordInfo::Fields::iterator it = info->GetFields().begin();
+         it != info->GetFields().end();
+         ++it) {
+      if (it->second.edge()->NeedsFinalization())
+        NoteField(&it->second, diag_field_requires_finalization_note_);
     }
   }
 
@@ -725,15 +759,6 @@ class BlinkGCPluginConsumer : public ASTConsumer {
     }
   }
 
-  void ReportFinalizerInNonFinalizedClass(RecordInfo* info,
-                                          CXXMethodDecl* dtor) {
-    SourceLocation loc = dtor->getLocStart();
-    SourceManager& manager = instance_.getSourceManager();
-    FullSourceLoc full_loc(loc, manager);
-    diagnostic_.Report(full_loc, diag_finalizer_in_nonfinalized_class_)
-        << info->record() << dtor;
-  }
-
   void ReportFinalizerAccessesFinalizedFields(
       CXXMethodDecl* dtor,
       CheckFinalizerVisitor::Errors* fields) {
@@ -749,6 +774,14 @@ class BlinkGCPluginConsumer : public ASTConsumer {
     }
   }
 
+  void ReportClassRequiresFinalization(RecordInfo* info) {
+    SourceLocation loc = info->record()->getInnerLocStart();
+    SourceManager& manager = instance_.getSourceManager();
+    FullSourceLoc full_loc(loc, manager);
+    diagnostic_.Report(full_loc, diag_class_requires_finalization_)
+        << info->record();
+  }
+
   void NoteFieldRequiresTracing(RecordInfo* holder, FieldDecl* field) {
     NoteField(field, diag_field_requires_tracing_note_);
   }
@@ -766,6 +799,21 @@ class BlinkGCPluginConsumer : public ASTConsumer {
     NoteField(point, diag_field_contains_gc_root_note_);
   }
 
+  void NoteUserDeclaredDestructor(CXXMethodDecl* dtor) {
+    SourceLocation loc = dtor->getLocStart();
+    SourceManager& manager = instance_.getSourceManager();
+    FullSourceLoc full_loc(loc, manager);
+    diagnostic_.Report(full_loc, diag_user_declared_destructor_note_);
+  }
+
+  void NoteBaseRequiresFinalization(BasePoint* base) {
+    SourceLocation loc = base->spec().getLocStart();
+    SourceManager& manager = instance_.getSourceManager();
+    FullSourceLoc full_loc(loc, manager);
+    diagnostic_.Report(full_loc, diag_base_requires_finalization_note_)
+        << base->info()->record();
+  }
+
   void NoteField(FieldPoint* point, unsigned note) {
     NoteField(point->field(), note);
   }
@@ -782,7 +830,7 @@ class BlinkGCPluginConsumer : public ASTConsumer {
   unsigned diag_fields_require_tracing_;
   unsigned diag_class_contains_invalid_fields_;
   unsigned diag_class_contains_gc_root_;
-  unsigned diag_finalizer_in_nonfinalized_class_;
+  unsigned diag_class_requires_finalization_;
   unsigned diag_finalizer_accesses_finalized_field_;
 
   unsigned diag_field_requires_tracing_note_;
@@ -792,6 +840,9 @@ class BlinkGCPluginConsumer : public ASTConsumer {
   unsigned diag_part_object_contains_gc_root_note_;
   unsigned diag_field_contains_gc_root_note_;
   unsigned diag_finalized_field_note_;
+  unsigned diag_user_declared_destructor_note_;
+  unsigned diag_base_requires_finalization_note_;
+  unsigned diag_field_requires_finalization_note_;
 
   CompilerInstance& instance_;
   DiagnosticsEngine& diagnostic_;
diff --git a/tools/clang/blink_gc_plugin/Edge.cpp b/tools/clang/blink_gc_plugin/Edge.cpp
index 2b5be75..c56a576 100644
--- a/tools/clang/blink_gc_plugin/Edge.cpp
+++ b/tools/clang/blink_gc_plugin/Edge.cpp
@@ -10,6 +10,9 @@ TracingStatus Value::NeedsTracing(NeedsTracingOption option) {
   return value_->NeedsTracing(option);
 }
 
+bool Value::NeedsFinalization() { return value_->NeedsFinalization(); }
+bool Collection::NeedsFinalization() { return info_->NeedsFinalization(); }
+
 void RecursiveEdgeVisitor::AtValue(Value*) {}
 void RecursiveEdgeVisitor::AtRawPtr(RawPtr*) {}
 void RecursiveEdgeVisitor::AtRefPtr(RefPtr*) {}
diff --git a/tools/clang/blink_gc_plugin/Edge.h b/tools/clang/blink_gc_plugin/Edge.h
index 5de4107..36ff1e8 100644
--- a/tools/clang/blink_gc_plugin/Edge.h
+++ b/tools/clang/blink_gc_plugin/Edge.h
@@ -75,6 +75,7 @@ class Edge {
 
   virtual ~Edge() { }
   virtual void Accept(EdgeVisitor*) = 0;
+  virtual bool NeedsFinalization() = 0;
   virtual TracingStatus NeedsTracing(NeedsTracingOption) {
     return TracingStatus::Unknown();
   }
@@ -94,6 +95,7 @@ class Value : public Edge {
  public:
   explicit Value(RecordInfo* value) : value_(value) {};
   bool IsValue() { return true; }
+  bool NeedsFinalization();
   TracingStatus NeedsTracing(NeedsTracingOption);
   void Accept(EdgeVisitor* visitor) { visitor->VisitValue(this); }
   RecordInfo* value() { return value_; }
@@ -118,6 +120,7 @@ class RawPtr : public PtrEdge {
  public:
   explicit RawPtr(Edge* ptr) : PtrEdge(ptr) { }
   bool IsRawPtr() { return true; }
+  bool NeedsFinalization() { return false; }
   TracingStatus NeedsTracing(NeedsTracingOption) {
     return TracingStatus::Unneeded();
   }
@@ -128,6 +131,7 @@ class RefPtr : public PtrEdge {
  public:
   explicit RefPtr(Edge* ptr) : PtrEdge(ptr) { }
   bool IsRefPtr() { return true; }
+  bool NeedsFinalization() { return true; }
   TracingStatus NeedsTracing(NeedsTracingOption) {
     return TracingStatus::Unneeded();
   }
@@ -138,6 +142,7 @@ class OwnPtr : public PtrEdge {
  public:
   explicit OwnPtr(Edge* ptr) : PtrEdge(ptr) { }
   bool IsOwnPtr() { return true; }
+  bool NeedsFinalization() { return true; }
   TracingStatus NeedsTracing(NeedsTracingOption) {
     return TracingStatus::Unneeded();
   }
@@ -148,6 +153,7 @@ class Member : public PtrEdge {
  public:
   explicit Member(Edge* ptr) : PtrEdge(ptr) { }
   bool IsMember() { return true; }
+  bool NeedsFinalization() { return false; }
   TracingStatus NeedsTracing(NeedsTracingOption) {
     return TracingStatus::Needed();
   }
@@ -158,6 +164,7 @@ class WeakMember : public PtrEdge {
  public:
   explicit WeakMember(Edge* ptr) : PtrEdge(ptr) { }
   bool IsWeakMember() { return true; }
+  bool NeedsFinalization() { return false; }
   TracingStatus NeedsTracing(NeedsTracingOption) {
     return TracingStatus::Needed();
   }
@@ -168,6 +175,7 @@ class Persistent : public PtrEdge {
  public:
   explicit Persistent(Edge* ptr) : PtrEdge(ptr) { }
   bool IsPersistent() { return true; }
+  bool NeedsFinalization() { return true; }
   TracingStatus NeedsTracing(NeedsTracingOption) {
     return TracingStatus::Unneeded();
   }
@@ -177,8 +185,10 @@ class Persistent : public PtrEdge {
 class Collection : public Edge {
  public:
   typedef std::vector<Edge*> Members;
-  Collection(bool on_heap, bool is_root)
-      : on_heap_(on_heap), is_root_(is_root) {}
+  Collection(RecordInfo* info, bool on_heap, bool is_root)
+      : info_(info),
+        on_heap_(on_heap),
+        is_root_(is_root) {}
   ~Collection() {
     for (Members::iterator it = members_.begin(); it != members_.end(); ++it) {
       assert(*it && "Collection-edge members must be non-null");
@@ -194,6 +204,7 @@ class Collection : public Edge {
     for (Members::iterator it = members_.begin(); it != members_.end(); ++it)
       (*it)->Accept(visitor);
   }
+  bool NeedsFinalization();
   TracingStatus NeedsTracing(NeedsTracingOption) {
     if (is_root_)
       return TracingStatus::Unneeded();
@@ -208,6 +219,7 @@ class Collection : public Edge {
     return status;
   }
  private:
+  RecordInfo* info_;
   Members members_;
   bool on_heap_;
   bool is_root_;
diff --git a/tools/clang/blink_gc_plugin/RecordInfo.cpp b/tools/clang/blink_gc_plugin/RecordInfo.cpp
index 2fa8d53..1878f1e 100644
--- a/tools/clang/blink_gc_plugin/RecordInfo.cpp
+++ b/tools/clang/blink_gc_plugin/RecordInfo.cpp
@@ -113,6 +113,21 @@ bool RecordInfo::IsGCFinalized() {
   return false;
 }
 
+// A mixin has not yet been "mixed in" if its only GC base is the mixin base.
+bool RecordInfo::IsUnmixedGCMixin() {
+  if (!IsGCDerived() || base_paths_->begin() == base_paths_->end())
+    return false;
+  // Get the last element of the first path.
+  CXXBasePaths::paths_iterator it = base_paths_->begin();
+  const CXXBasePathElement& elem = (*it)[it->size() - 1];
+  CXXRecordDecl* base = elem.Base->getType()->getAsCXXRecordDecl();
+  // If it is not a mixin base we are done.
+  if (!Config::IsGCMixinBase(base->getName()))
+    return false;
+  // Otherwise, this is unmixed if there are no other paths to GC bases.
+  return ++it == base_paths_->end();
+}
+
 // Test if a record is allocated on the managed heap.
 bool RecordInfo::IsGCAllocated() {
   return IsGCDerived() || IsHeapAllocatedCollection();
@@ -171,16 +186,20 @@ bool RecordInfo::InheritsNonPureTrace() {
 RecordInfo::Bases* RecordInfo::CollectBases() {
   // Compute the collection locally to avoid inconsistent states.
   Bases* bases = new Bases;
+  if (!record_->hasDefinition())
+    return bases;
   for (CXXRecordDecl::base_class_iterator it = record_->bases_begin();
        it != record_->bases_end();
        ++it) {
-    if (CXXRecordDecl* base = it->getType()->getAsCXXRecordDecl()) {
-      RecordInfo* info = cache_->Lookup(base);
-      TracingStatus status = info->InheritsNonPureTrace()
-                                 ? TracingStatus::Needed()
-                                 : TracingStatus::Unneeded();
-      bases->insert(std::make_pair(base, BasePoint(info, status)));
-    }
+    const CXXBaseSpecifier& spec = *it;
+    RecordInfo* info = cache_->Lookup(spec.getType());
+    if (!info)
+      continue;
+    CXXRecordDecl* base = info->record();
+    TracingStatus status = info->InheritsNonPureTrace()
+                               ? TracingStatus::Needed()
+                               : TracingStatus::Unneeded();
+    bases->insert(std::make_pair(base, BasePoint(spec, info, status)));
   }
   return bases;
 }
@@ -194,6 +213,8 @@ RecordInfo::Fields& RecordInfo::GetFields() {
 RecordInfo::Fields* RecordInfo::CollectFields() {
   // Compute the collection locally to avoid inconsistent states.
   Fields* fields = new Fields;
+  if (!record_->hasDefinition())
+    return fields;
   TracingStatus fields_status = TracingStatus::Unneeded();
   for (RecordDecl::field_iterator it = record_->field_begin();
        it != record_->field_end();
@@ -241,14 +262,28 @@ void RecordInfo::DetermineTracingMethods() {
   }
 }
 
+// TODO: Add classes with a finalize() method that specialize FinalizerTrait.
+bool RecordInfo::NeedsFinalization() {
+  return record_->hasNonTrivialDestructor();
+}
+
 // A class needs tracing if:
 // - it is allocated on the managed heap,
-// - it defines a trace method (of the proper signature), or
+// - it is derived from a class that needs tracing, or
 // - it contains fields that need tracing.
+// TODO: Defining NeedsTracing based on whether a class defines a trace method
+// (of the proper signature) over approximates too much. The use of transition
+// types causes some classes to have trace methods without them needing to be
+// traced.
 TracingStatus RecordInfo::NeedsTracing(Edge::NeedsTracingOption option) {
-  if (IsGCAllocated() || GetTraceMethod())
+  if (IsGCAllocated())
     return TracingStatus::Needed();
 
+  for (Bases::iterator it = GetBases().begin(); it != GetBases().end(); ++it) {
+    if (it->second.info()->NeedsTracing(option).IsNeeded())
+      return TracingStatus::Needed();
+  }
+
   if (option == Edge::kRecursive)
     GetFields();
 
@@ -266,15 +301,13 @@ Edge* RecordInfo::CreateEdge(const Type* type) {
     return 0;
   }
 
-  CXXRecordDecl* record = type->getAsCXXRecordDecl();
+  RecordInfo* info = cache_->Lookup(type);
 
   // If the type is neither a pointer or a C++ record we ignore it.
-  if (!record) {
+  if (!info) {
     return 0;
   }
 
-  RecordInfo* info = cache_->Lookup(record);
-
   TemplateArgs args;
 
   if (Config::IsRawPtr(info->name()) && info->GetTemplateArgs(1, &args)) {
@@ -328,7 +361,7 @@ Edge* RecordInfo::CreateEdge(const Type* type) {
     size_t count = Config::CollectionDimension(info->name());
     if (!info->GetTemplateArgs(count, &args))
       return 0;
-    Collection* edge = new Collection(on_heap, is_root);
+    Collection* edge = new Collection(info, on_heap, is_root);
     for (TemplateArgs::iterator it = args.begin(); it != args.end(); ++it) {
       if (Edge* member = CreateEdge(*it)) {
         edge->members().push_back(member);
diff --git a/tools/clang/blink_gc_plugin/RecordInfo.h b/tools/clang/blink_gc_plugin/RecordInfo.h
index a27f810..d8a5397 100644
--- a/tools/clang/blink_gc_plugin/RecordInfo.h
+++ b/tools/clang/blink_gc_plugin/RecordInfo.h
@@ -32,13 +32,17 @@ class GraphPoint {
 
 class BasePoint : public GraphPoint {
  public:
-  BasePoint(RecordInfo* info, const TracingStatus& status)
-      : info_(info), status_(status) {}
-  RecordInfo* info() { return info_; }
+  BasePoint(const clang::CXXBaseSpecifier& spec,
+            RecordInfo* info,
+            const TracingStatus& status)
+      : spec_(spec), info_(info), status_(status) {}
   const TracingStatus NeedsTracing() { return status_; }
   // Needed to change the status of bases with a pure-virtual trace.
   void MarkUnneeded() { status_ = TracingStatus::Unneeded(); }
+  const clang::CXXBaseSpecifier& spec() { return spec_; }
+  RecordInfo* info() { return info_; }
  private:
+  const clang::CXXBaseSpecifier& spec_;
   RecordInfo* info_;
   TracingStatus status_;
 };
@@ -82,9 +86,11 @@ class RecordInfo {
   bool IsGCDerived();
   bool IsGCAllocated();
   bool IsGCFinalized();
+  bool IsUnmixedGCMixin();
 
   bool IsStackAllocated();
   bool RequiresTraceMethod();
+  bool NeedsFinalization();
   TracingStatus NeedsTracing(Edge::NeedsTracingOption);
 
  private:
@@ -134,6 +140,14 @@ class RecordCache {
     return Lookup(clang::dyn_cast<clang::CXXRecordDecl>(decl));
   }
 
+  RecordInfo* Lookup(const clang::Type* type) {
+    return Lookup(type->getAsCXXRecordDecl());
+  }
+
+  RecordInfo* Lookup(const clang::QualType& type) {
+    return Lookup(type.getTypePtr());
+  }
+
   ~RecordCache() {
     for (Cache::iterator it = cache_.begin(); it != cache_.end(); ++it) {
       if (!it->second.fields_)
diff --git a/tools/clang/blink_gc_plugin/tests/class_requires_finalization_base.cpp b/tools/clang/blink_gc_plugin/tests/class_requires_finalization_base.cpp
new file mode 100644
index 0000000..85e05b7
--- /dev/null
+++ b/tools/clang/blink_gc_plugin/tests/class_requires_finalization_base.cpp
@@ -0,0 +1,19 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "class_requires_finalization_base.h"
+
+namespace WebCore {
+
+void NeedsFinalizer::trace(Visitor* visitor)
+{
+    A::trace(visitor);
+}
+
+void DoesNotNeedFinalizer::trace(Visitor* visitor)
+{
+    A::trace(visitor);
+}
+
+}
diff --git a/tools/clang/blink_gc_plugin/tests/class_requires_finalization_base.h b/tools/clang/blink_gc_plugin/tests/class_requires_finalization_base.h
new file mode 100644
index 0000000..9504dca
--- /dev/null
+++ b/tools/clang/blink_gc_plugin/tests/class_requires_finalization_base.h
@@ -0,0 +1,36 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CLASS_REQUIRES_FINALIZATION_BASE_H_
+#define CLASS_REQUIRES_FINALIZATION_BASE_H_
+
+#include "heap/stubs.h"
+
+namespace WebCore {
+
+class A : public GarbageCollected<A> {
+public:
+    virtual void trace(Visitor*) {}
+};
+
+class B {
+public:
+    ~B() { /* user-declared, thus, non-trivial */ }
+};
+
+// Second base class needs finalization.
+class NeedsFinalizer : public A, public B {
+public:
+    void trace(Visitor*);
+};
+
+// Base does not need finalization.
+class DoesNotNeedFinalizer : public A {
+public:
+    void trace(Visitor*);
+};
+
+}
+
+#endif
diff --git a/tools/clang/blink_gc_plugin/tests/class_requires_finalization_base.txt b/tools/clang/blink_gc_plugin/tests/class_requires_finalization_base.txt
new file mode 100644
index 0000000..935883d
--- /dev/null
+++ b/tools/clang/blink_gc_plugin/tests/class_requires_finalization_base.txt
@@ -0,0 +1,8 @@
+In file included from class_requires_finalization_base.cpp:5:
+./class_requires_finalization_base.h:23:1: warning: [blink-gc] Class 'NeedsFinalizer' requires finalization.
+class NeedsFinalizer : public A, public B {
+^
+./class_requires_finalization_base.h:23:34: note: [blink-gc] Base class 'B' requiring finalization declared here:
+class NeedsFinalizer : public A, public B {
+                                 ^
+1 warning generated.
diff --git a/tools/clang/blink_gc_plugin/tests/class_requires_finalization_field.cpp b/tools/clang/blink_gc_plugin/tests/class_requires_finalization_field.cpp
new file mode 100644
index 0000000..8667f88
--- /dev/null
+++ b/tools/clang/blink_gc_plugin/tests/class_requires_finalization_field.cpp
@@ -0,0 +1,34 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "class_requires_finalization_field.h"
+
+namespace WebCore {
+
+void NeedsFinalizer::trace(Visitor* visitor)
+{
+    visitor->trace(m_as);
+    A::trace(visitor);
+}
+
+void AlsoNeedsFinalizer::trace(Visitor* visitor)
+{
+    visitor->trace(m_bs);
+    A::trace(visitor);
+}
+
+void DoesNotNeedFinalizer::trace(Visitor* visitor)
+{
+    visitor->trace(m_bs);
+    A::trace(visitor);
+}
+
+void AlsoDoesNotNeedFinalizer::trace(Visitor* visitor)
+{
+    visitor->trace(m_as);
+    visitor->trace(m_cs);
+    A::trace(visitor);
+}
+
+}
diff --git a/tools/clang/blink_gc_plugin/tests/class_requires_finalization_field.h b/tools/clang/blink_gc_plugin/tests/class_requires_finalization_field.h
new file mode 100644
index 0000000..8d8ac35
--- /dev/null
+++ b/tools/clang/blink_gc_plugin/tests/class_requires_finalization_field.h
@@ -0,0 +1,80 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CLASS_REQUIRES_FINALIZATION_H_
+#define CLASS_REQUIRES_FINALIZATION_H_
+
+#include "heap/stubs.h"
+
+namespace WebCore {
+
+class A : public GarbageCollected<A> {
+public:
+    virtual void trace(Visitor*) { }
+};
+
+// Has a non-trivial dtor (user-declared).
+class B {
+public:
+    ~B() { }
+    void trace(Visitor*) { };
+};
+
+// Has a trivial dtor.
+class C {
+public:
+    void trace(Visitor*) { };
+};
+
+} // WebCore namespace
+
+namespace WTF {
+
+template<>
+struct VectorTraits<WebCore::C> {
+    static const bool needsDestruction = false;
+};
+
+} // WTF namespace
+
+namespace WebCore {
+
+// Off-heap vectors always need to be finalized.
+class NeedsFinalizer : public A {
+public:
+    void trace(Visitor*);
+private:
+    Vector<Member<A> > m_as;
+};
+
+// On-heap vectors with inlined objects that need destruction
+// need to be finalized.
+class AlsoNeedsFinalizer : public A {
+public:
+    void trace(Visitor*);
+private:
+    HeapVector<B, 10> m_bs;
+};
+
+// On-heap vectors with no inlined objects never need to be finalized.
+class DoesNotNeedFinalizer : public A {
+public:
+    void trace(Visitor*);
+private:
+    HeapVector<B> m_bs;
+};
+
+// On-heap vectors with inlined objects that don't need destruction
+// don't need to be finalized.
+class AlsoDoesNotNeedFinalizer : public A {
+public:
+    void trace(Visitor*);
+private:
+    HeapVector<Member<A>, 10> m_as;
+    HeapVector<C, 10> m_cs;
+};
+
+}
+
+#endif
diff --git a/tools/clang/blink_gc_plugin/tests/class_requires_finalization_field.txt b/tools/clang/blink_gc_plugin/tests/class_requires_finalization_field.txt
new file mode 100644
index 0000000..9e37c46
--- /dev/null
+++ b/tools/clang/blink_gc_plugin/tests/class_requires_finalization_field.txt
@@ -0,0 +1,14 @@
+In file included from class_requires_finalization_field.cpp:5:
+./class_requires_finalization_field.h:44:1: warning: [blink-gc] Class 'NeedsFinalizer' requires finalization.
+class NeedsFinalizer : public A {
+^
+./class_requires_finalization_field.h:48:5: note: [blink-gc] Field 'm_as' requiring finalization declared here:
+    Vector<Member<A> > m_as;
+    ^
+./class_requires_finalization_field.h:53:1: warning: [blink-gc] Class 'AlsoNeedsFinalizer' requires finalization.
+class AlsoNeedsFinalizer : public A {
+^
+./class_requires_finalization_field.h:57:5: note: [blink-gc] Field 'm_bs' requiring finalization declared here:
+    HeapVector<B, 10> m_bs;
+    ^
+2 warnings generated.
diff --git a/tools/clang/blink_gc_plugin/tests/class_requires_finalization_mixin.cpp b/tools/clang/blink_gc_plugin/tests/class_requires_finalization_mixin.cpp
new file mode 100644
index 0000000..c1a9ec7
--- /dev/null
+++ b/tools/clang/blink_gc_plugin/tests/class_requires_finalization_mixin.cpp
@@ -0,0 +1,26 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "class_requires_finalization_mixin.h"
+
+namespace WebCore {
+
+void Mixin::trace(Visitor* visitor)
+{
+    visitor->trace(m_onHeap);
+}
+
+void NeedsFinalizer::trace(Visitor* visitor)
+{
+    visitor->trace(m_obj);
+    Mixin::trace(visitor);
+}
+
+void HasFinalizer::trace(Visitor* visitor)
+{
+    visitor->trace(m_obj);
+    Mixin::trace(visitor);
+}
+
+}
diff --git a/tools/clang/blink_gc_plugin/tests/class_requires_finalization_mixin.h b/tools/clang/blink_gc_plugin/tests/class_requires_finalization_mixin.h
new file mode 100644
index 0000000..f321281
--- /dev/null
+++ b/tools/clang/blink_gc_plugin/tests/class_requires_finalization_mixin.h
@@ -0,0 +1,42 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CLASS_REQUIRES_FINALIZATION_MIXIN_H_
+#define CLASS_REQUIRES_FINALIZATION_MIXIN_H_
+
+#include "heap/stubs.h"
+
+namespace WebCore {
+
+class OffHeap : public RefCounted<OffHeap> { };
+class OnHeap : public GarbageCollected<OnHeap> { };
+
+class Mixin : public GarbageCollectedMixin {
+public:
+    void trace(Visitor*);
+private:
+    RefPtr<OffHeap> m_offHeap; // Requires finalization
+    Member<OnHeap> m_onHeap;
+};
+
+class NeedsFinalizer : public GarbageCollected<NeedsFinalizer>, public Mixin {
+    USING_GARBAGE_COLLECTED_MIXIN(NeedsFinalizer);
+public:
+    void trace(Visitor*);
+private:
+    Member<OnHeap> m_obj;
+};
+
+class HasFinalizer : public GarbageCollectedFinalized<HasFinalizer>,
+                     public Mixin {
+    USING_GARBAGE_COLLECTED_MIXIN(HasFinalizer);
+public:
+    void trace(Visitor*);
+private:
+    Member<OnHeap> m_obj;
+};
+
+}
+
+#endif
diff --git a/tools/clang/blink_gc_plugin/tests/class_requires_finalization_mixin.txt b/tools/clang/blink_gc_plugin/tests/class_requires_finalization_mixin.txt
new file mode 100644
index 0000000..f42a0d6
--- /dev/null
+++ b/tools/clang/blink_gc_plugin/tests/class_requires_finalization_mixin.txt
@@ -0,0 +1,8 @@
+In file included from class_requires_finalization_mixin.cpp:5:
+./class_requires_finalization_mixin.h:23:1: warning: [blink-gc] Class 'NeedsFinalizer' requires finalization.
+class NeedsFinalizer : public GarbageCollected<NeedsFinalizer>, public Mixin {
+^
+./class_requires_finalization_mixin.h:23:65: note: [blink-gc] Base class 'Mixin' requiring finalization declared here:
+class NeedsFinalizer : public GarbageCollected<NeedsFinalizer>, public Mixin {
+                                                                ^
+1 warning generated.
diff --git a/tools/clang/blink_gc_plugin/tests/destructor_in_nonfinalized_class.txt b/tools/clang/blink_gc_plugin/tests/destructor_in_nonfinalized_class.txt
index 042f66d..cf19ea1 100644
--- a/tools/clang/blink_gc_plugin/tests/destructor_in_nonfinalized_class.txt
+++ b/tools/clang/blink_gc_plugin/tests/destructor_in_nonfinalized_class.txt
@@ -1,4 +1,8 @@
-destructor_in_nonfinalized_class.cpp:9:1: warning: [blink-gc] Non-finalized class 'HeapObject' has a user-declared finalizer '~HeapObject'.
+In file included from destructor_in_nonfinalized_class.cpp:5:
+./destructor_in_nonfinalized_class.h:12:1: warning: [blink-gc] Class 'HeapObject' requires finalization.
+class HeapObject : public GarbageCollected<HeapObject> {
+^
+destructor_in_nonfinalized_class.cpp:9:1: note: [blink-gc] User-declared destructor declared here:
 HeapObject::~HeapObject()
 ^
 1 warning generated.
diff --git a/tools/clang/blink_gc_plugin/tests/heap/stubs.h b/tools/clang/blink_gc_plugin/tests/heap/stubs.h
index 10b777c..f6994f4 100644
--- a/tools/clang/blink_gc_plugin/tests/heap/stubs.h
+++ b/tools/clang/blink_gc_plugin/tests/heap/stubs.h
@@ -18,20 +18,45 @@ public:
 
 template<typename T> class RefPtr {
 public:
+    ~RefPtr() { }
     operator T*() const { return 0; }
 };
 
 template<typename T> class OwnPtr {
 public:
+    ~OwnPtr() { }
     operator T*() const { return 0; }
 };
 
-class DefaultAllocator { };
+class DefaultAllocator {
+public:
+    static const bool isGarbageCollected = false;
+};
+
+template<typename T>
+struct VectorTraits {
+    static const bool needsDestruction = true;
+};
+
+template<size_t inlineCapacity, bool isGarbageCollected, bool tNeedsDestruction>
+class VectorDestructorBase {
+public:
+    ~VectorDestructorBase() {}
+};
+
+template<size_t inlineCapacity>
+class VectorDestructorBase<inlineCapacity, true, false> {};
+
+template<>
+class VectorDestructorBase<0, true, true> {};
 
 template<typename T,
          size_t inlineCapacity = 0,
          typename Allocator = DefaultAllocator>
-class Vector { };
+class Vector : public VectorDestructorBase<inlineCapacity,
+                                           Allocator::isGarbageCollected,
+                                           VectorTraits<T>::needsDestruction> {
+};
 
 }
 
@@ -71,10 +96,13 @@ public:
     operator T*() const { return 0; }
 };
 
-class HeapAllocator { };
+class HeapAllocator {
+public:
+    static const bool isGarbageCollected = true;
+};
 
-template<typename T>
-class HeapVector : public Vector<T, 0, HeapAllocator> { };
+template<typename T, size_t inlineCapacity = 0>
+class HeapVector : public Vector<T, inlineCapacity, HeapAllocator> { };
 
 template<typename T>
 class PersistentHeapVector : public Vector<T, 0, HeapAllocator> { };
@@ -91,4 +119,13 @@ class GarbageCollectedMixin {
 
 }
 
+namespace WTF {
+
+template<typename T>
+struct VectorTraits<WebCore::Member<T> > {
+    static const bool needsDestruction = false;
+};
+
+}
+
 #endif
diff --git a/tools/clang/blink_gc_plugin/tests/own_ptr_to_gc_managed_class.h b/tools/clang/blink_gc_plugin/tests/own_ptr_to_gc_managed_class.h
index b7c263f..52062a1 100644
--- a/tools/clang/blink_gc_plugin/tests/own_ptr_to_gc_managed_class.h
+++ b/tools/clang/blink_gc_plugin/tests/own_ptr_to_gc_managed_class.h
@@ -17,7 +17,7 @@ private:
     OwnPtr<HeapObject> m_obj;
 };
 
-class HeapObject : public GarbageCollected<HeapObject> {
+class HeapObject : public GarbageCollectedFinalized<HeapObject> {
 public:
     void trace(Visitor*);
 private:
diff --git a/tools/clang/blink_gc_plugin/tests/own_ptr_to_gc_managed_class.txt b/tools/clang/blink_gc_plugin/tests/own_ptr_to_gc_managed_class.txt
index dc1b2b2..b1dac2c 100644
--- a/tools/clang/blink_gc_plugin/tests/own_ptr_to_gc_managed_class.txt
+++ b/tools/clang/blink_gc_plugin/tests/own_ptr_to_gc_managed_class.txt
@@ -6,7 +6,7 @@ class PartObject {
     OwnPtr<HeapObject> m_obj;
     ^
 ./own_ptr_to_gc_managed_class.h:20:1: warning: [blink-gc] Class 'HeapObject' contains invalid fields.
-class HeapObject : public GarbageCollected<HeapObject> {
+class HeapObject : public GarbageCollectedFinalized<HeapObject> {
 ^
 ./own_ptr_to_gc_managed_class.h:24:5: note: [blink-gc] OwnPtr field 'm_objs' to a GC managed class declared here:
     Vector<OwnPtr<HeapObject> > m_objs;
diff --git a/tools/clang/blink_gc_plugin/tests/ref_ptr_to_gc_managed_class.h b/tools/clang/blink_gc_plugin/tests/ref_ptr_to_gc_managed_class.h
index 086c9ef..cf1f629 100644
--- a/tools/clang/blink_gc_plugin/tests/ref_ptr_to_gc_managed_class.h
+++ b/tools/clang/blink_gc_plugin/tests/ref_ptr_to_gc_managed_class.h
@@ -17,7 +17,7 @@ private:
     RefPtr<HeapObject> m_obj;
 };
 
-class HeapObject : public GarbageCollected<HeapObject> {
+class HeapObject : public GarbageCollectedFinalized<HeapObject> {
 public:
     void trace(Visitor*);
 private:
diff --git a/tools/clang/blink_gc_plugin/tests/ref_ptr_to_gc_managed_class.txt b/tools/clang/blink_gc_plugin/tests/ref_ptr_to_gc_managed_class.txt
index a7cf651..fd49785 100644
--- a/tools/clang/blink_gc_plugin/tests/ref_ptr_to_gc_managed_class.txt
+++ b/tools/clang/blink_gc_plugin/tests/ref_ptr_to_gc_managed_class.txt
@@ -6,7 +6,7 @@ class PartObject {
     RefPtr<HeapObject> m_obj;
     ^
 ./ref_ptr_to_gc_managed_class.h:20:1: warning: [blink-gc] Class 'HeapObject' contains invalid fields.
-class HeapObject : public GarbageCollected<HeapObject> {
+class HeapObject : public GarbageCollectedFinalized<HeapObject> {
 ^
 ./ref_ptr_to_gc_managed_class.h:25:5: note: [blink-gc] RefPtr field 'm_objs' to a GC managed class declared here:
     Vector<RefPtr<HeapObject> > m_objs;