r339030 - [AST] Remove unnecessary indirections in DeclarationNameTable

Bruno Ricci via cfe-commits cfe-commits at lists.llvm.org
Mon Aug 6 09:47:31 PDT 2018


Author: brunoricci
Date: Mon Aug  6 09:47:31 2018
New Revision: 339030

URL: http://llvm.org/viewvc/llvm-project?rev=339030&view=rev
Log:
[AST] Remove unnecessary indirections in DeclarationNameTable

DeclarationNameTable currently hold 3 "void *" to
FoldingSet<CXXSpecialName>, FoldingSet<CXXLiteralOperatorIdName>
and FoldingSet<CXXDeductionGuideNameExtra>.

CXXSpecialName, CXXLiteralOperatorIdName and
CXXDeductionGuideNameExtra are private classes holding extra
information about a "special" declaration name and are in
AST/DeclarationName.cpp. The original intent seems to have
been to keep these classes private and only expose
DeclarationNameExtra and DeclarationName (the code dates from
2008 and has not been significantly changed since).

However this make the code less straightforward than necessary
because of the need to have "void *" in DeclarationNameTable
(with 1 of 3 comments wrong) and to manually allocate/deallocate
the FoldingSets.

Moreover removing the extra indirections reduce the run-time of
an fsyntax-only on all of Boost by 2.3% which is not totally
unexpected given how frequently this data structure is used
(especially for C++).

A concern raised by erichkeane during the review was that
including Type.h would increase the compile time unreasonably.
However test builds (both clean and incremental) showed that
this patch did not result in any compile time increase.

Reviewed By: erichkeane

Differential Revision: https://reviews.llvm.org/D50261

Modified:
    cfe/trunk/include/clang/AST/DeclarationName.h
    cfe/trunk/lib/AST/DeclarationName.cpp

Modified: cfe/trunk/include/clang/AST/DeclarationName.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclarationName.h?rev=339030&r1=339029&r2=339030&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/DeclarationName.h (original)
+++ cfe/trunk/include/clang/AST/DeclarationName.h Mon Aug  6 09:47:31 2018
@@ -14,11 +14,13 @@
 #ifndef LLVM_CLANG_AST_DECLARATIONNAME_H
 #define LLVM_CLANG_AST_DECLARATIONNAME_H
 
+#include "clang/AST/Type.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/PartialDiagnostic.h"
 #include "clang/Basic/SourceLocation.h"
 #include "llvm/ADT/DenseMapInfo.h"
+#include "llvm/ADT/FoldingSet.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/type_traits.h"
 #include <cassert>
@@ -39,9 +41,7 @@ class IdentifierInfo;
 class MultiKeywordSelector;
 enum OverloadedOperatorKind : int;
 struct PrintingPolicy;
-class QualType;
 class TemplateDecl;
-class Type;
 class TypeSourceInfo;
 class UsingDirectiveDecl;
 
@@ -343,33 +343,113 @@ inline bool operator>=(DeclarationName L
   return DeclarationName::compare(LHS, RHS) >= 0;
 }
 
