[PATCH] D38483: [ExprConstant] Allow constexpr ctor to modify non static data members in body

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington created this revision.

Previously, clang rejected the following (copied from PR19741):

  struct A {
    int a = 0;
    constexpr A() { a = 1; }
  };
  constexpr bool f() {
    constexpr A a;
    static_assert(a.a == 1, "");
    return a.a == 1;
  }
  static_assert(f(), "");

Clang didn't like the assignment to `a` in `A()` because it doesn't know that A is being constructed and therefore considers the subobject A.a to be const. The fix in this patch (which @rsmith suggested in the PR) is just to keep track of the set of objects that are currently being constructed, and strip away the const whenever we encounter a subobject of an object under construction.

https://bugs.llvm.org/show_bug.cgi?id=19741

Thanks for taking a look!
Erik


https://reviews.llvm.org/D38483

Files:
  lib/AST/ExprConstant.cpp
  test/SemaCXX/constant-expression-cxx1y.cpp


Index: test/SemaCXX/constant-expression-cxx1y.cpp
===================================================================
--- test/SemaCXX/constant-expression-cxx1y.cpp
+++ test/SemaCXX/constant-expression-cxx1y.cpp
@@ -988,3 +988,36 @@
   void();
 }
 constexpr int void_test = (Void(0), 1);
+
+namespace PR19741 {
+constexpr void addone(int &m) { m++; }
+
+struct S {
+  int m = 0;
+  constexpr S() { addone(m); }
+};
+constexpr bool evalS() {
+  constexpr S s;
+  return s.m == 1;
+}
+static_assert(evalS(), "");
+
+struct Nested {
+  struct First { int x = 42; };
+  union {
+    First first;
+    int second;
+  };
+  int x;
+  constexpr Nested(int x) : first(), x(x) { x = 4; }
+  constexpr Nested() : Nested(42) {
+    addone(first.x);
+    x = 3;
+  }
+};
+constexpr bool evalNested() {
+  constexpr Nested N;
+  return N.first.x == 43;
+}
+static_assert(evalNested(), "");
+} // namespace PR19741
Index: lib/AST/ExprConstant.cpp
===================================================================
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -573,6 +573,30 @@
     /// declaration whose initializer is being evaluated, if any.
     APValue *EvaluatingDeclValue;
 
+    typedef std::pair<APValue::LValueBase, unsigned> EvaluatingObject;
+
+    /// EvaluatingConstructors - Set of objects that are currently being
+    /// constructed.
+    llvm::DenseSet<EvaluatingObject> EvaluatingConstructors;
+
+    struct EvaluatingConstructorRAII {
+      EvalInfo &EI;
+      EvaluatingObject Object;
+      bool DidInsert;
+      EvaluatingConstructorRAII(EvalInfo &EI, EvaluatingObject Object)
+          : EI(EI), Object(Object) {
+        DidInsert = EI.EvaluatingConstructors.insert(Object).second;
+      }
+      ~EvaluatingConstructorRAII() {
+        if (DidInsert) EI.EvaluatingConstructors.erase(Object);
+      }
+    };
+
+    bool isEvaluatingConstructor(APValue::LValueBase Decl, unsigned CallIndex) {
+      return Decl == EvaluatingDecl ||
+        EvaluatingConstructors.count(EvaluatingObject(Decl, CallIndex));
+    }
+
     /// The current array initialization index, if we're performing array
     /// initialization.
     uint64_t ArrayInitIndex = -1;
@@ -3101,7 +3125,7 @@
   // FIXME: We don't set up EvaluatingDecl for local variables or temporaries,
   // and this doesn't do quite the right thing for const subobjects of the
   // object under construction.
-  if (LVal.getLValueBase() == Info.EvaluatingDecl) {
+  if (Info.isEvaluatingConstructor(LVal.getLValueBase(), LVal.CallIndex)) {
     BaseType = Info.Ctx.getCanonicalType(BaseType);
     BaseType.removeLocalConst();
   }
@@ -4254,6 +4278,8 @@
     return false;
   }
 
+  EvalInfo::EvaluatingConstructorRAII EvalObj(
+      Info, {This.getLValueBase(), This.CallIndex});
   CallStackFrame Frame(Info, CallLoc, Definition, &This, ArgValues);
 
   // FIXME: Creating an APValue just to hold a nonexistent return value is


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D38483.117445.patch
Type: text/x-patch
Size: 2917 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20171002/197ad9ea/attachment-0001.bin>