[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

227–233

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

[tbaeder]

clang/lib/AST/Interp/Interp.cpp

227–233

Oh, good catch, thanks.

[shafik]

clang/lib/AST/Interp/Interp.cpp

227–233

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

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

[shafik]

clang/lib/AST/Interp/Interp.cpp

227–233

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

[tbaeder]

clang/lib/AST/Interp/Interp.cpp

227–233

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

228–232

Combining predicates; NFC

clang/test/AST/Interp/cxx20.cpp

221–230

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/Function.h

public:
   /// Checks if the function is valid to call in constexpr.
   bool isConstexpr() const { return IsValid; }
 
   /// Checks if the function is virtual.
   bool isVirtual() const;
 
   /// Checks if the function is a constructor.
   bool isConstructor() const { return isa<CXXConstructorDecl>(F); }
+  /// Checks if the function is a destructor.
+  bool isDestructor() const { return isa<CXXDestructorDecl>(F); }
 
   /// Checks if the function is fully done compiling.
   bool isFullyCompiled() const { return IsFullyCompiled; }
 
   bool hasThisPointer() const { return HasThisPointer; }
 
   // Checks if the funtion already has a body attached.
   bool hasBody() const { return HasBody; }

clang/lib/AST/Interp/Interp.cpp

if (!Ptr.isElementPastEnd())
     return true;
   const SourceInfo &Loc = S.Current->getSource(OpPC);
   S.FFDiag(Loc, diag::note_constexpr_past_end_subobject) << CSK;
   return false;
 }
 
 bool CheckConst(InterpState &S, CodePtr OpPC, const Pointer &Ptr) {
   assert(Ptr.isLive() && "Pointer is not live");
-  if (!Ptr.isConst()) {
+  if (!Ptr.isConst())
+    return true;
+
+  // The This pointer is writable in constructors and destructors,
+  // even if isConst() returns true.
+  if (const Function *Func = S.Current->getFunction();
+      Func && (Func->isConstructor() || Func->isDestructor()) &&
+      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");
   if (!Ptr.isMutable()) {
     return true;
   }
 
   const SourceInfo &Loc = S.Current->getSource(OpPC);
   const FieldDecl *Field = Ptr.getField();
   S.FFDiag(Loc, diag::note_constexpr_access_mutable, 1) << AK_Read << Field;

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 C4(){}
   };
   constexpr C4 c4; // expected-error {{must be initialized by a constant expression}} \
                    // expected-note {{subobject of type 'bool' is not initialized}} \
                    // ref-error {{must be initialized by a constant expression}} \
                    // ref-note {{subobject of type 'bool' is not initialized}}
 #endif
 };
+
+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}}
+
+
+  class FooDtor {
+    int a;
+  public:
+    constexpr FooDtor() {
+      this->a = 10;
+    }
+    constexpr ~FooDtor() {
+      this->a = 12;
+    }
+  };
+
+  constexpr int foo() {
+    const FooDtor f;
+    return 0;
+  }
+  static_assert(foo() == 0);
+
+  template <bool Good>
+  struct ctor_test {
+    int a = 0;
+
+    constexpr ctor_test() {
+      if (Good)
+        a = 10;
+      int local = 100 / a; // expected-note {{division by zero}} \
+                           // ref-note {{division by zero}}
+    }
+  };
+
+  template <bool Good>
+  struct dtor_test {
+    int a = 0;
+
+    constexpr dtor_test() = default;
+    constexpr ~dtor_test() {
+      if (Good)
+        a = 10;
+      int local = 100 / a; // expected-note {{division by zero}} \
+                           // ref-note {{division by zero}}
+    }
+  };
+
+  constexpr ctor_test<true> good_ctor;
+  constexpr dtor_test<true> good_dtor;
+
+  constexpr ctor_test<false> bad_ctor; // expected-error {{must be initialized by a constant expression}} \
+                                       // expected-note {{in call to}} \
+                                       // ref-error {{must be initialized by a constant expression}} \
+                                       // ref-note {{in call to}}
+  constexpr dtor_test<false> bad_dtor; // expected-error {{must have constant destruction}} \
+                                       // expected-note {{in call to}} \
+                                       // ref-error {{must have constant destruction}} \
+                                       // ref-note {{in call to}}
+};