Blink GC plugin: Check polymorphic requirements on all GC-derived classes.

tools/clang/blink_gc_plugin/BlinkGCPlugin.cpp

 // 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.
 
 // This clang plugin checks various invariants of the Blink garbage
 // collection infrastructure.
 //
 // Errors are described at:
 // http://www.chromium.org/developers/blink-gc-plugin-errors
 
@@ skipping 933 matching lines @@
            it != info->GetBases().end();
            ++it) {
         if (!it->second.info()->IsStackAllocated())
           ReportDerivesNonStackAllocated(info, &it->second);
       }
     }
 
     if (CXXMethodDecl* trace = info->GetTraceMethod()) {
       if (trace->isPure())
         ReportClassDeclaresPureVirtualTrace(info, trace);
-      if (info->record()->isPolymorphic())
-        CheckPolymorphicClass(info, trace);
     } else if (info->RequiresTraceMethod()) {
       ReportClassRequiresTraceMethod(info);
     }
 
+    // Check polymorphic classes that are GC-derived or have a trace method.
+    if (info->record()->hasDefinition() && info->record()->isPolymorphic()) {
+      CXXMethodDecl* trace = info->GetTraceMethod();
+      if (trace || info->IsGCDerived())
+        CheckPolymorphicClass(info, trace);
+    }
+
     {
       CheckFieldsVisitor visitor(options_);
       if (visitor.ContainsInvalidFields(info))
         ReportClassContainsInvalidFields(info, &visitor.invalid_fields());
     }
 
     if (info->IsGCDerived()) {
 
       if (!info->IsGCMixin()) {
         CheckLeftMostDerived(info);
@@ skipping 33 matching lines @@
 
   // The GC infrastructure assumes that if the vtable of a polymorphic
   // base-class is not initialized for a given object (ie, it is partially
   // initialized) then the object does not need to be traced. Thus, we must
   // ensure that any polymorphic class with a trace method does not have any
   // tractable fields that are initialized before we are sure that the vtable
   // and the trace method are both defined.  There are two cases that need to
   // hold to satisfy that assumption:
   //
   // 1. If trace is virtual, then it must be defined in the left-most base.
-  // This ensures that if the vtable is initialized and it contains a pointer to
-  // the trace method.
+  // This ensures that if the vtable is initialized then it contains a pointer
+  // to the trace method.
   //
   // 2. If trace is non-virtual, then the trace method is defined and we must
   // ensure that the left-most base defines a vtable. This ensures that the
   // first thing to be initialized when constructing the object is the vtable
   // itself.
   void CheckPolymorphicClass(RecordInfo* info, CXXMethodDecl* trace) {
     CXXRecordDecl* left_most = info->record();
     CXXRecordDecl::base_class_iterator it = left_most->bases_begin();
     CXXRecordDecl* left_most_base = 0;
     while (it != left_most->bases_end()) {
       left_most_base = it->getType()->getAsCXXRecordDecl();
       if (!left_most_base && it->getType()->isDependentType())
         left_most_base = GetDependentTemplatedDecl(*it->getType());
 
       // TODO: Find a way to correctly check actual instantiations
       // for dependent types. The escape below will be hit, eg, when
       // we have a primary template with no definition and
       // specializations for each case (such as SupplementBase) in
       // which case we don't succeed in checking the required
       // properties.
       if (!left_most_base || !left_most_base->hasDefinition())
         return;
 
       StringRef name = left_most_base->getName();
       // We know GCMixin base defines virtual trace.
       if (Config::IsGCMixinBase(name))
         return;
 
       // Stop with the left-most prior to a safe polymorphic base (a safe base
-      // is non-polymorphic and contains no fields that need tracing).
+      // is non-polymorphic and contains no fields).
       if (Config::IsSafePolymorphicBase(name))
         break;
 
       left_most = left_most_base;
       it = left_most->bases_begin();
     }
 
     if (RecordInfo* left_most_info = cache_.Lookup(left_most)) {
 
       // Check condition (1):
-      if (trace->isVirtual()) {
+      if (trace && trace->isVirtual()) {
         if (CXXMethodDecl* trace = left_most_info->GetTraceMethod()) {
           if (trace->isVirtual())
             return;
         }
         ReportBaseClassMustDeclareVirtualTrace(info, left_most);
         return;
       }
 
       // Check condition (2):
-      if (DeclaresVirtualMethods(info->record()))
+      if (DeclaresVirtualMethods(left_most))
         return;
       if (left_most_base) {
         ++it; // Get the base next to the "safe polymorphic base"
         if (it != left_most->bases_end()) {
           if (CXXRecordDecl* next_base = it->getType()->getAsCXXRecordDecl()) {
             if (CXXRecordDecl* next_left_most = GetLeftMostBase(next_base)) {
               if (DeclaresVirtualMethods(next_left_most))
                 return;
               ReportLeftMostBaseMustBePolymorphic(info, next_left_most);
               return;
@@ skipping 792 matching lines @@
 
  private:
   BlinkGCPluginOptions options_;
 };
 
 }  // namespace
 
 static FrontendPluginRegistry::Add<BlinkGCPluginAction> X(
     "blink-gc-plugin",
     "Check Blink GC invariants");