[Clang][AST][NFC] MemberExpr stores NestedNameSpecifierLoc and DeclAccessPair separately

[sdkrystian]

Currently, MemberExpr allocates a trailing MemberExprNameQualifier object if it either has a NestedNameSpecifierLoc, or if it names a member found via using declaration. Since the presence of a nested-name-specifier does not necessarily imply the named member was found via using declaration, this patch removes MemberExprNameQualifier and allocates the members separately.

[llvmbot]

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-modules

Author: Krystian Stasiowski (sdkrystian)

Changes

Currently, MemberExpr allocates a trailing MemberExprNameQualifier object if it either has a NestedNameSpecifierLoc, or if it names a member found via using declaration. Since the presence of a nested-name-specifier does not necessarily imply the named member was found via using declaration, this patch removes MemberExprNameQualifier and allocates the members separately.

---

Full diff: https://github.com/llvm/llvm-project/pull/86678.diff

5 Files Affected:

• (modified) clang/include/clang/AST/Expr.h (+14-23)
• (modified) clang/include/clang/AST/Stmt.h (+7-4)
• (modified) clang/lib/AST/Expr.cpp (+21-21)
• (modified) clang/lib/Serialization/ASTReaderStmt.cpp (+9-17)
• (modified) clang/lib/Serialization/ASTWriterStmt.cpp (+4-7)

diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index 6e153ebe024b42..e43098e144c88f 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -3163,23 +3163,12 @@ class CallExpr : public Expr {
   }
 };
 
-/// Extra data stored in some MemberExpr objects.
-struct MemberExprNameQualifier {
-  /// The nested-name-specifier that qualifies the name, including
-  /// source-location information.
-  NestedNameSpecifierLoc QualifierLoc;
-
-  /// The DeclAccessPair through which the MemberDecl was found due to
-  /// name qualifiers.
-  DeclAccessPair FoundDecl;
-};
-
 /// MemberExpr - [C99 6.5.2.3] Structure and Union Members.  X->F and X.F.
 ///
 class MemberExpr final
     : public Expr,
