Remove LazyDefinitionDataPtr and rely on getMostRecentDecl. Fixes PR27754.

[rsmith]

include/clang/AST/DeclCXX.h

522

Does this still need to be mutable?

Also, you need to use struct DefinitionData *DefinitionData; here or this is ill-formed (no diagnostic required) by [basic.scope.class]p1 rule 2.

672

This also needs to perform an update. Maybe factor out a function for this and data() to call?

676

Likewise, we need an update here if DefinitionData is null.

[rsmith]

Review comments resolved and committed as r269858.

include/clang/AST/DeclCXX.h

public:
   QualType getType() const {
     return BaseTypeInfo->getType().getUnqualifiedType();
   }
 
   /// \brief Retrieves the type and source location of the base class.
   TypeSourceInfo *getTypeSourceInfo() const { return BaseTypeInfo; }
 };
 
-/// \brief A lazy pointer to the definition data for a declaration.
-/// FIXME: This is a little CXXRecordDecl-specific that the moment.
-template<typename Decl, typename T> class LazyDefinitionDataPtr {
-  llvm::PointerUnion<T *, Decl *> DataOrCanonicalDecl;
-
-  LazyDefinitionDataPtr update() {
-    if (Decl *Canon = DataOrCanonicalDecl.template dyn_cast<Decl*>()) {
-      if (Canon->isCanonicalDecl())
-        Canon->getMostRecentDecl();
-      else
-        // Declaration isn't canonical any more;
-        // update it and perform path compression.
-        *this = Canon->getPreviousDecl()->DefinitionData.update();
-    }
-    return *this;
-  }
-
-public:
-  LazyDefinitionDataPtr(Decl *Canon) : DataOrCanonicalDecl(Canon) {}
-  LazyDefinitionDataPtr(T *Data) : DataOrCanonicalDecl(Data) {}
-  T *getNotUpdated() { return DataOrCanonicalDecl.template dyn_cast<T*>(); }
-  T *get() { return update().getNotUpdated(); }
-};
-
 /// \brief Represents a C++ struct/union/class.
 class CXXRecordDecl : public RecordDecl {
 
   friend void TagDecl::startDefinition();
 
   /// Values used in DefinitionData fields to represent special members.
   enum SpecialMemberFlags {
     SMF_DefaultConstructor = 0x1,
ArrayRef<CXXBaseSpecifier> vbases() const {
       return llvm::makeArrayRef(getVBases(), NumVBases);
     }
 
   private:
     CXXBaseSpecifier *getBasesSlowCase() const;
     CXXBaseSpecifier *getVBasesSlowCase() const;
   };
 
-  typedef LazyDefinitionDataPtr<CXXRecordDecl, struct DefinitionData>
-      DefinitionDataPtr;
-  friend class LazyDefinitionDataPtr<CXXRecordDecl, struct DefinitionData>;
-
-  mutable DefinitionDataPtr DefinitionData;
+  mutable DefinitionData* DefinitionData;
 
   /// \brief Describes a C++ closure type (generated by a lambda expression).
   struct LambdaDefinitionData : public DefinitionData {
     typedef LambdaCapture Capture;
 
     LambdaDefinitionData(CXXRecordDecl *D, TypeSourceInfo *Info, 
                          bool Dependent, bool IsGeneric, 
                          LambdaCaptureDefault CaptureDefault) 
struct LambdaDefinitionData : public DefinitionData {
     Capture *Captures;
 
     /// \brief The type of the call method.
     TypeSourceInfo *MethodTyInfo;
        
   };
 
   struct DefinitionData &data() const {
-    auto *DD = DefinitionData.get();
+    // Complete the redecl chain reading all necessary information.
+    auto *DD = getMostRecentDecl()->DefinitionData;
     assert(DD && "queried property of class with no definition");
     return *DD;
   }
 
   struct LambdaDefinitionData &getLambdaData() const {
     // No update required: a merged definition cannot change any lambda
     // properties.
-    auto *DD = DefinitionData.getNotUpdated();
+    auto *DD = DefinitionData;
     assert(DD && DD->IsLambda && "queried lambda property of non-lambda class");
     return static_cast<LambdaDefinitionData&>(*DD);
   }
 
   /// \brief The template or declaration that this declaration
   /// describes or was instantiated from, respectively.
   ///
   /// For non-templates, this value will be null. For record
return cast<CXXRecordDecl>(
             static_cast<RecordDecl *>(this)->getMostRecentDecl());
   }
 
   const CXXRecordDecl *getMostRecentDecl() const {
     return const_cast<CXXRecordDecl*>(this)->getMostRecentDecl();
   }
 
   CXXRecordDecl *getDefinition() const {
-    auto *DD = DefinitionData.get();
+    auto *DD = DefinitionData;
     return DD ? DD->Definition : nullptr;
   }
 
-  bool hasDefinition() const { return DefinitionData.get(); }
+  bool hasDefinition() const { return DefinitionData; }
 
   static CXXRecordDecl *Create(const ASTContext &C, TagKind TK, DeclContext *DC,
                                SourceLocation StartLoc, SourceLocation IdLoc,
                                IdentifierInfo *Id,
                                CXXRecordDecl *PrevDecl = nullptr,
                                bool DelayTypeCreation = false);
   static CXXRecordDecl *CreateLambda(const ASTContext &C, DeclContext *DC,
                                      TypeSourceInfo *Info, SourceLocation Loc,
public:
   /// class.
   bool needsOverloadResolutionForDestructor() const {
     return data().NeedOverloadResolutionForDestructor;
   }
 
   /// \brief Determine whether this class describes a lambda function object.
   bool isLambda() const {
     // An update record can't turn a non-lambda into a lambda.
-    auto *DD = DefinitionData.getNotUpdated();
+    auto *DD = DefinitionData;
     return DD && DD->IsLambda;
   }
 
   /// \brief Determine whether this class describes a generic 
   /// lambda function object (i.e. function call operator is
   /// a template). 
   bool isGenericLambda() const; 
 

lib/AST/DeclCXX.cpp

 }
 
 CXXRecordDecl::CXXRecordDecl(Kind K, TagKind TK, const ASTContext &C,
                              DeclContext *DC, SourceLocation StartLoc,
                              SourceLocation IdLoc, IdentifierInfo *Id,
                              CXXRecordDecl *PrevDecl)
     : RecordDecl(K, TK, C, DC, StartLoc, IdLoc, Id, PrevDecl),
       DefinitionData(PrevDecl ? PrevDecl->DefinitionData
-                              : DefinitionDataPtr(this)),
+                              : nullptr),
       TemplateOrInstantiation() {}
 
 CXXRecordDecl *CXXRecordDecl::Create(const ASTContext &C, TagKind TK,
                                      DeclContext *DC, SourceLocation StartLoc,
                                      SourceLocation IdLoc, IdentifierInfo *Id,
                                      CXXRecordDecl* PrevDecl,
                                      bool DelayTypeCreation) {
   CXXRecordDecl *R = new (C, DC) CXXRecordDecl(CXXRecord, TK, C, DC, StartLoc,

lib/Serialization/ASTReaderDecl.cpp

for (unsigned I = 0, N = Lambda.NumCaptures; I != N; ++I) {
         break;
       }
     }
   }
 }
 
 void ASTDeclReader::MergeDefinitionData(
     CXXRecordDecl *D, struct CXXRecordDecl::DefinitionData &&MergeDD) {
-  assert(D->DefinitionData.getNotUpdated() &&
+  assert(D->DefinitionData &&
          "merging class definition into non-definition");
-  auto &DD = *D->DefinitionData.getNotUpdated();
+  auto &DD = *D->DefinitionData;
 
   if (DD.Definition != MergeDD.Definition) {
     // Track that we merged the definitions.
     Reader.MergedDeclContexts.insert(std::make_pair(MergeDD.Definition,
                                                     DD.Definition));
     Reader.PendingDefinitions.erase(MergeDD.Definition);
     MergeDD.Definition->IsCompleteDefinition = false;
     mergeDefinitionVisibility(DD.Definition, MergeDD.Definition);
else
     DD = new (C) struct CXXRecordDecl::DefinitionData(D);
 
   ReadCXXDefinitionData(*DD, Record, Idx);
 
   // We might already have a definition for this record. This can happen either
   // because we're reading an update record, or because we've already done some
   // merging. Either way, just merge into it.
   CXXRecordDecl *Canon = D->getCanonicalDecl();
-  if (Canon->DefinitionData.getNotUpdated()) {
+  if (Canon->DefinitionData) {
     MergeDefinitionData(Canon, std::move(*DD));
     D->DefinitionData = Canon->DefinitionData;
     return;
   }
 
   // Mark this declaration as being a definition.
   D->IsCompleteDefinition = true;
   D->DefinitionData = DD;
if (D->isCanonicalDecl()) { // It's kept in the folding set.
             CanonPattern->getCommonPtr()->Specializations.GetOrInsertNode(D);
       }
       // If there was already a canonical specialization, merge into it.
       if (CanonSpec != D) {
         mergeRedeclarable<TagDecl>(D, CanonSpec, Redecl);
 
         // This declaration might be a definition. Merge with any existing
         // definition.
-        if (auto *DDD = D->DefinitionData.getNotUpdated()) {
-          if (CanonSpec->DefinitionData.getNotUpdated())
+        if (auto *DDD = D->DefinitionData) {
+          if (CanonSpec->DefinitionData)
             MergeDefinitionData(CanonSpec, std::move(*DDD));
           else
             CanonSpec->DefinitionData = D->DefinitionData;
         }
         D->DefinitionData = CanonSpec->DefinitionData;
       }
     }
   }
void ASTDeclReader::mergeTemplatePattern(RedeclarableTemplateDecl *D,
   RedeclarableResult Result(DPattern->getCanonicalDecl()->getGlobalID(),
                             /*MergeWith*/ ExistingPattern, IsKeyDecl);
 
   if (auto *DClass = dyn_cast<CXXRecordDecl>(DPattern)) {
     // Merge with any existing definition.
     // FIXME: This is duplicated in several places. Refactor.
     auto *ExistingClass =
         cast<CXXRecordDecl>(ExistingPattern)->getCanonicalDecl();
-    if (auto *DDD = DClass->DefinitionData.getNotUpdated()) {
-      if (ExistingClass->DefinitionData.getNotUpdated()) {
+    if (auto *DDD = DClass->DefinitionData) {
+      if (ExistingClass->DefinitionData) {
         MergeDefinitionData(ExistingClass, std::move(*DDD));
       } else {
         ExistingClass->DefinitionData = DClass->DefinitionData;
         // We may have skipped this before because we thought that DClass
         // was the canonical declaration.
         Reader.PendingDefinitions.insert(DClass);
       }
     }

 /// looking for declarations to merge.
 DeclContext *ASTDeclReader::getPrimaryContextForMerging(ASTReader &Reader,
                                                         DeclContext *DC) {
   if (NamespaceDecl *ND = dyn_cast<NamespaceDecl>(DC))
     return ND->getOriginalNamespace();
 
   if (CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(DC)) {
     // Try to dig out the definition.
-    auto *DD = RD->DefinitionData.getNotUpdated();
+    auto *DD = RD->DefinitionData;
     if (!DD)
-      DD = RD->getCanonicalDecl()->DefinitionData.getNotUpdated();
+      DD = RD->getCanonicalDecl()->DefinitionData;
 
     // If there's no definition yet, then DC's definition is added by an update
     // record, but we've not yet loaded that update record. In this case, we
     // commit to DC being the canonical definition now, and will fix this when
     // we load the update record.
     if (!DD) {
       DD = new (Reader.Context) struct CXXRecordDecl::DefinitionData(RD);
       RD->IsCompleteDefinition = true;
case UPD_CXX_ADDED_FUNCTION_DEFINITION: {
       Reader.PendingBodies[FD] = GetCurrentCursorOffset();
       HasPendingBody = true;
       assert(Idx == Record.size() && "lazy body must be last");
       break;
     }
 
     case UPD_CXX_INSTANTIATED_CLASS_DEFINITION: {
       auto *RD = cast<CXXRecordDecl>(D);
-      auto *OldDD = RD->getCanonicalDecl()->DefinitionData.getNotUpdated();
+      auto *OldDD = RD->getCanonicalDecl()->DefinitionData;
       bool HadRealDefinition =
           OldDD && (OldDD->Definition != RD ||
                     !Reader.PendingFakeDefinitionData.count(OldDD));
       ReadCXXRecordDefinition(RD, /*Update*/true);
 
       // Visible update is handled separately.
       uint64_t LexicalOffset = ReadLocalOffset(Record, Idx);
       if (!HadRealDefinition && LexicalOffset) {

test/Modules/Inputs/PR27754/RConversionRuleParser.h

+#define _STL_ALGOBASE_H
+template < typename _Tp, _Tp > struct integral_constant
+{
+};
+typedef integral_constant < bool, true > true_type;
+#include <map>
+#include "TSchemaType.h"

test/Modules/Inputs/PR27754/TMetaUtils.h

+#include "RConversionRuleParser.h"
+void fn1() { true_type(); }

test/Modules/Inputs/PR27754/TSchemaType.h

+#ifndef _STL_ALGOBASE_H
+template <typename _Tp, _Tp> struct integral_constant {};
+struct A : integral_constant<bool, true> {};
+#endif

test/Modules/Inputs/PR27754/map

+class _Rb_tree {
+  _Rb_tree() { true_type(); }
+};

test/Modules/Inputs/PR27754/module.modulemap

+module "RConversionRuleParser.h" { header "RConversionRuleParser.h" }
+module "TMetaUtils.h" { header "TMetaUtils.h" }
+module "TSchemaType.h" { header "TSchemaType.h" }

test/Modules/pr27754.cpp

+// RUN: rm -rf %t
+// RUN: %clang_cc1 -std=c++11 -I%S/Inputs/PR27754 -verify %s
+// RUN: %clang_cc1 -std=c++11 -fmodules -fmodule-map-file=%S/Inputs/PR27754/module.modulemap -fmodules-cache-path=%t -I%S/Inputs/PR27754/ -verify %s
+
+#include "TMetaUtils.h"
+
+// expected-no-diagnostics