r269858 - PR27754: CXXRecordDecl::data() needs to perform an update even if it's called

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Tue May 17 16:02:12 PDT 2016


Forgot to mention in the commit message: this patch was collaboratively
produced by Vassil Vassilev and myself.

On Tue, May 17, 2016 at 3:44 PM, Richard Smith via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> Author: rsmith
> Date: Tue May 17 17:44:15 2016
> New Revision: 269858
>
> URL: http://llvm.org/viewvc/llvm-project?rev=269858&view=rev
> Log:
> PR27754: CXXRecordDecl::data() needs to perform an update even if it's
> called
> on a declaration that already knows the location of the DefinitionData
> object.
>
> Added:
>     cfe/trunk/test/Modules/Inputs/PR27754/
>     cfe/trunk/test/Modules/Inputs/PR27754/RConversionRuleParser.h
>     cfe/trunk/test/Modules/Inputs/PR27754/TMetaUtils.h
>     cfe/trunk/test/Modules/Inputs/PR27754/TSchemaType.h
>     cfe/trunk/test/Modules/Inputs/PR27754/algobase.h
>     cfe/trunk/test/Modules/Inputs/PR27754/module.modulemap
>     cfe/trunk/test/Modules/pr27754.cpp
> Modified:
>     cfe/trunk/include/clang/AST/DeclCXX.h
>     cfe/trunk/lib/AST/DeclCXX.cpp
>     cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
>
> Modified: cfe/trunk/include/clang/AST/DeclCXX.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=269858&r1=269857&r2=269858&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/AST/DeclCXX.h (original)
> +++ cfe/trunk/include/clang/AST/DeclCXX.h Tue May 17 17:44:15 2016
> @@ -257,30 +257,6 @@ public:
>    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 {
>
> @@ -543,11 +519,7 @@ class CXXRecordDecl : public RecordDecl
>      CXXBaseSpecifier *getVBasesSlowCase() const;
>    };
>
> -  typedef LazyDefinitionDataPtr<CXXRecordDecl, struct DefinitionData>
> -      DefinitionDataPtr;
> -  friend class LazyDefinitionDataPtr<CXXRecordDecl, struct
> DefinitionData>;
> -
> -  mutable DefinitionDataPtr DefinitionData;
> +  struct DefinitionData *DefinitionData;
>
>    /// \brief Describes a C++ closure type (generated by a lambda
> expression).
>    struct LambdaDefinitionData : public DefinitionData {
> @@ -610,8 +582,14 @@ class CXXRecordDecl : public RecordDecl
>
>    };
>
> +  struct DefinitionData *dataPtr() const {
> +    // Complete the redecl chain (if necessary).
> +    getMostRecentDecl();
> +    return DefinitionData;
> +  }
> +
>    struct DefinitionData &data() const {
> -    auto *DD = DefinitionData.get();
> +    auto *DD = dataPtr();
>      assert(DD && "queried property of class with no definition");
>      return *DD;
>    }
> @@ -619,7 +597,7 @@ class CXXRecordDecl : public RecordDecl
>    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);
>    }
> @@ -696,11 +674,13 @@ public:
>    }
>
>    CXXRecordDecl *getDefinition() const {
> -    auto *DD = DefinitionData.get();
> +    // We only need an update if we don't already know which
> +    // declaration is the definition.
> +    auto *DD = DefinitionData ? DefinitionData : dataPtr();
>      return DD ? DD->Definition : nullptr;
>    }
>
> -  bool hasDefinition() const { return DefinitionData.get(); }
> +  bool hasDefinition() const { return DefinitionData || dataPtr(); }
>
>    static CXXRecordDecl *Create(const ASTContext &C, TagKind TK,
> DeclContext *DC,
>                                 SourceLocation StartLoc, SourceLocation
> IdLoc,
> @@ -1044,7 +1024,7 @@ public:
>    /// \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;
>    }
>
>
> Modified: cfe/trunk/lib/AST/DeclCXX.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclCXX.cpp?rev=269858&r1=269857&r2=269858&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/AST/DeclCXX.cpp (original)
> +++ cfe/trunk/lib/AST/DeclCXX.cpp Tue May 17 17:44:15 2016
> @@ -88,7 +88,7 @@ CXXRecordDecl::CXXRecordDecl(Kind K, Tag
>                               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,
>
> Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=269858&r1=269857&r2=269858&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)
> +++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Tue May 17 17:44:15 2016
> @@ -1540,9 +1540,9 @@ void ASTDeclReader::ReadCXXDefinitionDat
>
>  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.
> @@ -1665,7 +1665,7 @@ void ASTDeclReader::ReadCXXRecordDefinit
>    // 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;
> @@ -2001,8 +2001,8 @@ ASTDeclReader::VisitClassTemplateSpecial
>
>          // 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;
> @@ -2326,8 +2326,8 @@ void ASTDeclReader::mergeTemplatePattern
>      // 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;
> @@ -2765,9 +2765,9 @@ DeclContext *ASTDeclReader::getPrimaryCo
>
>    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
> @@ -3772,7 +3772,7 @@ void ASTDeclReader::UpdateDecl(Decl *D,
>
>      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));
>
> Added: cfe/trunk/test/Modules/Inputs/PR27754/RConversionRuleParser.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR27754/RConversionRuleParser.h?rev=269858&view=auto
>
> ==============================================================================
> --- cfe/trunk/test/Modules/Inputs/PR27754/RConversionRuleParser.h (added)
> +++ cfe/trunk/test/Modules/Inputs/PR27754/RConversionRuleParser.h Tue May
> 17 17:44:15 2016
> @@ -0,0 +1,4 @@
> +#include "algobase.h"
> +typedef integral_constant<bool, true> true_type;
> +class _Rb_tree { _Rb_tree() { true_type(); } };
> +#include "TSchemaType.h"
>
> Added: cfe/trunk/test/Modules/Inputs/PR27754/TMetaUtils.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR27754/TMetaUtils.h?rev=269858&view=auto
>
> ==============================================================================
> --- cfe/trunk/test/Modules/Inputs/PR27754/TMetaUtils.h (added)
> +++ cfe/trunk/test/Modules/Inputs/PR27754/TMetaUtils.h Tue May 17 17:44:15
> 2016
> @@ -0,0 +1,2 @@
> +#include "RConversionRuleParser.h"
> +void fn1() { true_type(); }
>
> Added: cfe/trunk/test/Modules/Inputs/PR27754/TSchemaType.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR27754/TSchemaType.h?rev=269858&view=auto
>
> ==============================================================================
> --- cfe/trunk/test/Modules/Inputs/PR27754/TSchemaType.h (added)
> +++ cfe/trunk/test/Modules/Inputs/PR27754/TSchemaType.h Tue May 17
> 17:44:15 2016
> @@ -0,0 +1,2 @@
> +#include "algobase.h"
> +struct A : integral_constant<bool, true> {};
>
> Added: cfe/trunk/test/Modules/Inputs/PR27754/algobase.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR27754/algobase.h?rev=269858&view=auto
>
> ==============================================================================
> --- cfe/trunk/test/Modules/Inputs/PR27754/algobase.h (added)
> +++ cfe/trunk/test/Modules/Inputs/PR27754/algobase.h Tue May 17 17:44:15
> 2016
> @@ -0,0 +1,4 @@
> +#ifndef _STL_ALGOBASE_H
> +#define _STL_ALGOBASE_H
> +template<typename _Tp, _Tp> struct integral_constant {};
> +#endif
>
> Added: cfe/trunk/test/Modules/Inputs/PR27754/module.modulemap
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR27754/module.modulemap?rev=269858&view=auto
>
> ==============================================================================
> --- cfe/trunk/test/Modules/Inputs/PR27754/module.modulemap (added)
> +++ cfe/trunk/test/Modules/Inputs/PR27754/module.modulemap Tue May 17
> 17:44:15 2016
> @@ -0,0 +1,3 @@
> +module "RConversionRuleParser.h" { header "RConversionRuleParser.h" }
> +module "TMetaUtils.h" { header "TMetaUtils.h" }
> +module "TSchemaType.h" { header "TSchemaType.h" }
>
> Added: cfe/trunk/test/Modules/pr27754.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/pr27754.cpp?rev=269858&view=auto
>
> ==============================================================================
> --- cfe/trunk/test/Modules/pr27754.cpp (added)
> +++ cfe/trunk/test/Modules/pr27754.cpp Tue May 17 17:44:15 2016
> @@ -0,0 +1,7 @@
> +// 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
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160517/428b925d/attachment-0001.html>


More information about the cfe-commits mailing list