-      private llvm::TrailingObjects<MemberExpr, MemberExprNameQualifier,
-                                    ASTTemplateKWAndArgsInfo,
+      private llvm::TrailingObjects<MemberExpr, NestedNameSpecifierLoc,
+                                    DeclAccessPair, ASTTemplateKWAndArgsInfo,
                                     TemplateArgumentLoc> {
   friend class ASTReader;
   friend class ASTStmtReader;
@@ -3201,17 +3190,19 @@ class MemberExpr final
   /// MemberLoc - This is the location of the member name.
   SourceLocation MemberLoc;
 
-  size_t numTrailingObjects(OverloadToken<MemberExprNameQualifier>) const {
-    return hasQualifierOrFoundDecl();
+  size_t numTrailingObjects(OverloadToken<NestedNameSpecifierLoc>) const {
+    return hasQualifier();
+  }
+
+  size_t numTrailingObjects(OverloadToken<DeclAccessPair>) const {
+    return hasFoundDecl();
   }
 
   size_t numTrailingObjects(OverloadToken<ASTTemplateKWAndArgsInfo>) const {
     return hasTemplateKWAndArgsInfo();
   }
 
-  bool hasQualifierOrFoundDecl() const {
-    return MemberExprBits.HasQualifierOrFoundDecl;
-  }
+  bool hasFoundDecl() const { return MemberExprBits.HasFoundDecl; }
 
   bool hasTemplateKWAndArgsInfo() const {
     return MemberExprBits.HasTemplateKWAndArgsInfo;
@@ -3264,24 +3255,24 @@ class MemberExpr final
 
   /// Retrieves the declaration found by lookup.
   DeclAccessPair getFoundDecl() const {
-    if (!hasQualifierOrFoundDecl())
+    if (!hasFoundDecl())
       return DeclAccessPair::make(getMemberDecl(),
                                   getMemberDecl()->getAccess());
-    return getTrailingObjects<MemberExprNameQualifier>()->FoundDecl;
+    return *getTrailingObjects<DeclAccessPair>();
   }
 
   /// Determines whether this member expression actually had
   /// a C++ nested-name-specifier prior to the name of the member, e.g.,
   /// x->Base::foo.
-  bool hasQualifier() const { return getQualifier() != nullptr; }
+  bool hasQualifier() const { return MemberExprBits.HasQualifier; }
 
   /// If the member name was qualified, retrieves the
   /// nested-name-specifier that precedes the member name, with source-location
   /// information.
   NestedNameSpecifierLoc getQualifierLoc() const {
-    if (!hasQualifierOrFoundDecl())
+    if (!hasQualifier())
       return NestedNameSpecifierLoc();
-    return getTrailingObjects<MemberExprNameQualifier>()->QualifierLoc;
+    return *getTrailingObjects<NestedNameSpecifierLoc>();
   }
 
   /// If the member name was qualified, retrieves the
diff --git a/clang/include/clang/AST/Stmt.h b/clang/include/clang/AST/Stmt.h
index 55eca4007d17ea..425814912136f5 100644
--- a/clang/include/clang/AST/Stmt.h
+++ b/clang/include/clang/AST/Stmt.h
@@ -583,11 +583,14 @@ class alignas(void *) Stmt {
     unsigned IsArrow : 1;
 
     /// True if this member expression used a nested-name-specifier to
-    /// refer to the member, e.g., "x->Base::f", or found its member via
-    /// a using declaration.  When true, a MemberExprNameQualifier
-    /// structure is allocated immediately after the MemberExpr.
+    /// refer to the member, e.g., "x->Base::f".
     LLVM_PREFERRED_TYPE(bool)
-    unsigned HasQualifierOrFoundDecl : 1;
+    unsigned HasQualifier : 1;
+
+    // True if this member expression found its member via
+    /// a using declaration.
+    LLVM_PREFERRED_TYPE(bool)
+    unsigned HasFoundDecl : 1;
 
     /// True if this member expression specified a template keyword
     /// and/or a template argument list explicitly, e.g., x->f<int>,
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 6221ebd5c9b4e9..6835c43197e39f 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -1721,7 +1721,8 @@ MemberExpr::MemberExpr(Expr *Base, bool IsArrow, SourceLocation OperatorLoc,
   assert(!NameInfo.getName() ||
          MemberDecl->getDeclName() == NameInfo.getName());
   MemberExprBits.IsArrow = IsArrow;
-  MemberExprBits.HasQualifierOrFoundDecl = false;
+  MemberExprBits.HasQualifier = false;
+  MemberExprBits.HasFoundDecl = false;
   MemberExprBits.HasTemplateKWAndArgsInfo = false;
   MemberExprBits.HadMultipleCandidates = false;
   MemberExprBits.NonOdrUseReason = NOUR;
@@ -1735,30 +1736,30 @@ MemberExpr *MemberExpr::Create(
     ValueDecl *MemberDecl, DeclAccessPair FoundDecl,
     DeclarationNameInfo NameInfo, const TemplateArgumentListInfo *TemplateArgs,
     QualType T, ExprValueKind VK, ExprObjectKind OK, NonOdrUseReason NOUR) {
-  bool HasQualOrFound = QualifierLoc || FoundDecl.getDecl() != MemberDecl ||
-                        FoundDecl.getAccess() != MemberDecl->getAccess();
+  bool HasQualifier = QualifierLoc.hasQualifier();
+  bool HasFoundDecl = FoundDecl.getDecl() != MemberDecl ||
+                      FoundDecl.getAccess() != MemberDecl->getAccess();
   bool HasTemplateKWAndArgsInfo = TemplateArgs || TemplateKWLoc.isValid();
   std::size_t Size =
-      totalSizeToAlloc<MemberExprNameQualifier, ASTTemplateKWAndArgsInfo,
-                       TemplateArgumentLoc>(
-          HasQualOrFound ? 1 : 0, HasTemplateKWAndArgsInfo ? 1 : 0,
+      totalSizeToAlloc<NestedNameSpecifierLoc, DeclAccessPair,
+                       ASTTemplateKWAndArgsInfo, TemplateArgumentLoc>(
+          HasQualifier ? 1 : 0, HasFoundDecl ? 1 : 0,
+          HasTemplateKWAndArgsInfo ? 1 : 0,
           TemplateArgs ? TemplateArgs->size() : 0);
 
   void *Mem = C.Allocate(Size, alignof(MemberExpr));
   MemberExpr *E = new (Mem) MemberExpr(Base, IsArrow, OperatorLoc, MemberDecl,
                                        NameInfo, T, VK, OK, NOUR);
 
-  if (HasQualOrFound) {
-    E->MemberExprBits.HasQualifierOrFoundDecl = true;
-
-    MemberExprNameQualifier *NQ =
-        E->getTrailingObjects<MemberExprNameQualifier>();
-    NQ->QualifierLoc = QualifierLoc;
-    NQ->FoundDecl = FoundDecl;
-  }
+  E->MemberExprBits.HasQualifier = HasQualifier;
+  E->MemberExprBits.HasFoundDecl = HasFoundDecl;
+  E->MemberExprBits.HasTemplateKWAndArgsInfo = HasTemplateKWAndArgsInfo;
 
-  E->MemberExprBits.HasTemplateKWAndArgsInfo =
-      TemplateArgs || TemplateKWLoc.isValid();
+  if (HasQualifier)
+    new (E->getTrailingObjects<NestedNameSpecifierLoc>())
+        NestedNameSpecifierLoc(QualifierLoc);
+  if (HasFoundDecl)
+    *E->getTrailingObjects<DeclAccessPair>() = FoundDecl;
 
   // FIXME: remove remaining dependence computation to computeDependence().
   auto Deps = E->getDependence();
@@ -1785,12 +1786,11 @@ MemberExpr *MemberExpr::CreateEmpty(const ASTContext &Context,
                                     unsigned NumTemplateArgs) {
   assert((!NumTemplateArgs || HasTemplateKWAndArgsInfo) &&
          "template args but no template arg info?");
-  bool HasQualOrFound = HasQualifier || HasFoundDecl;
   std::size_t Size =
-      totalSizeToAlloc<MemberExprNameQualifier, ASTTemplateKWAndArgsInfo,
-                       TemplateArgumentLoc>(HasQualOrFound ? 1 : 0,
-                                            HasTemplateKWAndArgsInfo ? 1 : 0,
-                                            NumTemplateArgs);
+      totalSizeToAlloc<NestedNameSpecifierLoc, DeclAccessPair,
+                       ASTTemplateKWAndArgsInfo, TemplateArgumentLoc>(
+          HasQualifier ? 1 : 0, HasFoundDecl ? 1 : 0,
+          HasTemplateKWAndArgsInfo ? 1 : 0, NumTemplateArgs);
   void *Mem = Context.Allocate(Size, alignof(MemberExpr));
   return new (Mem) MemberExpr(EmptyShell());
 }
diff --git a/clang/lib/Serialization/ASTReaderStmt.cpp b/clang/lib/Serialization/ASTReaderStmt.cpp
index 674ed47581dfd3..bbeb6db011646f 100644
--- a/clang/lib/Serialization/ASTReaderStmt.cpp
+++ b/clang/lib/Serialization/ASTReaderStmt.cpp
@@ -1047,30 +1047,22 @@ void ASTStmtReader::VisitMemberExpr(MemberExpr *E) {
   E->MemberDNLoc = Record.readDeclarationNameLoc(E->MemberDecl->getDeclName());
   E->MemberLoc = Record.readSourceLocation();
   E->MemberExprBits.IsArrow = CurrentUnpackingBits->getNextBit();
-  E->MemberExprBits.HasQualifierOrFoundDecl = HasQualifier || HasFoundDecl;
+  E->MemberExprBits.HasQualifier = HasQualifier;
+  E->MemberExprBits.HasFoundDecl = HasFoundDecl;
   E->MemberExprBits.HasTemplateKWAndArgsInfo = HasTemplateInfo;
   E->MemberExprBits.HadMultipleCandidates = CurrentUnpackingBits->getNextBit();
   E->MemberExprBits.NonOdrUseReason =
       CurrentUnpackingBits->getNextBits(/*Width=*/2);
   E->MemberExprBits.OperatorLoc = Record.readSourceLocation();
 
-  if (HasQualifier || HasFoundDecl) {
-    DeclAccessPair FoundDecl;
-    if (HasFoundDecl) {
-      auto *FoundD = Record.readDeclAs<NamedDecl>();
-      auto AS = (AccessSpecifier)CurrentUnpackingBits->getNextBits(/*Width=*/2);
-      FoundDecl = DeclAccessPair::make(FoundD, AS);
-    } else {
-      FoundDecl = DeclAccessPair::make(E->MemberDecl,
-                                       E->MemberDecl->getAccess());
-    }
-    E->getTrailingObjects<MemberExprNameQualifier>()->FoundDecl = FoundDecl;
+  if (HasQualifier)
+    new (E->getTrailingObjects<NestedNameSpecifierLoc>())
+        NestedNameSpecifierLoc(Record.readNestedNameSpecifierLoc());
 
-    NestedNameSpecifierLoc QualifierLoc;
-    if (HasQualifier)
-      QualifierLoc = Record.readNestedNameSpecifierLoc();
-    E->getTrailingObjects<MemberExprNameQualifier>()->QualifierLoc =
-        QualifierLoc;
+  if (HasFoundDecl) {
+    auto *FoundD = Record.readDeclAs<NamedDecl>();
+    auto AS = (AccessSpecifier)CurrentUnpackingBits->getNextBits(/*Width=*/2);
+    *E->getTrailingObjects<DeclAccessPair>() = DeclAccessPair::make(FoundD, AS);
   }
 
   if (HasTemplateInfo)
diff --git a/clang/lib/Serialization/ASTWriterStmt.cpp b/clang/lib/Serialization/ASTWriterStmt.cpp
index 7ce48fede383ea..22e190450d3918 100644
--- a/clang/lib/Serialization/ASTWriterStmt.cpp
+++ b/clang/lib/Serialization/ASTWriterStmt.cpp
@@ -970,10 +970,7 @@ void ASTStmtWriter::VisitMemberExpr(MemberExpr *E) {
   VisitExpr(E);
 
   bool HasQualifier = E->hasQualifier();
-  bool HasFoundDecl =
-      E->hasQualifierOrFoundDecl() &&
-      (E->getFoundDecl().getDecl() != E->getMemberDecl() ||
-       E->getFoundDecl().getAccess() != E->getMemberDecl()->getAccess());
+  bool HasFoundDecl = E->hasFoundDecl();
   bool HasTemplateInfo = E->hasTemplateKWAndArgsInfo();
   unsigned NumTemplateArgs = E->getNumTemplateArgs();
 
@@ -995,15 +992,15 @@ void ASTStmtWriter::VisitMemberExpr(MemberExpr *E) {
   CurrentPackingBits.addBits(E->isNonOdrUse(), /*Width=*/2);
   Record.AddSourceLocation(E->getOperatorLoc());
 
+  if (HasQualifier)
+    Record.AddNestedNameSpecifierLoc(E->getQualifierLoc());
+
   if (HasFoundDecl) {
     DeclAccessPair FoundDecl = E->getFoundDecl();
     Record.AddDeclRef(FoundDecl.getDecl());
     CurrentPackingBits.addBits(FoundDecl.getAccess(), /*BitWidth=*/2);
   }
 
-  if (HasQualifier)
-    Record.AddNestedNameSpecifierLoc(E->getQualifierLoc());
-
   if (HasTemplateInfo)
     AddTemplateKWAndArgsInfo(*E->getTrailingObjects<ASTTemplateKWAndArgsInfo>(),
                              E->getTrailingObjects<TemplateArgumentLoc>());

[sdkrystian]

Changes were reviewed in #86682