[clang] [clang][bytecode] Fix const-in-mutable fields (PR #149286)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

<details>
<summary>Changes</summary>

For mutable and const fields, we have two bits in InlineDescriptor, which both get inherited down the hierarchy. When a field is both const and mutable, we CAN read from it if it is a mutable-in-const field, but we _can't_ read from it if it is a const-in-mutable field. We need another bit to distinguish the two cases.

---
Full diff: https://github.com/llvm/llvm-project/pull/149286.diff


5 Files Affected:

- (modified) clang/lib/AST/ByteCode/Descriptor.cpp (+3-1) 
- (modified) clang/lib/AST/ByteCode/Descriptor.h (+4) 
- (modified) clang/lib/AST/ByteCode/Interp.cpp (+1-1) 
- (modified) clang/lib/AST/ByteCode/Pointer.h (+5) 
- (modified) clang/test/AST/ByteCode/mutable.cpp (+48-8) 


``````````diff
diff --git a/clang/lib/AST/ByteCode/Descriptor.cpp b/clang/lib/AST/ByteCode/Descriptor.cpp
index c89eca9bef440..2d5334dbb46b6 100644
--- a/clang/lib/AST/ByteCode/Descriptor.cpp
+++ b/clang/lib/AST/ByteCode/Descriptor.cpp
@@ -160,8 +160,10 @@ static void initField(Block *B, std::byte *Ptr, bool IsConst, bool IsMutable,
   Desc->IsActive = IsActive && !IsUnionField;
   Desc->InUnion = InUnion;
   Desc->IsConst = IsConst || D->IsConst;
-  Desc->IsFieldMutable = IsMutable || D->IsMutable;
+  Desc->IsFieldMutable = (IsMutable || D->IsMutable);
   Desc->IsVolatile = IsVolatile || D->IsVolatile;
+  // True if this field is const AND the parent is mutable.
+  Desc->IsConstInMutable = Desc->IsConst && IsMutable;
 
   if (auto Fn = D->CtorFn)
     Fn(B, Ptr + FieldOffset, Desc->IsConst, Desc->IsFieldMutable,
diff --git a/clang/lib/AST/ByteCode/Descriptor.h b/clang/lib/AST/ByteCode/Descriptor.h
index 4591eabb69bb4..0227e4c0c7e38 100644
--- a/clang/lib/AST/ByteCode/Descriptor.h
+++ b/clang/lib/AST/ByteCode/Descriptor.h
@@ -101,6 +101,10 @@ struct InlineDescriptor {
   /// Flag indicating if the field is mutable (if in a record).
   LLVM_PREFERRED_TYPE(bool)
   unsigned IsFieldMutable : 1;
+  /// Flag indicating if this field is a const field nested in
+  /// a mutable parent field.
+  LLVM_PREFERRED_TYPE(bool)
+  unsigned IsConstInMutable : 1;
   /// Flag indicating if the field is an element of a composite array.
   LLVM_PREFERRED_TYPE(bool)
   unsigned IsArrayElement : 1;
diff --git a/clang/lib/AST/ByteCode/Interp.cpp b/clang/lib/AST/ByteCode/Interp.cpp
index edb1866b5265c..709b3bb64d280 100644
--- a/clang/lib/AST/ByteCode/Interp.cpp
+++ b/clang/lib/AST/ByteCode/Interp.cpp
@@ -566,7 +566,7 @@ bool CheckDowncast(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
 
 bool CheckConst(InterpState &S, CodePtr OpPC, const Pointer &Ptr) {
   assert(Ptr.isLive() && "Pointer is not live");
-  if (!Ptr.isConst() || Ptr.isMutable())
+  if (!Ptr.isConst() || !Ptr.isConstInMutable())
     return true;
 
   if (!Ptr.isBlockPointer())
diff --git a/clang/lib/AST/ByteCode/Pointer.h b/clang/lib/AST/ByteCode/Pointer.h
index e6a64e6658f06..da74013cf83a6 100644
--- a/clang/lib/AST/ByteCode/Pointer.h
+++ b/clang/lib/AST/ByteCode/Pointer.h
@@ -576,6 +576,11 @@ class Pointer {
       return true;
     return isRoot() ? getDeclDesc()->IsConst : getInlineDesc()->IsConst;
   }
+  bool isConstInMutable() const {
+    if (!isBlockPointer())
+      return false;
+    return isRoot() ? false : getInlineDesc()->IsConstInMutable;
+  }
 
   /// Checks if an object or a subfield is volatile.
   bool isVolatile() const {
diff --git a/clang/test/AST/ByteCode/mutable.cpp b/clang/test/AST/ByteCode/mutable.cpp
index aebbea920578c..35c5a0389921e 100644
--- a/clang/test/AST/ByteCode/mutable.cpp
+++ b/clang/test/AST/ByteCode/mutable.cpp
@@ -1,11 +1,7 @@
-// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++11 -verify=expected,expected11,both,both11 %s
-// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++14 -verify=expected,expected14,both %s
-// RUN: %clang_cc1 -std=c++11 -verify=ref,ref11,both,both11 %s
-// RUN: %clang_cc1 -std=c++14 -verify=ref,ref14,both %s
-
-
-
-
+// RUN: %clang_cc1 -std=c++11 -verify=expected,expected11,both,both11 %s -fexperimental-new-constant-interpreter
+// RUN: %clang_cc1 -std=c++14 -verify=expected,expected14,both        %s -fexperimental-new-constant-interpreter
+// RUN: %clang_cc1 -std=c++11 -verify=ref,ref11,both,both11           %s
+// RUN: %clang_cc1 -std=c++14 -verify=ref,ref14,both                  %s
 
 namespace Simple {
   struct S {
@@ -26,3 +22,47 @@ namespace Simple {
   static_assert(s2.a2 == 12, ""); // both11-error {{not an integral constant expression}} \
                                   // both11-note {{initializer of 's2' is not a constant expression}}
 }
+#if __cplusplus >= 201402L
+namespace ConstInMutable {
+  class B {
+    public:
+
+    const int f;
+    constexpr B() : f(12) {}
+  };
+  class A {
+    public:
+    mutable B b;
+    constexpr A() = default;
+  };
+  constexpr int constInMutable() {
+    A a;
+
+    int *m = (int*)&a.b.f;
+    *m = 12; // both-note {{modification of object of const-qualified type 'const int' is not allowed in a constant expression}}
+    return 1;
+  }
+  static_assert(constInMutable() == 1, ""); // both-error {{not an integral constant expression}} \
+                                            // both-note {{in call to}}
+}
+
+namespace MutableInConst {
+  class C {
+  public:
+    mutable int c;
+    constexpr C() : c(50) {}
+  };
+  class D {
+  public:
+    C c;
+    constexpr D() {}
+  };
+  constexpr int mutableInConst() {
+    const D d{};
+    int *m = (int*)&d.c.c;
+    *m = 12;
+    return 1;
+  }
+  static_assert(mutableInConst() == 1, "");
+}
+#endif

``````````

</details>


https://github.com/llvm/llvm-project/pull/149286