rsmith added inline comments.
================
Comment at: include/clang/AST/DeclCXX.h:405-416
@@ -404,1 +404,14 @@
+ enum AllowConstDefInitKind {
+ ACDI_Unknown,
+ ACDI_Yes,
+ ACDI_No,
+ };
+ unsigned AllowConstDefaultInit : 3;
+ AllowConstDefInitKind allowConstDefInitKind() {
+ return static_cast<AllowConstDefInitKind>(AllowConstDefaultInit);
+ }
+ void setAllowConstDefInitKind(AllowConstDefInitKind Allow) {
+ AllowConstDefaultInit = Allow;
+ }
+
----------------
Is it worth caching this? (If so, the bit-field should occupy at most 2 bits...)
================
Comment at: include/clang/AST/DeclCXX.h:1286
@@ -1272,1 +1285,3 @@
+ /// \brief Determine whether declaring a const with this type is ok per
+ /// core issue 253.
----------------
Declaring a const what?
================
Comment at: lib/AST/DeclCXX.cpp:396-425
@@ -394,1 +395,32 @@
+bool CXXRecordDecl::allowConstDefaultInitSlow() const {
+ assert(getDefinition() && "only call this on completed records");
+ if (hasUserProvidedDefaultConstructor()) {
+ data().setAllowConstDefInitKind(DefinitionData::ACDI_Yes);
+ return true;
+ }
+ for (const auto *F : fields()) {
+ if (F->hasInClassInitializer() || F->isUnnamedBitfield())
+ continue;
+ if (CXXRecordDecl *FieldType = F->getType()->getAsCXXRecordDecl()) {
+ if (!FieldType->allowConstDefaultInit()) {
+ data().setAllowConstDefInitKind(DefinitionData::ACDI_No);
+ return false;
+ }
+ } else {
+ data().setAllowConstDefInitKind(DefinitionData::ACDI_No);
+ return false;
+ }
+ }
+ for (const auto& BI : bases()) {
+ const RecordType *RT = BI.getType()->getAs<RecordType>();
+ CXXRecordDecl *Base = cast<CXXRecordDecl>(RT->getDecl());
+ if (!Base->allowConstDefaultInit()) {
+ data().setAllowConstDefInitKind(DefinitionData::ACDI_No);
+ return false;
+ }
+ }
+ data().setAllowConstDefInitKind(DefinitionData::ACDI_Yes);
+ return true;
+}
+
----------------
For all the other fields of this kind, we compute the flag as bases and members
are added to the class. It seems like we can do the same here, and all else
being equal, it would be better to not deviate from the existing pattern
without good reason.
The most natural way to do this would probably be to store a single flag for
`HasUninitializedFields`, and then compute whether default initialization would
leave a field uninitialized as `!hasUserProvidedDefaultConstructor() &&
HasUninitializedFields`.
================
Comment at: lib/AST/DeclCXX.cpp:403-413
@@ +402,13 @@
+ for (const auto *F : fields()) {
+ if (F->hasInClassInitializer() || F->isUnnamedBitfield())
+ continue;
+ if (CXXRecordDecl *FieldType = F->getType()->getAsCXXRecordDecl()) {
+ if (!FieldType->allowConstDefaultInit()) {
+ data().setAllowConstDefInitKind(DefinitionData::ACDI_No);
+ return false;
+ }
+ } else {
+ data().setAllowConstDefInitKind(DefinitionData::ACDI_No);
+ return false;
+ }
+ }
----------------
I think you should skip `mutable` members here, as the corresponding subobject
of a `const` object is not itself `const`.
http://reviews.llvm.org/D16552
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
