Convert MemberExpr creation and serialization to work the same way as

most / all other Expr subclasses.

This reinstates r362551, reverted in r362597, with a fix to a bug that
caused MemberExprs to sometimes have a null FoundDecl after a round-trip
through an AST file.

llvm-svn: 362756

clang/include/clang/AST/Expr.h

@@ -2735,6 +2735,7 @@ class MemberExpr final
                                     ASTTemplateKWAndArgsInfo,
                                     TemplateArgumentLoc> {
   friend class ASTReader;
+  friend class ASTStmtReader;
   friend class ASTStmtWriter;
   friend TrailingObjects;
 
@@ -2769,49 +2770,38 @@ class MemberExpr final
     return MemberExprBits.HasTemplateKWAndArgsInfo;
   }
 
+  MemberExpr(Expr *Base, bool IsArrow, SourceLocation OperatorLoc,
+             ValueDecl *MemberDecl, const DeclarationNameInfo &NameInfo,
+             QualType T, ExprValueKind VK, ExprObjectKind OK);
+  MemberExpr(EmptyShell Empty)
+      : Expr(MemberExprClass, Empty), Base(), MemberDecl() {}
+
 public:
-  MemberExpr(Expr *base, bool isarrow, SourceLocation operatorloc,
+  static MemberExpr *Create(const ASTContext &C, Expr *Base, bool IsArrow,
-             ValueDecl *memberdecl, const DeclarationNameInfo &NameInfo,
-             QualType ty, ExprValueKind VK, ExprObjectKind OK)
-      : Expr(MemberExprClass, ty, VK, OK, base->isTypeDependent(),
-             base->isValueDependent(), base->isInstantiationDependent(),
-             base->containsUnexpandedParameterPack()),
-        Base(base), MemberDecl(memberdecl), MemberDNLoc(NameInfo.getInfo()),
-        MemberLoc(NameInfo.getLoc()) {
-    assert(memberdecl->getDeclName() == NameInfo.getName());
-    MemberExprBits.IsArrow = isarrow;
-    MemberExprBits.HasQualifierOrFoundDecl = false;
-    MemberExprBits.HasTemplateKWAndArgsInfo = false;
-    MemberExprBits.HadMultipleCandidates = false;
-    MemberExprBits.OperatorLoc = operatorloc;
-  }
-
-  // NOTE: this constructor should be used only when it is known that
-  // the member name can not provide additional syntactic info
-  // (i.e., source locations for C++ operator names or type source info
-  // for constructors, destructors and conversion operators).
-  MemberExpr(Expr *base, bool isarrow, SourceLocation operatorloc,
-             ValueDecl *memberdecl, SourceLocation l, QualType ty,
-             ExprValueKind VK, ExprObjectKind OK)
-      : Expr(MemberExprClass, ty, VK, OK, base->isTypeDependent(),
-             base->isValueDependent(), base->isInstantiationDependent(),
-             base->containsUnexpandedParameterPack()),
-        Base(base), MemberDecl(memberdecl), MemberDNLoc(), MemberLoc(l) {
-    MemberExprBits.IsArrow = isarrow;
-    MemberExprBits.HasQualifierOrFoundDecl = false;
-    MemberExprBits.HasTemplateKWAndArgsInfo = false;
-    MemberExprBits.HadMultipleCandidates = false;
-    MemberExprBits.OperatorLoc = operatorloc;
-  }
-
-  static MemberExpr *Create(const ASTContext &C, Expr *base, bool isarrow,
                             SourceLocation OperatorLoc,
                             NestedNameSpecifierLoc QualifierLoc,
-                            SourceLocation TemplateKWLoc, ValueDecl *memberdecl,
+                            SourceLocation TemplateKWLoc, ValueDecl *MemberDecl,
-                            DeclAccessPair founddecl,
+                            DeclAccessPair FoundDecl,
                             DeclarationNameInfo MemberNameInfo,
-                            const TemplateArgumentListInfo *targs, QualType ty,
+                            const TemplateArgumentListInfo *TemplateArgs,
-                            ExprValueKind VK, ExprObjectKind OK);
+                            QualType T, ExprValueKind VK, ExprObjectKind OK);
+
+  /// Create an implicit MemberExpr, with no location, qualifier, template
+  /// arguments, and so on.
+  static MemberExpr *CreateImplicit(const ASTContext &C, Expr *Base,
+                                    bool IsArrow, ValueDecl *MemberDecl,
+                                    QualType T, ExprValueKind VK,
+                                    ExprObjectKind OK) {
+    return Create(C, Base, IsArrow, SourceLocation(), NestedNameSpecifierLoc(),
+                  SourceLocation(), MemberDecl,
+                  DeclAccessPair::make(MemberDecl, MemberDecl->getAccess()),
+                  DeclarationNameInfo(), nullptr, T, VK, OK);
+  }
+
+  static MemberExpr *CreateEmpty(const ASTContext &Context, bool HasQualifier,
+                                 bool HasFoundDecl,
+                                 bool HasTemplateKWAndArgsInfo,
+                                 unsigned NumTemplateArgs);
 
   void setBase(Expr *E) { Base = E; }
   Expr *getBase() const { return cast<Expr>(Base); }

