This patch demonstrates the fixes needed to support deserializing functions
with decltype return types correctly.
The problem is that depending on the order other types are deserialized a
decltype might end up needing to recursively deserialize itself. Currently this
causes a crash when the type is profiled.
An assert is added to make sure that the same type is not returned with
different type pointers, as clang relies on the pointer value for equality
comparisons.
What would be the proper way to fix this? The current implementation uses ugly
placement new and relies on the types to eventually become complete.
http://llvm-reviews.chandlerc.com/D1359
Files:
include/clang/AST/ASTContext.h
include/clang/AST/Type.h
lib/AST/ASTContext.cpp
lib/AST/Type.cpp
lib/Serialization/ASTReader.cpp
test/PCH/cxx11-decltype.cpp
Index: include/clang/AST/ASTContext.h
===================================================================
--- include/clang/AST/ASTContext.h
+++ include/clang/AST/ASTContext.h
@@ -824,6 +824,15 @@
QualType getTypeDeclTypeSlow(const TypeDecl *Decl) const;
public:
+ /// \brief getPlacementType - Return a type of the right size for placement
+ /// new
+ template <typename PlacementType>
+ PlacementType *getPlacementType(QualType Canonical = QualType()) const {
+ void *Memory = Allocate(sizeof(PlacementType), TypeAlignment);
+ memset(Memory, 0, sizeof(PlacementType));
+ return (PlacementType *) new (Memory) PlacementDummyType(Canonical);
+ }
+
/// \brief Return the uniqued reference to the type for an address space
/// qualified type with the specified type and address space.
///
@@ -1058,7 +1067,8 @@
QualType getCanonicalTemplateSpecializationType(TemplateName T,
const TemplateArgument *Args,
- unsigned NumArgs) const;
+ unsigned NumArgs,
+ bool Dependent) const;
QualType getTemplateSpecializationType(TemplateName T,
const TemplateArgumentListInfo &Args,
@@ -1107,7 +1117,8 @@
QualType getTypeOfType(QualType t) const;
/// \brief C++11 decltype.
- QualType getDecltypeType(Expr *e, QualType UnderlyingType) const;
+ QualType getDecltypeType(Expr *e, QualType UnderlyingType,
+ void *Placement = 0) const;
/// \brief Unary type transforms
QualType getUnaryTransformType(QualType BaseType, QualType UnderlyingType,
Index: include/clang/AST/Type.h
===================================================================
--- include/clang/AST/Type.h
+++ include/clang/AST/Type.h
@@ -1022,6 +1022,7 @@
} // end namespace llvm
namespace clang {
+ class PlacementDummyType;
/// \brief Base class that is common to both the \c ExtQuals and \c Type
/// classes, which allows \c QualType to access the common fields between the
@@ -1044,7 +1045,17 @@
friend class QualType;
friend class Type;
friend class ExtQuals;
+ friend class PlacementDummyType;
};
+
+class PlacementDummyType : public ExtQualsTypeCommonBase {
+ Type *this_() { return (Type *)this; }
+public:
+ PlacementDummyType(QualType canon)
+ : ExtQualsTypeCommonBase(this_(),
+ canon.isNull() ? QualType(this_(), 0) : canon) {
+ }
+};
/// ExtQuals - We can encode up to four bits in the low bits of a
/// type pointer, but there are many more type qualifiers that we want
@@ -3698,7 +3709,7 @@
TemplateSpecializationType(TemplateName T,
const TemplateArgument *Args,
unsigned NumArgs, QualType Canon,
- QualType Aliased);
+ QualType Aliased, bool Dependent = false);
friend class ASTContext; // ASTContext creates these
Index: lib/AST/ASTContext.cpp
===================================================================
--- lib/AST/ASTContext.cpp
+++ lib/AST/ASTContext.cpp
@@ -3165,7 +3165,7 @@
"Caller must compute aliased type");
IsTypeAlias = false;
CanonType = getCanonicalTemplateSpecializationType(Template, Args,
- NumArgs);
+ NumArgs, false);
}
// Allocate the (non-canonical) template specialization type, but don't
@@ -3186,7 +3186,8 @@
QualType
ASTContext::getCanonicalTemplateSpecializationType(TemplateName Template,
const TemplateArgument *Args,
- unsigned NumArgs) const {
+ unsigned NumArgs,
+ bool Dependent) const {
assert(!Template.getAsDependentTemplateName() &&
"No dependent template names here!");
@@ -3218,12 +3219,13 @@
TypeAlignment);
Spec = new (Mem) TemplateSpecializationType(CanonTemplate,
CanonArgs.data(), NumArgs,
- QualType(), QualType());
+ QualType(), QualType(),
+ Dependent);
Types.push_back(Spec);
TemplateSpecializationTypes.InsertNode(Spec, InsertPos);
}
- assert(Spec->isDependentType() &&
+ assert((Dependent || Spec->isDependentType()) &&
"Non-dependent template-id type must have a canonical type");
return QualType(Spec, 0);
}
@@ -3602,7 +3604,8 @@
/// memory savings. Since decltype(t) is fairly uncommon, space shouldn't be
/// an issue. This doesn't effect the type checker, since it operates
/// on canonical types (which are always unique).
-QualType ASTContext::getDecltypeType(Expr *e, QualType UnderlyingType) const {
+QualType ASTContext::getDecltypeType(Expr *e, QualType UnderlyingType,
+ void *Placement) const {
DecltypeType *dt;
// C++0x [temp.type]p2:
@@ -3619,17 +3622,28 @@
if (Canon) {
// We already have a "canonical" version of an equivalent, dependent
// decltype type. Use that as our canonical type.
- dt = new (*this, TypeAlignment) DecltypeType(e, UnderlyingType,
- QualType((DecltypeType*)Canon, 0));
+ if (Placement)
+ dt = new (Placement) DecltypeType(e, UnderlyingType,
+ QualType((DecltypeType*)Canon, 0));
+ else
+ dt = new (*this, TypeAlignment) DecltypeType(e, UnderlyingType,
+ QualType((DecltypeType*)Canon, 0));
} else {
// Build a new, canonical typeof(expr) type.
- Canon = new (*this, TypeAlignment) DependentDecltypeType(*this, e);
+ if (Placement)
+ Canon = new (Placement) DependentDecltypeType(*this, e);
+ else
+ Canon = new (*this, TypeAlignment) DependentDecltypeType(*this, e);
DependentDecltypeTypes.InsertNode(Canon, InsertPos);
dt = Canon;
}
} else {
- dt = new (*this, TypeAlignment) DecltypeType(e, UnderlyingType,
- getCanonicalType(UnderlyingType));
+ if (Placement)
+ dt = new (Placement) DecltypeType(e, UnderlyingType,
+ getCanonicalType(UnderlyingType));
+ else
+ dt = new (*this, TypeAlignment) DecltypeType(e, UnderlyingType,
+ getCanonicalType(UnderlyingType));
}
Types.push_back(dt);
return QualType(dt, 0);
Index: lib/AST/Type.cpp
===================================================================
--- lib/AST/Type.cpp
+++ lib/AST/Type.cpp
@@ -1962,7 +1962,7 @@
TemplateSpecializationType::
TemplateSpecializationType(TemplateName T,
const TemplateArgument *Args, unsigned NumArgs,
- QualType Canon, QualType AliasedType)
+ QualType Canon, QualType AliasedType, bool Dependent)
: Type(TemplateSpecialization,
Canon.isNull()? QualType(this, 0) : Canon,
Canon.isNull()? T.isDependent() : Canon->isDependentType(),
@@ -1979,7 +1979,8 @@
"Unexpected template name for TemplateSpecializationType");
bool InstantiationDependent;
(void)InstantiationDependent;
- assert((!Canon.isNull() ||
+ assert((Dependent ||
+ !Canon.isNull() ||
T.isDependent() ||
::anyDependentTemplateArguments(Args, NumArgs,
InstantiationDependent)) &&
Index: lib/Serialization/ASTReader.cpp
===================================================================
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -4725,8 +4725,21 @@
}
case TYPE_DECLTYPE: {
+ DependentDecltypeType *pTemp =
+ Context.getPlacementType<DependentDecltypeType>();
+ // Prevent recursive reads from loading this type again to stop
+ // the profile of the recursive decltype from accessing data that
+ // hasn't finished loading.
+ TypesLoaded[Index] = QualType(pTemp, 0);
+
QualType UnderlyingType = readType(*Loc.F, Record, Idx);
- return Context.getDecltypeType(ReadExpr(*Loc.F), UnderlyingType);
+ QualType T = Context.getDecltypeType(ReadExpr(*Loc.F), UnderlyingType,
+ pTemp);
+ if (T.getTypePtrOrNull() != pTemp)
+ Context.Deallocate(pTemp);
+ // Reset TypeLoaded so right logic is applied in GetType.
+ TypesLoaded[Index] = QualType();
+ return T;
}
case TYPE_UNARY_TRANSFORM: {
@@ -4856,10 +4869,20 @@
case TYPE_INJECTED_CLASS_NAME: {
CXXRecordDecl *D = ReadDeclAs<CXXRecordDecl>(*Loc.F, Record, Idx);
QualType TST = readType(*Loc.F, Record, Idx); // probably derivable
+
+ if (!TypesLoaded[Index].isNull()) {
+ // Type has been loaded recursively, so lets return it as is.
+ // Otherwise there will be two pointers to the same type referenced
+ // in different places.
+ return TypesLoaded[Index];
+ }
+
// FIXME: ASTContext::getInjectedClassNameType is not currently suitable
// for AST reading, too much interdependencies.
- return
- QualType(new (Context, TypeAlignment) InjectedClassNameType(D, TST), 0);
+ QualType T = QualType(new (Context, TypeAlignment)
+ InjectedClassNameType(D, TST), 0);
+ TypesLoaded[Index] = T;
+ return T;
}
case TYPE_TEMPLATE_TYPE_PARM: {
@@ -4922,13 +4945,22 @@
ReadTemplateArgumentList(Args, *Loc.F, Record, Idx);
QualType Underlying = readType(*Loc.F, Record, Idx);
QualType T;
+ if (!TypesLoaded[Index].isNull()) {
+ // Type has been loaded recursively, so lets return it as is.
+ // Otherwise there will be two pointers to the same type referenced
+ // in different places.
+ return TypesLoaded[Index];
+ }
if (Underlying.isNull())
T = Context.getCanonicalTemplateSpecializationType(Name, Args.data(),
- Args.size());
+ Args.size(),
+ IsDependent);
else
T = Context.getTemplateSpecializationType(Name, Args.data(),
Args.size(), Underlying);
const_cast<Type*>(T.getTypePtr())->setDependent(IsDependent);
+ // Save type here so same type will not be loaded recursively
+ TypesLoaded[Index] = T;
return T;
}
@@ -5277,7 +5309,15 @@
Index -= NUM_PREDEF_TYPE_IDS;
assert(Index < TypesLoaded.size() && "Type index out-of-range");
if (TypesLoaded[Index].isNull()) {
- TypesLoaded[Index] = readTypeRecord(Index);
+ QualType ReadType = readTypeRecord(Index);
+ if (TypesLoaded[Index].isNull())
+ TypesLoaded[Index] = ReadType;
+ else {
+ assert(ReadType == TypesLoaded[Index]
+ && "Different type pointers returned for recursive load");
+ // Type was loaded recursively, lets just return.
+ return TypesLoaded[Index].withFastQualifiers(FastQuals);
+ }
if (TypesLoaded[Index].isNull())
return QualType();
Index: test/PCH/cxx11-decltype.cpp
===================================================================
--- /dev/null
+++ test/PCH/cxx11-decltype.cpp
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -pedantic -std=c++1y -emit-pch %s -o %t
+// RUN: %clang_cc1 -pedantic -std=c++1y -include-pch %t -verify %s
+
+#ifndef HEADER_INCLUDED
+
+#define HEADER_INCLUDED
+
+template <typename T>
+struct TemplateType {
+};
+
+template <typename T>
+auto Func0(T &&Param) -> TemplateType<decltype(Param)>;
+
+template <typename T>
+auto Func1(T &&Param) -> TemplateType<decltype(Param)>;
+
+#else
+
+struct TestStruct {
+};
+
+void Test() {
+
+ TestStruct Test;
+ Func1(Test);
+ Func0(Test);
+}
+// expected-no-diagnostics
+
+#endif
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
