thakis added a comment.

thanks!


================
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;
+    }
+
----------------
rsmith wrote:
> Is it worth caching this? (If so, the bit-field should occupy at most 2 
> bits...)
The motivation is that a chain of

struct A0 {};
struct A1 {  const A0 a; }
struct A2 {  const A1 a; }
struct A3 {  const A2 a; }

needs O(n^2) without caching. Changed the bitfield to 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.
----------------
rsmith wrote:
> Declaring a const what?
Done (I think?)

================
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;
+}
+
----------------
rsmith wrote:
> 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`.
My reasoning here was that for the vast majority of types, we'll never have a 
const variable of that type, so computing this for all types seemed like a 
waste.


http://reviews.llvm.org/D16552



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to