+/// CXXSpecialName - Records the type associated with one of the
+/// "special" kinds of declaration names in C++, e.g., constructors,
+/// destructors, and conversion functions.
+class CXXSpecialName : public DeclarationNameExtra,
+                       public llvm::FoldingSetNode {
+public:
+  /// Type - The type associated with this declaration name.
+  QualType Type;
+
+  /// FETokenInfo - Extra information associated with this declaration
+  /// name that can be used by the front end. All bits are really needed
+  /// so it is not possible to stash something in the low order bits.
+  void *FETokenInfo;
+
+  void Profile(llvm::FoldingSetNodeID &ID) {
+    ID.AddInteger(ExtraKindOrNumArgs);
+    ID.AddPointer(Type.getAsOpaquePtr());
+  }
+};
+
+/// Contains extra information for the name of a C++ deduction guide.
+class CXXDeductionGuideNameExtra : public DeclarationNameExtra,
+                                   public llvm::FoldingSetNode {
+public:
+  /// The template named by the deduction guide.
+  TemplateDecl *Template;
+
+  /// FETokenInfo - Extra information associated with this operator
+  /// name that can be used by the front end. All bits are really needed
+  /// so it is not possible to stash something in the low order bits.
+  void *FETokenInfo;
+
+  void Profile(llvm::FoldingSetNodeID &ID) { ID.AddPointer(Template); }
+};
+
+/// CXXOperatorIdName - Contains extra information for the name of an
+/// overloaded operator in C++, such as "operator+.
+class CXXOperatorIdName : public DeclarationNameExtra {
+public:
+  /// FETokenInfo - Extra information associated with this operator
+  /// name that can be used by the front end. All bits are really needed
+  /// so it is not possible to stash something in the low order bits.
+  void *FETokenInfo;
+};
+
+/// CXXLiteralOperatorName - Contains the actual identifier that makes up the
+/// name.
+///
+/// This identifier is stored here rather than directly in DeclarationName so as
+/// to allow Objective-C selectors, which are about a million times more common,
+/// to consume minimal memory.
+class CXXLiteralOperatorIdName : public DeclarationNameExtra,
+                                 public llvm::FoldingSetNode {
+public:
+  IdentifierInfo *ID;
+
+  /// FETokenInfo - Extra information associated with this operator
+  /// name that can be used by the front end. All bits are really needed
+  /// so it is not possible to stash something in the low order bits.
+  void *FETokenInfo;
+
+  void Profile(llvm::FoldingSetNodeID &FSID) { FSID.AddPointer(ID); }
+};
+
 /// DeclarationNameTable - Used to store and retrieve DeclarationName
 /// instances for the various kinds of declaration names, e.g., normal
 /// identifiers, C++ constructor names, etc. This class contains
 /// uniqued versions of each of the C++ special names, which can be