clang/include/clang/AST/Stmt.h

@@ -453,6 +453,7 @@ protected:
   enum { NumCallExprBits = 32 };
 
   class MemberExprBitfields {
+    friend class ASTStmtReader;
     friend class MemberExpr;
 
     unsigned : NumExprBits;

clang/lib/AST/DeclBase.cpp

@@ -920,6 +920,7 @@ bool Decl::AccessDeclContextSanity() const {
   if (isa<TranslationUnitDecl>(this) ||
       isa<TemplateTypeParmDecl>(this) ||
       isa<NonTypeTemplateParmDecl>(this) ||
+      !getDeclContext() ||
       !isa<CXXRecordDecl>(getDeclContext()) ||
       isInvalidDecl() ||
       isa<StaticAssertDecl>(this) ||

clang/lib/AST/Expr.cpp

@@ -1538,29 +1538,44 @@ UnaryExprOrTypeTraitExpr::UnaryExprOrTypeTraitExpr(
   }
 }
 
+MemberExpr::MemberExpr(Expr *Base, bool IsArrow, SourceLocation OperatorLoc,
+                       ValueDecl *MemberDecl,
+                       const DeclarationNameInfo &NameInfo, QualType T,
+                       ExprValueKind VK, ExprObjectKind OK)
+    : Expr(MemberExprClass, T, VK, OK, Base->isTypeDependent(),
+           Base->isValueDependent(), Base->isInstantiationDependent(),
+           Base->containsUnexpandedParameterPack()),
+      Base(Base), MemberDecl(MemberDecl), MemberDNLoc(NameInfo.getInfo()),
+      MemberLoc(NameInfo.getLoc()) {
+  assert(!NameInfo.getName() ||
+         MemberDecl->getDeclName() == NameInfo.getName());
+  MemberExprBits.IsArrow = IsArrow;
+  MemberExprBits.HasQualifierOrFoundDecl = false;
+  MemberExprBits.HasTemplateKWAndArgsInfo = false;
+  MemberExprBits.HadMultipleCandidates = false;
+  MemberExprBits.OperatorLoc = OperatorLoc;
+}
+
 MemberExpr *MemberExpr::Create(
-    const ASTContext &C, Expr *base, bool isarrow, SourceLocation OperatorLoc,
+    const ASTContext &C, Expr *Base, bool IsArrow, SourceLocation OperatorLoc,
     NestedNameSpecifierLoc QualifierLoc, SourceLocation TemplateKWLoc,
-    ValueDecl *memberdecl, DeclAccessPair founddecl,
+    ValueDecl *MemberDecl, DeclAccessPair FoundDecl,
-    DeclarationNameInfo nameinfo, const TemplateArgumentListInfo *targs,
+    DeclarationNameInfo NameInfo, const TemplateArgumentListInfo *TemplateArgs,
-    QualType ty, ExprValueKind vk, ExprObjectKind ok) {
+    QualType T, ExprValueKind VK, ExprObjectKind OK) {
-
+  bool HasQualOrFound = QualifierLoc || FoundDecl.getDecl() != MemberDecl ||
-  bool hasQualOrFound = (QualifierLoc ||
+                        FoundDecl.getAccess() != MemberDecl->getAccess();
-                         founddecl.getDecl() != memberdecl ||
+  bool HasTemplateKWAndArgsInfo = TemplateArgs || TemplateKWLoc.isValid();
-                         founddecl.getAccess() != memberdecl->getAccess());
-
-  bool HasTemplateKWAndArgsInfo = targs || TemplateKWLoc.isValid();
   std::size_t Size =
       totalSizeToAlloc<MemberExprNameQualifier, ASTTemplateKWAndArgsInfo,
-                       TemplateArgumentLoc>(hasQualOrFound ? 1 : 0,
+                       TemplateArgumentLoc>(
-                                            HasTemplateKWAndArgsInfo ? 1 : 0,
+          HasQualOrFound ? 1 : 0, HasTemplateKWAndArgsInfo ? 1 : 0,
-                                            targs ? targs->size() : 0);
+          TemplateArgs ? TemplateArgs->size() : 0);
 
   void *Mem = C.Allocate(Size, alignof(MemberExpr));
   MemberExpr *E = new (Mem)
-      MemberExpr(base, isarrow, OperatorLoc, memberdecl, nameinfo, ty, vk, ok);
+      MemberExpr(Base, IsArrow, OperatorLoc, MemberDecl, NameInfo, T, VK, OK);
 
-  if (hasQualOrFound) {
+  if (HasQualOrFound) {
     // FIXME: Wrong. We should be looking at the member declaration we found.
     if (QualifierLoc && QualifierLoc.getNestedNameSpecifier()->isDependent()) {
       E->setValueDependent(true);
@@ -1576,19 +1591,20 @@ MemberExpr *MemberExpr::Create(
     MemberExprNameQualifier *NQ =
         E->getTrailingObjects<MemberExprNameQualifier>();
     NQ->QualifierLoc = QualifierLoc;
-    NQ->FoundDecl = founddecl;
+    NQ->FoundDecl = FoundDecl;
   }
 
   E->MemberExprBits.HasTemplateKWAndArgsInfo =
-      (targs || TemplateKWLoc.isValid());
+      TemplateArgs || TemplateKWLoc.isValid();
 
-  if (targs) {
+  if (TemplateArgs) {
     bool Dependent = false;
     bool InstantiationDependent = false;
     bool ContainsUnexpandedParameterPack = false;
     E->getTrailingObjects<ASTTemplateKWAndArgsInfo>()->initializeFrom(
-        TemplateKWLoc, *targs, E->getTrailingObjects<TemplateArgumentLoc>(),
+        TemplateKWLoc, *TemplateArgs,
-        Dependent, InstantiationDependent, ContainsUnexpandedParameterPack);
+        E->getTrailingObjects<TemplateArgumentLoc>(), Dependent,
+        InstantiationDependent, ContainsUnexpandedParameterPack);
     if (InstantiationDependent)
       E->setInstantiationDependent(true);
   } else if (TemplateKWLoc.isValid()) {
@@ -1599,6 +1615,22 @@ MemberExpr *MemberExpr::Create(
   return E;
 }
 
+MemberExpr *MemberExpr::CreateEmpty(const ASTContext &Context,
+                                    bool HasQualifier, bool HasFoundDecl,
+                                    bool HasTemplateKWAndArgsInfo,
+                                    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);
+  void *Mem = Context.Allocate(Size, alignof(MemberExpr));
+  return new (Mem) MemberExpr(EmptyShell());
+}
+
 SourceLocation MemberExpr::getBeginLoc() const {
   if (isImplicitAccess()) {
     if (hasQualifier())

clang/lib/Frontend/Rewrite/RewriteModernObjC.cpp

@@ -881,9 +881,8 @@ RewriteModernObjC::getIvarAccessString(ObjCIvarDecl *D) {
                                         IvarT, nullptr,
                                         /*BitWidth=*/nullptr, /*Mutable=*/true,
                                         ICIS_NoInit);
-      MemberExpr *ME = new (Context)
+      MemberExpr *ME = MemberExpr::CreateImplicit(
-          MemberExpr(PE, true, SourceLocation(), FD, SourceLocation(),
+          *Context, PE, true, FD, FD->getType(), VK_LValue, OK_Ordinary);
-                     FD->getType(), VK_LValue, OK_Ordinary);
       IvarT = Context->getDecltypeType(ME, ME->getType());
     }
   }
@@ -2736,9 +2735,9 @@ Stmt *RewriteModernObjC::RewriteObjCArrayLiteralExpr(ObjCArrayLiteral *Exp) {
                                     Context->getPointerType(Context->VoidPtrTy),
                                     nullptr, /*BitWidth=*/nullptr,
                                     /*Mutable=*/true, ICIS_NoInit);
-  MemberExpr *ArrayLiteralME = new (Context)
+  MemberExpr *ArrayLiteralME =
-      MemberExpr(NSArrayCallExpr, false, SourceLocation(), ARRFD,
+      MemberExpr::CreateImplicit(*Context, NSArrayCallExpr, false, ARRFD,
-                 SourceLocation(), ARRFD->getType(), VK_LValue, OK_Ordinary);
+                                 ARRFD->getType(), VK_LValue, OK_Ordinary);
   QualType ConstIdT = Context->getObjCIdType().withConst();
   CStyleCastExpr * ArrayLiteralObjects =
     NoTypeInfoCStyleCastExpr(Context,
@@ -2865,9 +2864,9 @@ Stmt *RewriteModernObjC::RewriteObjCDictionaryLiteralExpr(ObjCDictionaryLiteral
                                        Context->getPointerType(Context->VoidPtrTy),
                                        nullptr, /*BitWidth=*/nullptr,
                                        /*Mutable=*/true, ICIS_NoInit);
-  MemberExpr *DictLiteralValueME = new (Context)
+  MemberExpr *DictLiteralValueME =
-      MemberExpr(NSValueCallExpr, false, SourceLocation(), ARRFD,
+      MemberExpr::CreateImplicit(*Context, NSValueCallExpr, false, ARRFD,
-                 SourceLocation(), ARRFD->getType(), VK_LValue, OK_Ordinary);
+                                 ARRFD->getType(), VK_LValue, OK_Ordinary);
   QualType ConstIdT = Context->getObjCIdType().withConst();
   CStyleCastExpr * DictValueObjects =
     NoTypeInfoCStyleCastExpr(Context,
@@ -2878,9 +2877,9 @@ Stmt *RewriteModernObjC::RewriteObjCDictionaryLiteralExpr(ObjCDictionaryLiteral
   Expr *NSKeyCallExpr = CallExpr::Create(
       *Context, NSDictDRE, KeyExprs, NSDictFType, VK_LValue, SourceLocation());
 
-  MemberExpr *DictLiteralKeyME = new (Context)
+  MemberExpr *DictLiteralKeyME =
-      MemberExpr(NSKeyCallExpr, false, SourceLocation(), ARRFD,
+      MemberExpr::CreateImplicit(*Context, NSKeyCallExpr, false, ARRFD,
-                 SourceLocation(), ARRFD->getType(), VK_LValue, OK_Ordinary);
+                                 ARRFD->getType(), VK_LValue, OK_Ordinary);
 
   CStyleCastExpr * DictKeyObjects =
     NoTypeInfoCStyleCastExpr(Context,
@@ -3180,9 +3179,8 @@ Expr *RewriteModernObjC::SynthMsgSendStretCallExpr(FunctionDecl *MsgSendStretFla
                                     returnType, nullptr,
                                     /*BitWidth=*/nullptr,
                                     /*Mutable=*/true, ICIS_NoInit);
-  MemberExpr *ME = new (Context)
+  MemberExpr *ME = MemberExpr::CreateImplicit(
-      MemberExpr(STCE, false, SourceLocation(), FieldD, SourceLocation(),
+      *Context, STCE, false, FieldD, FieldD->getType(), VK_LValue, OK_Ordinary);
-                 FieldD->getType(), VK_LValue, OK_Ordinary);
 
   return ME;
 }
@@ -4629,9 +4627,8 @@ Stmt *RewriteModernObjC::SynthesizeBlockCall(CallExpr *Exp, const Expr *BlockExp
                                     Context->VoidPtrTy, nullptr,
                                     /*BitWidth=*/nullptr, /*Mutable=*/true,
                                     ICIS_NoInit);
-  MemberExpr *ME =
+  MemberExpr *ME = MemberExpr::CreateImplicit(
-      new (Context) MemberExpr(PE, true, SourceLocation(), FD, SourceLocation(),
+      *Context, PE, true, FD, FD->getType(), VK_LValue, OK_Ordinary);
-                               FD->getType(), VK_LValue, OK_Ordinary);
 
   CastExpr *FunkCast = NoTypeInfoCStyleCastExpr(Context, PtrToFuncCastType,
                                                 CK_BitCast, ME);
@@ -4676,9 +4673,8 @@ Stmt *RewriteModernObjC::RewriteBlockDeclRefExpr(DeclRefExpr *DeclRefExp) {
                                     Context->VoidPtrTy, nullptr,
                                     /*BitWidth=*/nullptr, /*Mutable=*/true,
                                     ICIS_NoInit);
-  MemberExpr *ME = new (Context)
+  MemberExpr *ME = MemberExpr::CreateImplicit(
-      MemberExpr(DeclRefExp, isArrow, SourceLocation(), FD, SourceLocation(),
+      *Context, DeclRefExp, isArrow, FD, FD->getType(), VK_LValue, OK_Ordinary);
-                 FD->getType(), VK_LValue, OK_Ordinary);
 
   StringRef Name = VD->getName();
   FD = FieldDecl::Create(*Context, nullptr, SourceLocation(), SourceLocation(),
@@ -4686,9 +4682,8 @@ Stmt *RewriteModernObjC::RewriteBlockDeclRefExpr(DeclRefExpr *DeclRefExp) {
                          Context->VoidPtrTy, nullptr,
                          /*BitWidth=*/nullptr, /*Mutable=*/true,
                          ICIS_NoInit);
-  ME =
+  ME = MemberExpr::CreateImplicit(*Context, ME, true, FD, DeclRefExp->getType(),
-      new (Context) MemberExpr(ME, true, SourceLocation(), FD, SourceLocation(),
+                                  VK_LValue, OK_Ordinary);
-                               DeclRefExp->getType(), VK_LValue, OK_Ordinary);
 
   // Need parens to enforce precedence.
   ParenExpr *PE = new (Context) ParenExpr(DeclRefExp->getExprLoc(),
@@ -7528,9 +7523,8 @@ Stmt *RewriteModernObjC::RewriteObjCIvarRefExpr(ObjCIvarRefExpr *IV) {
                                             IvarT, nullptr,
                                             /*BitWidth=*/nullptr,
                                             /*Mutable=*/true, ICIS_NoInit);
-          MemberExpr *ME = new (Context)
+          MemberExpr *ME = MemberExpr::CreateImplicit(
-              MemberExpr(PE, true, SourceLocation(), FD, SourceLocation(),
+              *Context, PE, true, FD, FD->getType(), VK_LValue, OK_Ordinary);
-                         FD->getType(), VK_LValue, OK_Ordinary);
           IvarT = Context->getDecltypeType(ME, ME->getType());
         }
       }
@@ -7557,9 +7551,9 @@ Stmt *RewriteModernObjC::RewriteObjCIvarRefExpr(ObjCIvarRefExpr *IV) {
                                           D->getType(), nullptr,
                                           /*BitWidth=*/D->getBitWidth(),
                                           /*Mutable=*/true, ICIS_NoInit);
-        MemberExpr *ME = new (Context)
+        MemberExpr *ME =
-            MemberExpr(PE, /*isArrow*/ false, SourceLocation(), FD,
+            MemberExpr::CreateImplicit(*Context, PE, /*isArrow*/ false, FD,
-                       SourceLocation(), FD->getType(), VK_LValue, OK_Ordinary);
+                                       FD->getType(), VK_LValue, OK_Ordinary);
         Replacement = ME;
 
       }

clang/lib/Frontend/Rewrite/RewriteObjC.cpp

@@ -3793,9 +3793,8 @@ Stmt *RewriteObjC::SynthesizeBlockCall(CallExpr *Exp, const Expr *BlockExp) {
                                     Context->VoidPtrTy, nullptr,
                                     /*BitWidth=*/nullptr, /*Mutable=*/true,
                                     ICIS_NoInit);
-  MemberExpr *ME =
+  MemberExpr *ME = MemberExpr::CreateImplicit(
-      new (Context) MemberExpr(PE, true, SourceLocation(), FD, SourceLocation(),
+      *Context, PE, true, FD, FD->getType(), VK_LValue, OK_Ordinary);
-                               FD->getType(), VK_LValue, OK_Ordinary);
 
   CastExpr *FunkCast = NoTypeInfoCStyleCastExpr(Context, PtrToFuncCastType,
                                                 CK_BitCast, ME);
@@ -3840,8 +3839,8 @@ Stmt *RewriteObjC::RewriteBlockDeclRefExpr(DeclRefExpr *DeclRefExp) {
                                     Context->VoidPtrTy, nullptr,
                                     /*BitWidth=*/nullptr, /*Mutable=*/true,
                                     ICIS_NoInit);
-  MemberExpr *ME = new (Context)
+  MemberExpr *ME =
-      MemberExpr(DeclRefExp, isArrow, SourceLocation(), FD, SourceLocation(),
+      MemberExpr::CreateImplicit(*Context, DeclRefExp, isArrow, FD,
                                  FD->getType(), VK_LValue, OK_Ordinary);
 
   StringRef Name = VD->getName();
@@ -3850,9 +3849,8 @@ Stmt *RewriteObjC::RewriteBlockDeclRefExpr(DeclRefExpr *DeclRefExp) {
                          Context->VoidPtrTy, nullptr,
                          /*BitWidth=*/nullptr, /*Mutable=*/true,
                          ICIS_NoInit);
-  ME =
+  ME = MemberExpr::CreateImplicit(*Context, ME, true, FD, DeclRefExp->getType(),
-      new (Context) MemberExpr(ME, true, SourceLocation(), FD, SourceLocation(),
+                                  VK_LValue, OK_Ordinary);
-                               DeclRefExp->getType(), VK_LValue, OK_Ordinary);
 
   // Need parens to enforce precedence.
   ParenExpr *PE = new (Context) ParenExpr(DeclRefExp->getExprLoc(),
@@ -5830,10 +5828,10 @@ Stmt *RewriteObjCFragileABI::RewriteObjCIvarRefExpr(ObjCIvarRefExpr *IV) {
                                               OldRange.getEnd(),
                                               castExpr);
       if (IV->isFreeIvar() &&
-          declaresSameEntity(CurMethodDef->getClassInterface(), iFaceDecl->getDecl())) {
+          declaresSameEntity(CurMethodDef->getClassInterface(),
-        MemberExpr *ME = new (Context)
+                             iFaceDecl->getDecl())) {
-            MemberExpr(PE, true, SourceLocation(), D, IV->getLocation(),
+        MemberExpr *ME = MemberExpr::CreateImplicit(
-                       D->getType(), VK_LValue, OK_Ordinary);
+            *Context, PE, true, D, D->getType(), VK_LValue, OK_Ordinary);
         Replacement = ME;
       } else {
         IV->setBase(PE);

clang/lib/Sema/SemaExprCXX.cpp

@@ -7189,9 +7189,12 @@ ExprResult Sema::BuildCXXMemberCallExpr(Expr *E, NamedDecl *FoundDecl,
     }
   }
 
-  MemberExpr *ME = new (Context) MemberExpr(
+  MemberExpr *ME = MemberExpr::Create(
-      Exp.get(), /*IsArrow=*/false, SourceLocation(), Method, SourceLocation(),
+      Context, Exp.get(), /*IsArrow=*/false, SourceLocation(),
-      Context.BoundMemberTy, VK_RValue, OK_Ordinary);
+      NestedNameSpecifierLoc(), SourceLocation(), Method,
+      DeclAccessPair::make(FoundDecl, FoundDecl->getAccess()),
+      DeclarationNameInfo(), /*TemplateArgs=*/nullptr, Context.BoundMemberTy,
+      VK_RValue, OK_Ordinary);
   if (HadMultipleCandidates)
     ME->setHadMultipleCandidates(true);
   MarkMemberReferenced(ME);

clang/lib/Serialization/ASTReaderStmt.cpp

@@ -752,9 +752,46 @@ void ASTStmtReader::VisitCXXMemberCallExpr(CXXMemberCallExpr *E) {
 }
 
 void ASTStmtReader::VisitMemberExpr(MemberExpr *E) {
-  // Don't call VisitExpr, this is fully initialized at creation.
+  VisitExpr(E);
-  assert(E->getStmtClass() == Stmt::MemberExprClass &&
+
-         "It's a subclass, we must advance Idx!");
+  bool HasQualifier = Record.readInt();
+  bool HasFoundDecl = Record.readInt();
+  bool HasTemplateInfo = Record.readInt();
+  unsigned NumTemplateArgs = Record.readInt();
+
+  E->Base = Record.readSubExpr();
+  E->MemberDecl = Record.readDeclAs<ValueDecl>();
+  Record.readDeclarationNameLoc(E->MemberDNLoc, E->MemberDecl->getDeclName());
+  E->MemberLoc = Record.readSourceLocation();
+  E->MemberExprBits.IsArrow = Record.readInt();
+  E->MemberExprBits.HasQualifierOrFoundDecl = HasQualifier || HasFoundDecl;
+  E->MemberExprBits.HasTemplateKWAndArgsInfo = HasTemplateInfo;
+  E->MemberExprBits.HadMultipleCandidates = Record.readInt();
+  E->MemberExprBits.OperatorLoc = Record.readSourceLocation();
+
+  if (HasQualifier || HasFoundDecl) {
+    DeclAccessPair FoundDecl;
+    if (HasFoundDecl) {
+      auto *FoundD = Record.readDeclAs<NamedDecl>();
+      auto AS = (AccessSpecifier)Record.readInt();
+      FoundDecl = DeclAccessPair::make(FoundD, AS);
+    } else {
+      FoundDecl = DeclAccessPair::make(E->MemberDecl,
+                                       E->MemberDecl->getAccess());
+    }
+    E->getTrailingObjects<MemberExprNameQualifier>()->FoundDecl = FoundDecl;
+
+    NestedNameSpecifierLoc QualifierLoc;
+    if (HasQualifier)
+      QualifierLoc = Record.readNestedNameSpecifierLoc();
+    E->getTrailingObjects<MemberExprNameQualifier>()->QualifierLoc =
+        QualifierLoc;
+  }
+
+  if (HasTemplateInfo)
+    ReadTemplateKWAndArgsInfo(
+        *E->getTrailingObjects<ASTTemplateKWAndArgsInfo>(),
+        E->getTrailingObjects<TemplateArgumentLoc>(), NumTemplateArgs);
 }
 
 void ASTStmtReader::VisitObjCIsaExpr(ObjCIsaExpr *E) {
@@ -2551,55 +2588,12 @@ Stmt *ASTReader::ReadStmtFromStream(ModuleFile &F) {
           Context, /*NumArgs=*/Record[ASTStmtReader::NumExprFields], Empty);
       break;
 
-    case EXPR_MEMBER: {
+    case EXPR_MEMBER:
-      // We load everything here and fully initialize it at creation.
+      S = MemberExpr::CreateEmpty(Context, Record[ASTStmtReader::NumExprFields],
-      // That way we can use MemberExpr::Create and don't have to duplicate its
+                                  Record[ASTStmtReader::NumExprFields + 1],
-      // logic with a MemberExpr::CreateEmpty.
+                                  Record[ASTStmtReader::NumExprFields + 2],
-
+                                  Record[ASTStmtReader::NumExprFields + 3]);
-      assert(Record.getIdx() == 0);
-      NestedNameSpecifierLoc QualifierLoc;
-      if (Record.readInt()) { // HasQualifier.
-        QualifierLoc = Record.readNestedNameSpecifierLoc();
-      }
-
-      SourceLocation TemplateKWLoc;
-      TemplateArgumentListInfo ArgInfo;
-      bool HasTemplateKWAndArgsInfo = Record.readInt();
-      if (HasTemplateKWAndArgsInfo) {
-        TemplateKWLoc = Record.readSourceLocation();
-        unsigned NumTemplateArgs = Record.readInt();
-        ArgInfo.setLAngleLoc(Record.readSourceLocation());
-        ArgInfo.setRAngleLoc(Record.readSourceLocation());
-        for (unsigned i = 0; i != NumTemplateArgs; ++i)
-          ArgInfo.addArgument(Record.readTemplateArgumentLoc());
-      }
-
-      bool HadMultipleCandidates = Record.readInt();
-
-      auto *FoundD = Record.readDeclAs<NamedDecl>();
-      auto AS = (AccessSpecifier)Record.readInt();
-      DeclAccessPair FoundDecl = DeclAccessPair::make(FoundD, AS);
-
-      QualType T = Record.readType();
-      auto VK = static_cast<ExprValueKind>(Record.readInt());
-      auto OK = static_cast<ExprObjectKind>(Record.readInt());
-      Expr *Base = ReadSubExpr();
-      auto *MemberD = Record.readDeclAs<ValueDecl>();
-      SourceLocation MemberLoc = Record.readSourceLocation();
-      DeclarationNameInfo MemberNameInfo(MemberD->getDeclName(), MemberLoc);
-      bool IsArrow = Record.readInt();
-      SourceLocation OperatorLoc = Record.readSourceLocation();
-
-      S = MemberExpr::Create(Context, Base, IsArrow, OperatorLoc, QualifierLoc,
-                             TemplateKWLoc, MemberD, FoundDecl, MemberNameInfo,
-                             HasTemplateKWAndArgsInfo ? &ArgInfo : nullptr, T,
-                             VK, OK);
-      Record.readDeclarationNameLoc(cast<MemberExpr>(S)->MemberDNLoc,
-                                    MemberD->getDeclName());
-      if (HadMultipleCandidates)
-        cast<MemberExpr>(S)->setHadMultipleCandidates(true);
       break;
-    }
 
     case EXPR_BINARY_OPERATOR:
       S = new (Context) BinaryOperator(Empty);

clang/lib/Serialization/ASTWriterStmt.cpp

@@ -660,39 +660,45 @@ void ASTStmtWriter::VisitCallExpr(CallExpr *E) {
 }
 
 void ASTStmtWriter::VisitMemberExpr(MemberExpr *E) {
-  // Don't call VisitExpr, we'll write everything here.
+  VisitExpr(E);
 
-  Record.push_back(E->hasQualifier());
+  bool HasQualifier = E->hasQualifier();
-  if (E->hasQualifier())
+  bool HasFoundDecl =
-    Record.AddNestedNameSpecifierLoc(E->getQualifierLoc());
+      E->hasQualifierOrFoundDecl() &&
-
+      (E->getFoundDecl().getDecl() != E->getMemberDecl() ||
-  Record.push_back(E->hasTemplateKWAndArgsInfo());
+       E->getFoundDecl().getAccess() != E->getMemberDecl()->getAccess());
-  if (E->hasTemplateKWAndArgsInfo()) {
+  bool HasTemplateInfo = E->hasTemplateKWAndArgsInfo();
-    Record.AddSourceLocation(E->getTemplateKeywordLoc());
   unsigned NumTemplateArgs = E->getNumTemplateArgs();
+
+  // Write these first for easy access when deserializing, as they affect the
+  // size of the MemberExpr.
+  Record.push_back(HasQualifier);
+  Record.push_back(HasFoundDecl);
+  Record.push_back(HasTemplateInfo);
   Record.push_back(NumTemplateArgs);
-    Record.AddSourceLocation(E->getLAngleLoc());
-    Record.AddSourceLocation(E->getRAngleLoc());
-    for (unsigned i=0; i != NumTemplateArgs; ++i)
-      Record.AddTemplateArgumentLoc(E->getTemplateArgs()[i]);
-  }
 
+  Record.AddStmt(E->getBase());
+  Record.AddDeclRef(E->getMemberDecl());
+  Record.AddDeclarationNameLoc(E->MemberDNLoc,
+                               E->getMemberDecl()->getDeclName());
+  Record.AddSourceLocation(E->getMemberLoc());
+  Record.push_back(E->isArrow());
   Record.push_back(E->hadMultipleCandidates());
+  Record.AddSourceLocation(E->getOperatorLoc());
 
+  if (HasFoundDecl) {
     DeclAccessPair FoundDecl = E->getFoundDecl();
     Record.AddDeclRef(FoundDecl.getDecl());
     Record.push_back(FoundDecl.getAccess());
+  }
+
+  if (HasQualifier)
+    Record.AddNestedNameSpecifierLoc(E->getQualifierLoc());
+
+  if (HasTemplateInfo)
+    AddTemplateKWAndArgsInfo(*E->getTrailingObjects<ASTTemplateKWAndArgsInfo>(),
+                             E->getTrailingObjects<TemplateArgumentLoc>());
 
-  Record.AddTypeRef(E->getType());
-  Record.push_back(E->getValueKind());
-  Record.push_back(E->getObjectKind());
-  Record.AddStmt(E->getBase());
-  Record.AddDeclRef(E->getMemberDecl());
-  Record.AddSourceLocation(E->getMemberLoc());
-  Record.push_back(E->isArrow());
-  Record.AddSourceLocation(E->getOperatorLoc());
-  Record.AddDeclarationNameLoc(E->MemberDNLoc,
-                               E->getMemberDecl()->getDeclName());
   Code = serialization::EXPR_MEMBER;
 }
 

clang/test/PCH/cxx-templates.cpp

@@ -156,3 +156,12 @@ namespace ClassScopeExplicitSpecializations {
   static_assert(A<4>().f<0>() == 2, "");
   static_assert(A<4>().f<1>() == 1, "");
 }
+
+namespace DependentMemberExpr {
+#ifndef NO_ERRORS
+  // This used to mark 'f' invalid without producing any diagnostic. That's a
+  // little hard to detect, but we can make sure that constexpr evaluation
+  // fails when it should.
+  static_assert(A<int>().f() == 1); // expected-error {{static_assert failed}}
+#endif
+}

clang/test/PCH/cxx-templates.h

@@ -439,3 +439,12 @@ namespace ClassScopeExplicitSpecializations {
   template<> template<> constexpr int B<0>::w<int> = 7;
   template<> template<> constexpr int B<0>::w<float> = 8;
 }
+
+namespace DependentMemberExpr {
+  struct Base {
+    constexpr int setstate() { return 0; }
+  };
+  template<typename T> struct A : Base {
+    constexpr int f() { return Base::setstate(); }
+  };
+}