[clang] e28d9ba - PR44958: Allow member calls and typeid / dynamic_cast on mutable objects

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2020-02-18T14:57:13-08:00
New Revision: e28d9bae4b3be60e90daa69a2eeb3254c952e051

URL: https://github.com/llvm/llvm-project/commit/e28d9bae4b3be60e90daa69a2eeb3254c952e051
DIFF: https://github.com/llvm/llvm-project/commit/e28d9bae4b3be60e90daa69a2eeb3254c952e051.diff

LOG: PR44958: Allow member calls and typeid / dynamic_cast on mutable objects
and objects with mutable subobjects.

The standard wording doesn't really cover these cases; accepting all
such cases seems most in line with what we do in other cases and what
other compilers do. (Essentially this means we're assuming that objects
external to the evaluation are always in-lifetime.)

Added: 
    

Modified: 
    clang/lib/AST/ExprConstant.cpp
    clang/test/SemaCXX/constant-expression-cxx2a.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index dfa9444c59aa..641368ebfdd9 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -3140,6 +3140,13 @@ struct CompleteObject {
       : Base(Base), Value(Value), Type(Type) {}
 
   bool mayAccessMutableMembers(EvalInfo &Info, AccessKinds AK) const {
+    // If this isn't a "real" access (eg, if it's just accessing the type
+    // info), allow it. We assume the type doesn't change dynamically for
+    // subobjects of constexpr objects (even though we'd hit UB here if it
+    // did). FIXME: Is this right?
+    if (!isAnyAccess(AK))
+      return true;
+
     // In C++14 onwards, it is permitted to read a mutable member whose
     // lifetime began within the evaluation.
     // FIXME: Should we also allow this in C++11?

diff  --git a/clang/test/SemaCXX/constant-expression-cxx2a.cpp b/clang/test/SemaCXX/constant-expression-cxx2a.cpp
index 9012f377e1bc..3d5172619dbf 100644
--- a/clang/test/SemaCXX/constant-expression-cxx2a.cpp
+++ b/clang/test/SemaCXX/constant-expression-cxx2a.cpp
@@ -1279,3 +1279,57 @@ namespace value_dependent_init {
     A a = T();
   }
 }
+
+namespace mutable_subobjects {
+  struct A {
+    int m;
+    mutable int n; // expected-note 2{{here}}
+    constexpr int f() const { return m; }
+    constexpr int g() const { return n; } // expected-note {{mutable}}
+  };
+
+  constexpr A a = {1, 2};
+  static_assert(a.f() == 1); // OK (PR44958)
+  static_assert(a.g() == 2); // expected-error {{constant}} expected-note {{in call}}
+
+  constexpr A b = a; // expected-error {{constant}} expected-note {{read of mutable member 'n'}} expected-note {{in call}}
+
+  auto &ti1 = typeid(a);
+  auto &ti2 = typeid(a.m);
+  auto &ti3 = typeid(a.n);
+
+  constexpr void destroy1() { // expected-error {{constexpr}}
+    a.~A(); // expected-note {{cannot modify an object that is visible outside}}
+  }
+  // FIXME: These should both be rejected once P0593 is implemented.
+  using T = int;
+  constexpr void destroy2() {
+    a.m.~T();
+  }
+  constexpr void destroy3() {
+    a.n.~T();
+  }
+
+  struct X {
+    mutable int n = 0;
+    virtual constexpr ~X() {}
+  };
+  struct Y : X {
+  };
+  constexpr Y y;
+  constexpr const X *p = &y;
+  constexpr const Y *q = dynamic_cast<const Y*>(p);
+
+  // FIXME: It's unclear whether this should be accepted. The dynamic_cast is
+  // undefined after 'z.y.~Y()`, for example. We essentially assume that all
+  // objects that the evaluator can reach have unbounded lifetimes. (We make
+  // the same assumption when evaluating member function calls.)
+  struct Z {
+    mutable Y y;
+  };
+  constexpr Z z;
+  constexpr const X *pz = &z.y;
+  constexpr const Y *qz = dynamic_cast<const Y*>(pz);
+  auto &zti = typeid(z.y);
+  static_assert(&zti == &typeid(Y));
+}