[patch] Remove ASTConsumer::HandleVTable()'s bool parameter

[Nico Weber]

Hi,

sema calls HandleVTable() with a bool parameter which is then threaded
through three layers. The only effect of this bool is an early return at
the last layer.

Instead, remove this parameter and call HandleVTable() only if the bool is
true. No intended behavior change, this is just a code simplification.

I'm not sure why these three hops are done – it's not due to code rot, this
was added in one piece in r103718. That's why I'm not just landing this but
asking for precommit review – does anyone know if there's a reason for the
current path?

Nico
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150112/5a3533cd/attachment.html>
-------------- next part --------------
Index: include/clang/AST/ASTConsumer.h
===================================================================
--- include/clang/AST/ASTConsumer.h	(revision 225759)
+++ include/clang/AST/ASTConsumer.h	(working copy)
@@ -129,11 +129,7 @@
   /// required.
   ///
   /// \param RD The class whose vtable was used.
-  ///
-  /// \param DefinitionRequired Whether a definition of this vtable is
-  /// required in this translation unit; otherwise, it is only needed if
-  /// it was actually used.
-  virtual void HandleVTable(CXXRecordDecl *RD, bool DefinitionRequired) {}
+  virtual void HandleVTable(CXXRecordDecl *RD) {}
 
   /// \brief If the consumer is interested in entities getting modified after
   /// their initial creation, it should return a pointer to
Index: include/clang/Frontend/MultiplexConsumer.h
===================================================================
--- include/clang/Frontend/MultiplexConsumer.h	(revision 225759)
+++ include/clang/Frontend/MultiplexConsumer.h	(working copy)
@@ -49,7 +49,7 @@
                             llvm::StringRef Value) override;
   void HandleDependentLibrary(llvm::StringRef Lib) override;
   void CompleteTentativeDefinition(VarDecl *D) override;
-  void HandleVTable(CXXRecordDecl *RD, bool DefinitionRequired) override;
+  void HandleVTable(CXXRecordDecl *RD) override;
   ASTMutationListener *GetASTMutationListener() override;
   ASTDeserializationListener *GetASTDeserializationListener() override;
   void PrintStats() override;
Index: lib/CodeGen/CGVTables.cpp
===================================================================
--- lib/CodeGen/CGVTables.cpp	(revision 225759)
+++ lib/CodeGen/CGVTables.cpp	(working copy)
@@ -747,19 +747,13 @@
   llvm_unreachable("Invalid TemplateSpecializationKind!");
 }
 
-/// This is a callback from Sema to tell us that it believes that a
-/// particular v-table is required to be emitted in this translation
-/// unit.
+/// This is a callback from Sema to tell us that that a particular v-table is
+/// required to be emitted in this translation unit.
 ///
-/// The reason we don't simply trust this callback is because Sema
-/// will happily report that something is used even when it's used
-/// only in code that we don't actually have to emit.
-///
-/// \param isRequired - if true, the v-table is mandatory, e.g.
-///   because the translation unit defines the key function
-void CodeGenModule::EmitVTable(CXXRecordDecl *theClass, bool isRequired) {
-  if (!isRequired) return;
-
+/// This is only called for vtables that _must_ be emitted (mainly due to key
+/// functions).  For weak vtables, CodeGen tracks when they are needed and
+/// emits them as-needed.
+void CodeGenModule::EmitVTable(CXXRecordDecl *theClass) {
   VTables.GenerateClassData(theClass);
 }
 
Index: lib/CodeGen/CodeGenAction.cpp
===================================================================
--- lib/CodeGen/CodeGenAction.cpp	(revision 225759)
+++ lib/CodeGen/CodeGenAction.cpp	(working copy)
@@ -196,8 +196,8 @@
       Gen->CompleteTentativeDefinition(D);
     }
 
-    void HandleVTable(CXXRecordDecl *RD, bool DefinitionRequired) override {
-      Gen->HandleVTable(RD, DefinitionRequired);
+    void HandleVTable(CXXRecordDecl *RD) override {
+      Gen->HandleVTable(RD);
     }
 
     void HandleLinkerOptionPragma(llvm::StringRef Opts) override {
Index: lib/CodeGen/CodeGenModule.h
===================================================================
--- lib/CodeGen/CodeGenModule.h	(revision 225759)
+++ lib/CodeGen/CodeGenModule.h	(working copy)
@@ -993,7 +993,7 @@
 
   void EmitTentativeDefinition(const VarDecl *D);
 
-  void EmitVTable(CXXRecordDecl *Class, bool DefinitionRequired);
+  void EmitVTable(CXXRecordDecl *Class);
 
   /// Emit the RTTI descriptors for the builtin types.
   void EmitFundamentalRTTIDescriptors();
Index: lib/CodeGen/ModuleBuilder.cpp
===================================================================
--- lib/CodeGen/ModuleBuilder.cpp	(revision 225759)
+++ lib/CodeGen/ModuleBuilder.cpp	(working copy)
@@ -211,11 +211,11 @@
       Builder->EmitTentativeDefinition(D);
     }
 
-    void HandleVTable(CXXRecordDecl *RD, bool DefinitionRequired) override {
+    void HandleVTable(CXXRecordDecl *RD) override {
       if (Diags.hasErrorOccurred())
         return;
 
-      Builder->EmitVTable(RD, DefinitionRequired);
+      Builder->EmitVTable(RD);
     }
 
     void HandleLinkerOptionPragma(llvm::StringRef Opts) override {
Index: lib/Frontend/MultiplexConsumer.cpp
===================================================================
--- lib/Frontend/MultiplexConsumer.cpp	(revision 225759)
+++ lib/Frontend/MultiplexConsumer.cpp	(working copy)
@@ -292,10 +292,9 @@
     Consumer->CompleteTentativeDefinition(D);
 }
 
-void MultiplexConsumer::HandleVTable(
-    CXXRecordDecl *RD, bool DefinitionRequired) {
+void MultiplexConsumer::HandleVTable(CXXRecordDecl *RD) {
   for (auto &Consumer : Consumers)
-    Consumer->HandleVTable(RD, DefinitionRequired);
+    Consumer->HandleVTable(RD);
 }
 
 ASTMutationListener *MultiplexConsumer::GetASTMutationListener() {
Index: lib/Sema/SemaDeclCXX.cpp
===================================================================
--- lib/Sema/SemaDeclCXX.cpp	(revision 225759)
+++ lib/Sema/SemaDeclCXX.cpp	(working copy)
@@ -13117,7 +13117,8 @@
     DefinedAnything = true;
     MarkVirtualMembersReferenced(Loc, Class);
     CXXRecordDecl *Canonical = cast<CXXRecordDecl>(Class->getCanonicalDecl());
-    Consumer.HandleVTable(Class, VTablesUsed[Canonical]);
+    if (VTablesUsed[Canonical])
+      Consumer.HandleVTable(Class);
 
     // Optionally warn if we're emitting a weak vtable.
     if (Class->isExternallyVisible() &&