-/// retrieved using its member functions (e.g.,
-/// getCXXConstructorName).
+/// retrieved using its member functions (e.g., getCXXConstructorName).
 class DeclarationNameTable {
+  /// Used to allocate elements in the FoldingSets and
+  /// in the array of CXXOperatorIdName below.
   const ASTContext &Ctx;
 
-  // Actually a FoldingSet<CXXSpecialName> *
-  void *CXXSpecialNamesImpl;
-
-  // Operator names
+  /// Manage the uniqued CXXSpecialName, which contain extra information
+  /// for the "special" kinds of declaration names in C++ such as constructors,
+  /// destructors and conversion functions. getCXXConstructorName,
+  /// getCXXDestructorName, getCXXConversionFunctionName, and getCXXSpecialName
+  /// can be used to obtain a DeclarationName from the corresponding type.
+  llvm::FoldingSet<CXXSpecialName> CXXSpecialNames;
+
+  /// Manage the uniqued CXXOperatorIdName, which contain extra information
+  /// for the name of overloaded C++ operators. getCXXOperatorName
+  /// can be used to obtain a DeclarationName from the operator kind.
+  /// This points to the first element of an array of NUM_OVERLOADED_OPERATORS
+  /// CXXOperatorIdName which is constructed by DeclarationNameTable.
   CXXOperatorIdName *CXXOperatorNames;
 
-  // Actually a CXXOperatorIdName*
-  void *CXXLiteralOperatorNames;
-
-  // FoldingSet<CXXDeductionGuideNameExtra> *
-  void *CXXDeductionGuideNames;
+  /// Manage the uniqued CXXLiteralOperatorIdName, which contain extra
+  /// information for the name of C++ literal operators.
+  /// getCXXLiteralOperatorName can be used to obtain a DeclarationName
+  /// from the corresponding IdentifierInfo.
+  llvm::FoldingSet<CXXLiteralOperatorIdName> CXXLiteralOperatorNames;
+
+  /// Manage the uniqued CXXDeductionGuideNameExtra, which contain
+  /// extra information for the name of a C++ deduction guide.
+  /// getCXXDeductionGuideName can be used to obtain a DeclarationName
+  /// from the corresponding template declaration.
+  llvm::FoldingSet<CXXDeductionGuideNameExtra> CXXDeductionGuideNames;
 
 public:
   DeclarationNameTable(const ASTContext &C);
   DeclarationNameTable(const DeclarationNameTable &) = delete;
   DeclarationNameTable &operator=(const DeclarationNameTable &) = delete;
-
-  ~DeclarationNameTable();
+  DeclarationNameTable(DeclarationNameTable &&) = delete;
+  DeclarationNameTable &operator=(DeclarationNameTable &&) = delete;
+  ~DeclarationNameTable() = default;
 
   /// getIdentifier - Create a declaration name that is a simple
   /// identifier.

Modified: cfe/trunk/lib/AST/DeclarationName.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclarationName.cpp?rev=339030&r1=339029&r2=339030&view=diff
==============================================================================
--- cfe/trunk/lib/AST/DeclarationName.cpp (original)
+++ cfe/trunk/lib/AST/DeclarationName.cpp Mon Aug  6 09:47:31 2018
@@ -39,74 +39,6 @@
 
 using namespace clang;
 
-namespace clang {
-
-/// CXXSpecialName - Records the type associated with one of the
-/// "special" kinds of declaration names in C++, e.g., constructors,
-/// destructors, and conversion functions.
-class CXXSpecialName
-  : public DeclarationNameExtra, public llvm::FoldingSetNode {
-public:
-  /// Type - The type associated with this declaration name.
-  QualType Type;
-
-  /// FETokenInfo - Extra information associated with this declaration
-  /// name that can be used by the front end.
-  void *FETokenInfo;
-
-  void Profile(llvm::FoldingSetNodeID &ID) {
-    ID.AddInteger(ExtraKindOrNumArgs);
-    ID.AddPointer(Type.getAsOpaquePtr());
-  }
-};
-
-/// Contains extra information for the name of a C++ deduction guide.
-class CXXDeductionGuideNameExtra : public DeclarationNameExtra,
-                                   public llvm::FoldingSetNode {
-public:
-  /// The template named by the deduction guide.
-  TemplateDecl *Template;
-
-  /// FETokenInfo - Extra information associated with this operator
-  /// name that can be used by the front end.
-  void *FETokenInfo;
-
-  void Profile(llvm::FoldingSetNodeID &ID) {
-    ID.AddPointer(Template);
-  }
-};
-
-/// CXXOperatorIdName - Contains extra information for the name of an
-/// overloaded operator in C++, such as "operator+.
-class CXXOperatorIdName : public DeclarationNameExtra {
-public:
-  /// FETokenInfo - Extra information associated with this operator
-  /// name that can be used by the front end.
-  void *FETokenInfo;
-};
-
-/// CXXLiteralOperatorName - Contains the actual identifier that makes up the
-/// name.
-///
-/// This identifier is stored here rather than directly in DeclarationName so as
-/// to allow Objective-C selectors, which are about a million times more common,
-/// to consume minimal memory.
-class CXXLiteralOperatorIdName
-  : public DeclarationNameExtra, public llvm::FoldingSetNode {
-public:
-  IdentifierInfo *ID;
-
-  /// FETokenInfo - Extra information associated with this operator
-  /// name that can be used by the front end.
-  void *FETokenInfo;
-
-  void Profile(llvm::FoldingSetNodeID &FSID) {
-    FSID.AddPointer(ID);
-  }
-};
-
-} // namespace clang
-
 static int compareInt(unsigned A, unsigned B) {
   return (A < B ? -1 : (A > B ? 1 : 0));
 }
@@ -436,10 +368,6 @@ LLVM_DUMP_METHOD void DeclarationName::d
 }
 
 DeclarationNameTable::DeclarationNameTable(const ASTContext &C) : Ctx(C) {
-  CXXSpecialNamesImpl = new llvm::FoldingSet<CXXSpecialName>;
-  CXXLiteralOperatorNames = new llvm::FoldingSet<CXXLiteralOperatorIdName>;
-  CXXDeductionGuideNames = new llvm::FoldingSet<CXXDeductionGuideNameExtra>;
-
   // Initialize the overloaded operator names.
   CXXOperatorNames = new (Ctx) CXXOperatorIdName[NUM_OVERLOADED_OPERATORS];
   for (unsigned Op = 0; Op < NUM_OVERLOADED_OPERATORS; ++Op) {
@@ -449,21 +377,6 @@ DeclarationNameTable::DeclarationNameTab
   }
 }
 
