<div dir="ltr">Forgot to mention in the commit message: this patch was collaboratively produced by Vassil Vassilev and myself.</div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, May 17, 2016 at 3:44 PM, Richard Smith via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: rsmith<br>
Date: Tue May 17 17:44:15 2016<br>
New Revision: 269858<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=269858&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=269858&view=rev</a><br>
Log:<br>
PR27754: CXXRecordDecl::data() needs to perform an update even if it's called<br>
on a declaration that already knows the location of the DefinitionData object.<br>
<br>
Added:<br>
cfe/trunk/test/Modules/Inputs/PR27754/<br>
cfe/trunk/test/Modules/Inputs/PR27754/RConversionRuleParser.h<br>
cfe/trunk/test/Modules/Inputs/PR27754/TMetaUtils.h<br>
cfe/trunk/test/Modules/Inputs/PR27754/TSchemaType.h<br>
cfe/trunk/test/Modules/Inputs/PR27754/algobase.h<br>
cfe/trunk/test/Modules/Inputs/PR27754/module.modulemap<br>
cfe/trunk/test/Modules/pr27754.cpp<br>
Modified:<br>
cfe/trunk/include/clang/AST/DeclCXX.h<br>
cfe/trunk/lib/AST/DeclCXX.cpp<br>
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp<br>
<br>
Modified: cfe/trunk/include/clang/AST/DeclCXX.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=269858&r1=269857&r2=269858&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=269858&r1=269857&r2=269858&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/AST/DeclCXX.h (original)<br>
+++ cfe/trunk/include/clang/AST/DeclCXX.h Tue May 17 17:44:15 2016<br>
@@ -257,30 +257,6 @@ public:<br>
TypeSourceInfo *getTypeSourceInfo() const { return BaseTypeInfo; }<br>
};<br>
<br>
-/// \brief A lazy pointer to the definition data for a declaration.<br>
-/// FIXME: This is a little CXXRecordDecl-specific that the moment.<br>
-template<typename Decl, typename T> class LazyDefinitionDataPtr {<br>
- llvm::PointerUnion<T *, Decl *> DataOrCanonicalDecl;<br>
-<br>
- LazyDefinitionDataPtr update() {<br>
- if (Decl *Canon = DataOrCanonicalDecl.template dyn_cast<Decl*>()) {<br>
- if (Canon->isCanonicalDecl())<br>
- Canon->getMostRecentDecl();<br>
- else<br>
- // Declaration isn't canonical any more;<br>
- // update it and perform path compression.<br>
- *this = Canon->getPreviousDecl()->DefinitionData.update();<br>
- }<br>
- return *this;<br>
- }<br>
-<br>
-public:<br>
- LazyDefinitionDataPtr(Decl *Canon) : DataOrCanonicalDecl(Canon) {}<br>
- LazyDefinitionDataPtr(T *Data) : DataOrCanonicalDecl(Data) {}<br>
- T *getNotUpdated() { return DataOrCanonicalDecl.template dyn_cast<T*>(); }<br>
- T *get() { return update().getNotUpdated(); }<br>
-};<br>
-<br>
/// \brief Represents a C++ struct/union/class.<br>
class CXXRecordDecl : public RecordDecl {<br>
<br>
@@ -543,11 +519,7 @@ class CXXRecordDecl : public RecordDecl<br>
CXXBaseSpecifier *getVBasesSlowCase() const;<br>
};<br>
<br>
- typedef LazyDefinitionDataPtr<CXXRecordDecl, struct DefinitionData><br>
- DefinitionDataPtr;<br>
- friend class LazyDefinitionDataPtr<CXXRecordDecl, struct DefinitionData>;<br>
-<br>
- mutable DefinitionDataPtr DefinitionData;<br>
+ struct DefinitionData *DefinitionData;<br>
<br>
/// \brief Describes a C++ closure type (generated by a lambda expression).<br>
struct LambdaDefinitionData : public DefinitionData {<br>
@@ -610,8 +582,14 @@ class CXXRecordDecl : public RecordDecl<br>
<br>
};<br>
<br>
+ struct DefinitionData *dataPtr() const {<br>
+ // Complete the redecl chain (if necessary).<br>
+ getMostRecentDecl();<br>
+ return DefinitionData;<br>
+ }<br>
+<br>
struct DefinitionData &data() const {<br>
- auto *DD = DefinitionData.get();<br>
+ auto *DD = dataPtr();<br>
assert(DD && "queried property of class with no definition");<br>
return *DD;<br>
}<br>
@@ -619,7 +597,7 @@ class CXXRecordDecl : public RecordDecl<br>
struct LambdaDefinitionData &getLambdaData() const {<br>
// No update required: a merged definition cannot change any lambda<br>
// properties.<br>
- auto *DD = DefinitionData.getNotUpdated();<br>
+ auto *DD = DefinitionData;<br>
assert(DD && DD->IsLambda && "queried lambda property of non-lambda class");<br>
return static_cast<LambdaDefinitionData&>(*DD);<br>
}<br>
@@ -696,11 +674,13 @@ public:<br>
}<br>
<br>
CXXRecordDecl *getDefinition() const {<br>
- auto *DD = DefinitionData.get();<br>
+ // We only need an update if we don't already know which<br>
+ // declaration is the definition.<br>
+ auto *DD = DefinitionData ? DefinitionData : dataPtr();<br>
return DD ? DD->Definition : nullptr;<br>
}<br>
<br>
- bool hasDefinition() const { return DefinitionData.get(); }<br>
+ bool hasDefinition() const { return DefinitionData || dataPtr(); }<br>
<br>
static CXXRecordDecl *Create(const ASTContext &C, TagKind TK, DeclContext *DC,<br>
SourceLocation StartLoc, SourceLocation IdLoc,<br>
@@ -1044,7 +1024,7 @@ public:<br>
/// \brief Determine whether this class describes a lambda function object.<br>
bool isLambda() const {<br>
// An update record can't turn a non-lambda into a lambda.<br>
- auto *DD = DefinitionData.getNotUpdated();<br>
+ auto *DD = DefinitionData;<br>
return DD && DD->IsLambda;<br>
}<br>
<br>
<br>
Modified: cfe/trunk/lib/AST/DeclCXX.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclCXX.cpp?rev=269858&r1=269857&r2=269858&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclCXX.cpp?rev=269858&r1=269857&r2=269858&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/AST/DeclCXX.cpp (original)<br>
+++ cfe/trunk/lib/AST/DeclCXX.cpp Tue May 17 17:44:15 2016<br>
@@ -88,7 +88,7 @@ CXXRecordDecl::CXXRecordDecl(Kind K, Tag<br>
CXXRecordDecl *PrevDecl)<br>
: RecordDecl(K, TK, C, DC, StartLoc, IdLoc, Id, PrevDecl),<br>
DefinitionData(PrevDecl ? PrevDecl->DefinitionData<br>
- : DefinitionDataPtr(this)),<br>
+ : nullptr),<br>
TemplateOrInstantiation() {}<br>
<br>
CXXRecordDecl *CXXRecordDecl::Create(const ASTContext &C, TagKind TK,<br>
<br>
Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=269858&r1=269857&r2=269858&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=269858&r1=269857&r2=269858&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)<br>
+++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Tue May 17 17:44:15 2016<br>
@@ -1540,9 +1540,9 @@ void ASTDeclReader::ReadCXXDefinitionDat<br>
<br>
void ASTDeclReader::MergeDefinitionData(<br>
CXXRecordDecl *D, struct CXXRecordDecl::DefinitionData &&MergeDD) {<br>
- assert(D->DefinitionData.getNotUpdated() &&<br>
+ assert(D->DefinitionData &&<br>
"merging class definition into non-definition");<br>
- auto &DD = *D->DefinitionData.getNotUpdated();<br>
+ auto &DD = *D->DefinitionData;<br>
<br>
if (DD.Definition != MergeDD.Definition) {<br>
// Track that we merged the definitions.<br>
@@ -1665,7 +1665,7 @@ void ASTDeclReader::ReadCXXRecordDefinit<br>
// because we're reading an update record, or because we've already done some<br>
// merging. Either way, just merge into it.<br>
CXXRecordDecl *Canon = D->getCanonicalDecl();<br>
- if (Canon->DefinitionData.getNotUpdated()) {<br>
+ if (Canon->DefinitionData) {<br>
MergeDefinitionData(Canon, std::move(*DD));<br>
D->DefinitionData = Canon->DefinitionData;<br>
return;<br>
@@ -2001,8 +2001,8 @@ ASTDeclReader::VisitClassTemplateSpecial<br>
<br>
// This declaration might be a definition. Merge with any existing<br>
// definition.<br>
- if (auto *DDD = D->DefinitionData.getNotUpdated()) {<br>
- if (CanonSpec->DefinitionData.getNotUpdated())<br>
+ if (auto *DDD = D->DefinitionData) {<br>
+ if (CanonSpec->DefinitionData)<br>
MergeDefinitionData(CanonSpec, std::move(*DDD));<br>
else<br>
CanonSpec->DefinitionData = D->DefinitionData;<br>
@@ -2326,8 +2326,8 @@ void ASTDeclReader::mergeTemplatePattern<br>
// FIXME: This is duplicated in several places. Refactor.<br>
auto *ExistingClass =<br>
cast<CXXRecordDecl>(ExistingPattern)->getCanonicalDecl();<br>
- if (auto *DDD = DClass->DefinitionData.getNotUpdated()) {<br>
- if (ExistingClass->DefinitionData.getNotUpdated()) {<br>
+ if (auto *DDD = DClass->DefinitionData) {<br>
+ if (ExistingClass->DefinitionData) {<br>
MergeDefinitionData(ExistingClass, std::move(*DDD));<br>
} else {<br>
ExistingClass->DefinitionData = DClass->DefinitionData;<br>
@@ -2765,9 +2765,9 @@ DeclContext *ASTDeclReader::getPrimaryCo<br>
<br>
if (CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(DC)) {<br>
// Try to dig out the definition.<br>
- auto *DD = RD->DefinitionData.getNotUpdated();<br>
+ auto *DD = RD->DefinitionData;<br>
if (!DD)<br>
- DD = RD->getCanonicalDecl()->DefinitionData.getNotUpdated();<br>
+ DD = RD->getCanonicalDecl()->DefinitionData;<br>
<br>
// If there's no definition yet, then DC's definition is added by an update<br>
// record, but we've not yet loaded that update record. In this case, we<br>
@@ -3772,7 +3772,7 @@ void ASTDeclReader::UpdateDecl(Decl *D,<br>
<br>
case UPD_CXX_INSTANTIATED_CLASS_DEFINITION: {<br>
auto *RD = cast<CXXRecordDecl>(D);<br>
- auto *OldDD = RD->getCanonicalDecl()->DefinitionData.getNotUpdated();<br>
+ auto *OldDD = RD->getCanonicalDecl()->DefinitionData;<br>
bool HadRealDefinition =<br>
OldDD && (OldDD->Definition != RD ||<br>
!Reader.PendingFakeDefinitionData.count(OldDD));<br>
<br>
Added: cfe/trunk/test/Modules/Inputs/PR27754/RConversionRuleParser.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR27754/RConversionRuleParser.h?rev=269858&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR27754/RConversionRuleParser.h?rev=269858&view=auto</a><br>
==============================================================================<br>
--- cfe/trunk/test/Modules/Inputs/PR27754/RConversionRuleParser.h (added)<br>
+++ cfe/trunk/test/Modules/Inputs/PR27754/RConversionRuleParser.h Tue May 17 17:44:15 2016<br>
@@ -0,0 +1,4 @@<br>
+#include "algobase.h"<br>
+typedef integral_constant<bool, true> true_type;<br>
+class _Rb_tree { _Rb_tree() { true_type(); } };<br>
+#include "TSchemaType.h"<br>
<br>
Added: cfe/trunk/test/Modules/Inputs/PR27754/TMetaUtils.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR27754/TMetaUtils.h?rev=269858&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR27754/TMetaUtils.h?rev=269858&view=auto</a><br>
==============================================================================<br>
--- cfe/trunk/test/Modules/Inputs/PR27754/TMetaUtils.h (added)<br>
+++ cfe/trunk/test/Modules/Inputs/PR27754/TMetaUtils.h Tue May 17 17:44:15 2016<br>
@@ -0,0 +1,2 @@<br>
+#include "RConversionRuleParser.h"<br>
+void fn1() { true_type(); }<br>
<br>
Added: cfe/trunk/test/Modules/Inputs/PR27754/TSchemaType.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR27754/TSchemaType.h?rev=269858&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR27754/TSchemaType.h?rev=269858&view=auto</a><br>
==============================================================================<br>
--- cfe/trunk/test/Modules/Inputs/PR27754/TSchemaType.h (added)<br>
+++ cfe/trunk/test/Modules/Inputs/PR27754/TSchemaType.h Tue May 17 17:44:15 2016<br>
@@ -0,0 +1,2 @@<br>
+#include "algobase.h"<br>
+struct A : integral_constant<bool, true> {};<br>
<br>
Added: cfe/trunk/test/Modules/Inputs/PR27754/algobase.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR27754/algobase.h?rev=269858&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR27754/algobase.h?rev=269858&view=auto</a><br>
==============================================================================<br>
--- cfe/trunk/test/Modules/Inputs/PR27754/algobase.h (added)<br>
+++ cfe/trunk/test/Modules/Inputs/PR27754/algobase.h Tue May 17 17:44:15 2016<br>
@@ -0,0 +1,4 @@<br>
+#ifndef _STL_ALGOBASE_H<br>
+#define _STL_ALGOBASE_H<br>
+template<typename _Tp, _Tp> struct integral_constant {};<br>
+#endif<br>
<br>
Added: cfe/trunk/test/Modules/Inputs/PR27754/module.modulemap<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR27754/module.modulemap?rev=269858&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR27754/module.modulemap?rev=269858&view=auto</a><br>
==============================================================================<br>
--- cfe/trunk/test/Modules/Inputs/PR27754/module.modulemap (added)<br>
+++ cfe/trunk/test/Modules/Inputs/PR27754/module.modulemap Tue May 17 17:44:15 2016<br>
@@ -0,0 +1,3 @@<br>
+module "RConversionRuleParser.h" { header "RConversionRuleParser.h" }<br>
+module "TMetaUtils.h" { header "TMetaUtils.h" }<br>
+module "TSchemaType.h" { header "TSchemaType.h" }<br>
<br>
Added: cfe/trunk/test/Modules/pr27754.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/pr27754.cpp?rev=269858&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/pr27754.cpp?rev=269858&view=auto</a><br>
==============================================================================<br>
--- cfe/trunk/test/Modules/pr27754.cpp (added)<br>
+++ cfe/trunk/test/Modules/pr27754.cpp Tue May 17 17:44:15 2016<br>
@@ -0,0 +1,7 @@<br>
+// RUN: rm -rf %t<br>
+// RUN: %clang_cc1 -std=c++11 -I%S/Inputs/PR27754 -verify %s<br>
+// 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<br>
+<br>
+#include "TMetaUtils.h"<br>
+<br>
+// expected-no-diagnostics<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div>