[cfe-commits] r130601 - in /cfe/trunk: include/clang/AST/DeclCXX.h include/clang/AST/Type.h lib/AST/DeclCXX.cpp lib/AST/Type.cpp lib/Sema/SemaExprCXX.cpp lib/Serialization/ASTReaderDecl.cpp lib/Serialization/ASTWriter.cpp
Chandler Carruth
chandlerc at gmail.com
Sat Apr 30 02:17:45 PDT 2011
Author: chandlerc
Date: Sat Apr 30 04:17:45 2011
New Revision: 130601
URL: http://llvm.org/viewvc/llvm-project?rev=130601&view=rev
Log:
Completely re-implement the core logic behind the __is_standard_layout
type trait. The previous implementation suffered from several problems:
1) It implemented all of the logic in RecordType by walking over every
base and field in a CXXRecordDecl and validating the constraints of
the standard. This made for very straightforward code, but is
extremely inefficient. It also is conceptually wrong, the logic tied
to the C++ definition of standard-layout classes should be in
CXXRecordDecl, not RecordType.
2) To address the performance problems with #1, a cache bit was added to
CXXRecordDecl, and at the completion of every C++ class, the
RecordType was queried to determine if it was a standard layout
class, and that state was cached. Two things went very very wrong
with this. First, the caching version of the query *was never
called*. Even within the recursive steps of the walk over all fields
and bases the caching variant was not called, making each query
a full *recursive* walk. Second, despite the cache not being used, it
was computed for every class declared, even when the trait was never
used in the program. This probably significantly regressed compile
time performance for edge-case files.
3) An ASTContext was required merely to query the type trait because
querying it performed the actual computations.
4) The caching bit wasn't managed correctly (uninitialized).
The new implementation follows the system for all the other traits on
C++ classes by encoding all the state needed in the definition data and
building up the trait incrementally as each base and member are added to
the definition of the class.
The idiosyncracies of the specification of standard-layout classes
requires more state than I would like; currently 5 bits. I could
eliminate one of the bits easily at the expense of both clarity and
resilience of the code. I might be able to eliminate one of the other
bits by computing its state in terms of other state bits in the
definition. I've already done that in one place where there was a fairly
simple way to achieve it.
It's possible some of the bits could be moved out of the definition data
and into some other structure which isn't serialized if the serialized
bloat is a problem. That would preclude serialization of a partial class
declaration, but that's likely already precluded.
Comments on any of these issues welcome.
Modified:
cfe/trunk/include/clang/AST/DeclCXX.h
cfe/trunk/include/clang/AST/Type.h
cfe/trunk/lib/AST/DeclCXX.cpp
cfe/trunk/lib/AST/Type.cpp
cfe/trunk/lib/Sema/SemaExprCXX.cpp
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
cfe/trunk/lib/Serialization/ASTWriter.cpp
Modified: cfe/trunk/include/clang/AST/DeclCXX.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=130601&r1=130600&r2=130601&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/DeclCXX.h (original)
+++ cfe/trunk/include/clang/AST/DeclCXX.h Sat Apr 30 04:17:45 2011
@@ -321,6 +321,22 @@
/// member.
bool HasStandardLayout : 1;
+ /// HasNoNonEmptyBases - True when there are no non-empty base classes.
+ ///
+ /// This is a helper bit of state used to implement HasStandardLayout more
+ /// efficiently.
+ bool HasNoNonEmptyBases : 1;
+
+ /// HasPrivateFields - True when there are private non-static data members.
+ bool HasPrivateFields : 1;
+
+ /// HasProtectedFields - True when there are protected non-static data
+ /// members.
+ bool HasProtectedFields : 1;
+
+ /// HasPublicFields - True when there are private non-static data members.
+ bool HasPublicFields : 1;
+
/// HasTrivialConstructor - True when this class has a trivial constructor.
///
/// C++ [class.ctor]p5. A constructor is trivial if it is an
@@ -780,8 +796,8 @@
/// which means that the class contains or inherits a pure virtual function.
bool isAbstract() const { return data().Abstract; }
- // hasStandardLayout - Whether this class has standard layout
- // (C++ [class]p7)
+ /// hasStandardLayout - Whether this class has standard layout
+ /// (C++ [class]p7)
bool hasStandardLayout() const { return data().HasStandardLayout; }
// hasTrivialConstructor - Whether this class has a trivial constructor
Modified: cfe/trunk/include/clang/AST/Type.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Type.h?rev=130601&r1=130600&r2=130601&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/Type.h (original)
+++ cfe/trunk/include/clang/AST/Type.h Sat Apr 30 04:17:45 2011
@@ -2843,7 +2843,7 @@
bool hasConstFields() const { return false; }
/// \brief Whether this class has standard layout
- bool hasStandardLayout(ASTContext& Ctx) const;
+ bool hasStandardLayout() const;
bool isSugared() const { return false; }
QualType desugar() const { return QualType(this, 0); }
Modified: cfe/trunk/lib/AST/DeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclCXX.cpp?rev=130601&r1=130600&r2=130601&view=diff
==============================================================================
--- cfe/trunk/lib/AST/DeclCXX.cpp (original)
+++ cfe/trunk/lib/AST/DeclCXX.cpp Sat Apr 30 04:17:45 2011
@@ -31,8 +31,9 @@
: UserDeclaredConstructor(false), UserDeclaredCopyConstructor(false),
UserDeclaredCopyAssignment(false), UserDeclaredDestructor(false),
Aggregate(true), PlainOldData(true), Empty(true), Polymorphic(false),
- Abstract(false), HasStandardLayout(true), HasTrivialConstructor(true),
- HasConstExprNonCopyMoveConstructor(false),
+ Abstract(false), HasStandardLayout(true), HasNoNonEmptyBases(true),
+ HasPrivateFields(false), HasProtectedFields(false), HasPublicFields(false),
+ HasTrivialConstructor(true), HasConstExprNonCopyMoveConstructor(false),
HasTrivialCopyConstructor(true), HasTrivialMoveConstructor(true),
HasTrivialCopyAssignment(true), HasTrivialMoveAssignment(true),
HasTrivialDestructor(true), HasNonLiteralTypeFieldsOrBases(false),
@@ -111,8 +112,22 @@
// A class with a non-empty base class is not empty.
// FIXME: Standard ref?
- if (!BaseClassDecl->isEmpty())
+ if (!BaseClassDecl->isEmpty()) {
+ if (!data().Empty) {
+ // C++0x [class]p7:
+ // A standard-layout class is a class that:
+ // [...]
+ // -- either has no non-static data members in the most derived
+ // class and at most one base class with non-static data members,
+ // or has no base classes with non-static data members, and
+ // If this is the second non-empty base, then neither of these two
+ // clauses can be true.
+ data().HasStandardLayout = false;
+ }
+
data().Empty = false;
+ data().HasNoNonEmptyBases = false;
+ }
// C++ [class.virtual]p1:
// A class that declares or inherits a virtual function is called a
@@ -120,6 +135,12 @@
if (BaseClassDecl->isPolymorphic())
data().Polymorphic = true;
+ // C++0x [class]p7:
+ // A standard-layout class is a class that: [...]
+ // -- has no non-standard-layout base classes
+ if (!BaseClassDecl->hasStandardLayout())
+ data().HasStandardLayout = false;
+
// Record if this base is the first non-literal field or base.
if (!hasNonLiteralTypeFieldsOrBases() && !BaseType->isLiteralType())
data().HasNonLiteralTypeFieldsOrBases = true;
@@ -160,6 +181,11 @@
// -- class X has no virtual functions and no virtual base classes, and
data().HasTrivialCopyAssignment = false;
data().HasTrivialMoveAssignment = false;
+
+ // C++0x [class]p7:
+ // A standard-layout class is a class that: [...]
+ // -- has [...] no virtual base classes
+ data().HasStandardLayout = false;
} else {
// C++ [class.ctor]p5:
// A constructor is trivial if all the direct base classes of its
@@ -410,6 +436,11 @@
data().HasTrivialCopyAssignment = false;
data().HasTrivialMoveAssignment = false;
// FIXME: Destructor?
+
+ // C++0x [class]p7:
+ // A standard-layout class is a class that: [...]
+ // -- has no virtual functions
+ data().HasStandardLayout = false;
}
}
@@ -621,7 +652,21 @@
data().Aggregate = false;
data().PlainOldData = false;
}
-
+
+ // C++0x [class]p7:
+ // A standard-layout class is a class that:
+ // [...]
+ // -- has the same access control for all non-static data members,
+ switch (D->getAccess()) {
+ case AS_private: data().HasPrivateFields = true; break;
+ case AS_protected: data().HasProtectedFields = true; break;
+ case AS_public: data().HasPublicFields = true; break;
+ case AS_none: assert(0 && "Invalid access specifier");
+ };
+ if ((data().HasPrivateFields + data().HasProtectedFields +
+ data().HasPublicFields) > 1)
+ data().HasStandardLayout = false;
+
// C++0x [class]p9:
// A POD struct is a class that is both a trivial class and a
// standard-layout class, and has no non-static data members of type
@@ -630,9 +675,15 @@
QualType T = Context.getBaseElementType(Field->getType());
if (!T->isPODType())
data().PlainOldData = false;
- if (T->isReferenceType())
+ if (T->isReferenceType()) {
data().HasTrivialConstructor = false;
+ // C++0x [class]p7:
+ // A standard-layout class is a class that:
+ // -- has no non-static data members of type [...] reference,
+ data().HasStandardLayout = false;
+ }
+
// Record if this field is the first non-literal field or base.
if (!hasNonLiteralTypeFieldsOrBases() && !T->isLiteralType())
data().HasNonLiteralTypeFieldsOrBases = true;
@@ -669,9 +720,53 @@
if (!FieldRec->hasTrivialDestructor())
data().HasTrivialDestructor = false;
+
+ // C++0x [class]p7:
+ // A standard-layout class is a class that:
+ // -- has no non-static data members of type non-standard-layout
+ // class (or array of such types) [...]
+ if (!FieldRec->hasStandardLayout())
+ data().HasStandardLayout = false;
+
+ // C++0x [class]p7:
+ // A standard-layout class is a class that:
+ // [...]
+ // -- has no base classes of the same type as the first non-static
+ // data member.
+ // We don't want to expend bits in the state of the record decl
+ // tracking whether this is the first non-static data member so we
+ // cheat a bit and use some of the existing state: the empty bit.
+ // Virtual bases and virtual methods make a class non-empty, but they
+ // also make it non-standard-layout so we needn't check here.
+ // A non-empty base class may leave the class standard-layout, but not
+ // if we have arrived here, and have at least on non-static data
+ // member. If HasStandardLayout remains true, then the first non-static
+ // data member must come through here with Empty still true, and Empty
+ // will subsequently be set to false below.
+ if (data().HasStandardLayout && data().Empty) {
+ for (CXXRecordDecl::base_class_const_iterator BI = bases_begin(),
+ BE = bases_end();
+ BI != BE; ++BI) {
+ if (Context.hasSameUnqualifiedType(BI->getType(), T)) {
+ data().HasStandardLayout = false;
+ break;
+ }
+ }
+ }
}
}
-
+
+ // C++0x [class]p7:
+ // A standard-layout class is a class that:
+ // [...]
+ // -- either has no non-static data members in the most derived
+ // class and at most one base class with non-static data members,
+ // or has no base classes with non-static data members, and
+ // At this point we know that we have a non-static data member, so the last
+ // clause holds.
+ if (!data().HasNoNonEmptyBases)
+ data().HasStandardLayout = false;
+
// If this is not a zero-length bit-field, then the class is not empty.
if (data().Empty) {
if (!Field->getBitWidth())
@@ -926,10 +1021,6 @@
void CXXRecordDecl::completeDefinition() {
completeDefinition(0);
-
- ASTContext &Context = getASTContext();
- if (const RecordType *RT = getTypeForDecl()->getAs<RecordType>())
- data().HasStandardLayout = RT->hasStandardLayout(Context);
}
void CXXRecordDecl::completeDefinition(CXXFinalOverriderMap *FinalOverriders) {
Modified: cfe/trunk/lib/AST/Type.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Type.cpp?rev=130601&r1=130600&r2=130601&view=diff
==============================================================================
--- cfe/trunk/lib/AST/Type.cpp (original)
+++ cfe/trunk/lib/AST/Type.cpp Sat Apr 30 04:17:45 2011
@@ -1448,7 +1448,7 @@
return BasesWithFields;
}
-bool RecordType::hasStandardLayout(ASTContext& Context) const {
+bool RecordType::hasStandardLayout() const {
CXXRecordDecl *RD = cast<CXXRecordDecl>(getDecl());
if (! RD) {
assert(cast<RecordDecl>(getDecl()) &&
@@ -1456,85 +1456,7 @@
return true;
}
- // A standard-layout class is a class that:
-
- for (CXXRecordDecl::method_iterator M = RD->method_begin(),
- ME = RD->method_end(); M != ME; ++M) {
- CXXMethodDecl *Method = *M;
-
- // C++0x [class]p7:
- // A standard-layout class is a class that [...]
- // -- has no virtual functions (10.3) [...]
- if (Method->isVirtual())
- return false;
- }
-
- AccessSpecifier AS = AS_none;
- QualType FirstFieldType;
- bool FirstFieldType_set = false;
- uint64_t FieldCount = 0;
-
- for (CXXRecordDecl::field_iterator Field = RD->field_begin(),
- E = RD->field_end(); Field != E; ++Field, ++FieldCount) {
- // C++0x [class]p7:
- // A standard-layout class is a class that [...]
- // -- has no non-static data members of type non-standard-layout class
- // (or array of such types) or reference [...]
- QualType FieldType = Context.getBaseElementType((*Field)->getType());
- if (const RecordType *T =
- Context.getBaseElementType(FieldType)->getAs<RecordType>()) {
- if (! T->hasStandardLayout(Context) || T->isReferenceType())
- return false;
- }
- if (! FirstFieldType_set) {
- FirstFieldType = FieldType;
- FirstFieldType_set = true;
- }
-
- // C++0x [class]p7:
- // A standard-layout class is a class that [...]
- // -- has the same access control (Clause 11) for all non-static data
- // members [...]
- if (AS == AS_none)
- AS = (*Field)->getAccess();
- else if (AS != (*Field)->getAccess())
- return false;
- }
-
- for (CXXRecordDecl::base_class_const_iterator B = RD->bases_begin(),
- BE = RD->bases_end(); B != BE; ++B) {
- // C++0x [class]p7:
- // A standard-layout class is a class that [...]
- // -- no virtual base classes (10.1) [...]
- if (B->isVirtual())
- return false;
-
- QualType BT = B->getType();
-
- // C++0x [class]p7:
- // A standard-layout class is a class that [...]
- // -- has no non-standard-layout base classes [...]
- if (const RecordType *T = BT->getAs<RecordType>())
- if (! T->hasStandardLayout(Context))
- return false;
-
- // C++0x [class]p7:
- // A standard-layout class is a class that [...]
- // -- has no base classes of the same type as the first non-static data
- // member.
- if (BT == FirstFieldType)
- return false;
-
- // C++0x [class]p7:
- // A standard-layout class is a class that [...]
- // -- either has no non-static data members in the most derived class
- // and at most one base class with non-static data members, or has
- // no base classes with non-static data members [...]
- if (countBasesWithFields(BT) > (FieldCount == 0 ? 1 : 0))
- return false;
- }
-
- return true;
+ return RD->hasStandardLayout();
}
bool EnumType::classof(const TagType *TT) {
Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=130601&r1=130600&r2=130601&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Sat Apr 30 04:17:45 2011
@@ -2452,7 +2452,7 @@
KeyLoc))
return true;
if (const RecordType *RT = C.getBaseElementType(T)->getAs<RecordType>())
- return RT->hasStandardLayout(C);
+ return RT->hasStandardLayout();
return false;
case UTT_IsUnsigned:
return T->isUnsignedIntegerType();
Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=130601&r1=130600&r2=130601&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Sat Apr 30 04:17:45 2011
@@ -843,6 +843,10 @@
Data.Polymorphic = Record[Idx++];
Data.Abstract = Record[Idx++];
Data.HasStandardLayout = Record[Idx++];
+ Data.HasNoNonEmptyBases = Record[Idx++];
+ Data.HasPrivateFields = Record[Idx++];
+ Data.HasProtectedFields = Record[Idx++];
+ Data.HasPublicFields = Record[Idx++];
Data.HasTrivialConstructor = Record[Idx++];
Data.HasConstExprNonCopyMoveConstructor = Record[Idx++];
Data.HasTrivialCopyConstructor = Record[Idx++];
Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=130601&r1=130600&r2=130601&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTWriter.cpp Sat Apr 30 04:17:45 2011
@@ -3783,6 +3783,10 @@
Record.push_back(Data.Polymorphic);
Record.push_back(Data.Abstract);
Record.push_back(Data.HasStandardLayout);
+ Record.push_back(Data.HasNoNonEmptyBases);
+ Record.push_back(Data.HasPrivateFields);
+ Record.push_back(Data.HasProtectedFields);
+ Record.push_back(Data.HasPublicFields);
Record.push_back(Data.HasTrivialConstructor);
Record.push_back(Data.HasConstExprNonCopyMoveConstructor);
Record.push_back(Data.HasTrivialCopyConstructor);
More information about the cfe-commits
mailing list