-DeclarationNameTable::~DeclarationNameTable() {
-  auto *SpecialNames =
-      static_cast<llvm::FoldingSet<CXXSpecialName> *>(CXXSpecialNamesImpl);
-  auto *LiteralNames =
-      static_cast<llvm::FoldingSet<CXXLiteralOperatorIdName> *>(
-          CXXLiteralOperatorNames);
-  auto *DeductionGuideNames =
-      static_cast<llvm::FoldingSet<CXXDeductionGuideNameExtra> *>(
-          CXXDeductionGuideNames);
-
-  delete SpecialNames;
-  delete LiteralNames;
-  delete DeductionGuideNames;
-}
-
 DeclarationName DeclarationNameTable::getCXXConstructorName(CanQualType Ty) {
   return getCXXSpecialName(DeclarationName::CXXConstructorName,
                            Ty.getUnqualifiedType());
@@ -478,15 +391,11 @@ DeclarationName
 DeclarationNameTable::getCXXDeductionGuideName(TemplateDecl *Template) {
   Template = cast<TemplateDecl>(Template->getCanonicalDecl());
 
-  auto *DeductionGuideNames =
-      static_cast<llvm::FoldingSet<CXXDeductionGuideNameExtra> *>(
-          CXXDeductionGuideNames);
-
   llvm::FoldingSetNodeID ID;
   ID.AddPointer(Template);
 
   void *InsertPos = nullptr;
-  if (auto *Name = DeductionGuideNames->FindNodeOrInsertPos(ID, InsertPos))
+  if (auto *Name = CXXDeductionGuideNames.FindNodeOrInsertPos(ID, InsertPos))
     return DeclarationName(Name);
 
   auto *Name = new (Ctx) CXXDeductionGuideNameExtra;
@@ -494,7 +403,7 @@ DeclarationNameTable::getCXXDeductionGui
   Name->Template = Template;
   Name->FETokenInfo = nullptr;
 
-  DeductionGuideNames->InsertNode(Name, InsertPos);
+  CXXDeductionGuideNames.InsertNode(Name, InsertPos);
   return DeclarationName(Name);
 }
 
@@ -509,8 +418,6 @@ DeclarationNameTable::getCXXSpecialName(
   assert(Kind >= DeclarationName::CXXConstructorName &&
          Kind <= DeclarationName::CXXConversionFunctionName &&
          "Kind must be a C++ special name kind");
-  llvm::FoldingSet<CXXSpecialName> *SpecialNames
-    = static_cast<llvm::FoldingSet<CXXSpecialName>*>(CXXSpecialNamesImpl);
 
   DeclarationNameExtra::ExtraKind EKind;
   switch (Kind) {
@@ -535,7 +442,7 @@ DeclarationNameTable::getCXXSpecialName(
   ID.AddPointer(Ty.getAsOpaquePtr());
 
   void *InsertPos = nullptr;
-  if (CXXSpecialName *Name = SpecialNames->FindNodeOrInsertPos(ID, InsertPos))
+  if (CXXSpecialName *Name = CXXSpecialNames.FindNodeOrInsertPos(ID, InsertPos))
     return DeclarationName(Name);
 
   CXXSpecialName *SpecialName = new (Ctx) CXXSpecialName;
@@ -543,7 +450,7 @@ DeclarationNameTable::getCXXSpecialName(
   SpecialName->Type = Ty;
   SpecialName->FETokenInfo = nullptr;
 
-  SpecialNames->InsertNode(SpecialName, InsertPos);
+  CXXSpecialNames.InsertNode(SpecialName, InsertPos);
   return DeclarationName(SpecialName);
 }
 
@@ -554,16 +461,12 @@ DeclarationNameTable::getCXXOperatorName
 
 DeclarationName
 DeclarationNameTable::getCXXLiteralOperatorName(IdentifierInfo *II) {
-  llvm::FoldingSet<CXXLiteralOperatorIdName> *LiteralNames
-    = static_cast<llvm::FoldingSet<CXXLiteralOperatorIdName>*>
-                                                      (CXXLiteralOperatorNames);
-
   llvm::FoldingSetNodeID ID;
   ID.AddPointer(II);
 
   void *InsertPos = nullptr;
   if (CXXLiteralOperatorIdName *Name =
-                               LiteralNames->FindNodeOrInsertPos(ID, InsertPos))
+          CXXLiteralOperatorNames.FindNodeOrInsertPos(ID, InsertPos))
     return DeclarationName (Name);
 
   CXXLiteralOperatorIdName *LiteralName = new (Ctx) CXXLiteralOperatorIdName;
@@ -571,7 +474,7 @@ DeclarationNameTable::getCXXLiteralOpera
   LiteralName->ID = II;
   LiteralName->FETokenInfo = nullptr;
 
-  LiteralNames->InsertNode(LiteralName, InsertPos);
+  CXXLiteralOperatorNames.InsertNode(LiteralName, InsertPos);
   return DeclarationName(LiteralName);
 }
 




More information about the cfe-commits mailing list