[clang][Interp] This pointers are writable in constructors

[tbaeder]

Summary

This is a c++20 extension as far as I know.

[tbaeder]

Ping

[tbaeder]

Ping

[tbaeder]

Ping

[tbaeder]

Ping

[aaron.ballman]

clang/lib/AST/Interp/Interp.cpp

264–270

The same is true for destructors as well: https://godbolt.org/z/a49aEErz8

[tbaeder]

clang/lib/AST/Interp/Interp.cpp

264–270

Oh, good catch, thanks.

[shafik]

clang/lib/AST/Interp/Interp.cpp

264–270

Interesting case: https://godbolt.org/z/5r5fdh9jr

Implementation divergence, have to figure out who is correct here.

[shafik]

clang/lib/AST/Interp/Interp.cpp

264–270

I believe clang is correct: https://eel.is/c++draft/expr.const#4.8

[tbaeder]

clang/lib/AST/Interp/Interp.cpp

264–270

The output is the same with the new constant interpreter; that case is probably handled before it's constant evaluated.

[tbaeder]

Ping

[aaron.ballman]

clang/lib/AST/Interp/Interp.cpp

265–269

Combining predicates; NFC

clang/test/AST/Interp/cxx20.cpp

133–142

I'd feel more comfortable if we had some way to validate that the write to a in both of these cases actually caused the correct value to appear in a. We're just validating that this compiles and doesn't crash, but we're not validating that the actual behavior occurs. How about adding tests like:

template <bool Good>
struct ctor_test {
  int a = 0;

  constexpr ctor_test() {
    if (Good)
      a = 10;
    int local = 100 / a;
  }
};

template <bool Good>
struct dtor_test {
  int a = 0;

  constexpr dtor_test() = default;
  constexpr ~dtor_test() {
    if (Good)
      a = 10;
    int local = 100 / a;
  }
};

constexpr ctor_test<true> good_ctor;
constexpr dtor_test<true> good_dtor;

constexpr ctor_test<false> bad_ctor;
constexpr dtor_test<false> bad_dtor;

[tbaeder]

Added the new test case.

[aaron.ballman]

LGTM!

clang/lib/AST/Interp/Interp.cpp

 }
 
 bool CheckConst(InterpState &S, CodePtr OpPC, const Pointer &Ptr) {
   assert(Ptr.isLive() && "Pointer is not live");
   if (!Ptr.isConst()) {
     return true;
   }
 
+  const Function *Func = S.Current->getFunction();
+  if (Func && Func->isConstructor()) {
+    // The This pointer is writable in constructors, even if
+    // isConst() returns true.
+    if (Ptr.block() == S.Current->getThis().block())
+      return true;
+  }
+
   const QualType Ty = Ptr.getType();
   const SourceInfo &Loc = S.Current->getSource(OpPC);
   S.FFDiag(Loc, diag::note_constexpr_modify_const_type) << Ty;
   return false;
 }
 
 bool CheckMutable(InterpState &S, CodePtr OpPC, const Pointer &Ptr) {
   assert(Ptr.isLive() && "Pointer is not live");

clang/lib/AST/Interp/Pointer.h

public:
   /// Returns the byte offset from the start.
   unsigned getByteOffset() const {
     return Offset;
   }
 
   /// Returns the number of elements.
   unsigned getNumElems() const { return getSize() / elemSize(); }
 
+  Block *block() const { return Pointee; }
+
   /// Returns the index into an array.
   int64_t getIndex() const {
     if (isElementPastEnd())
       return 1;
     if (auto ElemSize = elemSize())
       return getOffset() / ElemSize;
     return 0;
   }

clang/test/AST/Interp/cxx20.cpp

public:
     constexpr Derived() : Base() {} // expected-note {{subobject of type 'int' is not initialized}}
   };
 
 constexpr Derived D; // expected-error {{must be initialized by a constant expression}} \\
                      // expected-note {{in call to 'Derived()'}} \
                      // ref-error {{must be initialized by a constant expression}} \
                      // ref-note {{subobject of type 'int' is not initialized}}
 };
+
+namespace ConstThis {
+  class Foo {
+    const int T = 12; // expected-note {{declared const here}} \
+                      // ref-note {{declared const here}}
+    int a;
+  public:
+    constexpr Foo() {
+      this->a = 10;
+      T = 13; // expected-error {{cannot assign to non-static data member 'T' with const-qualified type}} \
+              // ref-error {{cannot assign to non-static data member 'T' with const-qualified type}}
+    }
+  };
+  constexpr Foo F; // expected-error {{must be initialized by a constant expression}} \
+                   // ref-error {{must be initialized by a constant expression}}
+
+};