[clang][bytecode] Add InitLinkScope for temporary variables

[tbaederr]

No description provided.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

---

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

2 Files Affected:

• (modified) clang/lib/AST/ByteCode/Compiler.cpp (+1-3)
• (modified) clang/test/AST/ByteCode/records.cpp (+6)

diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp
index 0fc942a4f1bc4f..46545a5b3267e6 100644
--- a/clang/lib/AST/ByteCode/Compiler.cpp
+++ b/clang/lib/AST/ByteCode/Compiler.cpp
@@ -3287,6 +3287,7 @@ template <class Emitter> bool Compiler<Emitter>::visit(const Expr *E) {
 
     if (!this->emitGetPtrLocal(*LocalIndex, E))
       return false;
+    InitLinkScope<Emitter> ILS(this, InitLink::Temp(*LocalIndex));
     return this->visitInitializer(E);
   }
 
@@ -4254,9 +4255,6 @@ bool Compiler<Emitter>::VisitCXXThisExpr(const CXXThisExpr *E) {
   // instance pointer of the current function frame, but e.g. to the declaration
   // currently being initialized. Here we emit the necessary instruction(s) for
   // this scenario.
-  if (!InitStackActive || !E->isImplicit())
-    return this->emitThis(E);
-
   if (InitStackActive && !InitStack.empty()) {
     unsigned StartIndex = 0;
     for (StartIndex = InitStack.size() - 1; StartIndex > 0; --StartIndex) {
diff --git a/clang/test/AST/ByteCode/records.cpp b/clang/test/AST/ByteCode/records.cpp
index 7e3cf5b94518f7..2799afa9577b78 100644
--- a/clang/test/AST/ByteCode/records.cpp
+++ b/clang/test/AST/ByteCode/records.cpp
@@ -1653,3 +1653,9 @@ namespace ExprWithCleanups {
   constexpr auto F = true ? 1i : 2i;
   static_assert(F == 1i, "");
 }
+
+namespace ExplicitThisInTemporary {
+  struct B { B *p = this; };
+  constexpr bool g(B b) { return &b == b.p; }
+  static_assert(g({}), "");
+}

[shafik]

Can you add more details to the summary explaining why the fix is necessary.

[tbaederr]

@AaronBallman Can you confirm whether the following behavior is correct?

When a CXXDefaultInitExpr is the child node of an InitListExpr, all CXXThisExpr found within that CXXDefaultInitExpr point to the InitListExpr and not to the actual instance pointer of the current stack frame.

To properly implement this, I'd need to create a temporary variable whenever I encounter a InitListExpr though, which I'd like to avoid.

[AaronBallman]

all CXXThisExpr found within that CXXDefaultInitExpr point to the InitListExpr and not to the actual instance pointer of the current stack frame.

What do you mean "point to"? (They would point to the this object for the class in which the default init expr is found.)

[tbaederr]

In this code:

namespace ExplicitThisInTemporary {
 struct B { B *p = this; };
  constexpr bool g(B b) { return &b == b.p; }
  static_assert(g({}), "");
}

The AST for the static_assert expr is:

CallExpr 0x52100009ed98 '_Bool'
|-ImplicitCastExpr 0x52100009ed78 '_Bool (*)(B)' <FunctionToPointerDecay>
| `-DeclRefExpr 0x5210000754a0 '_Bool (B)' lvalue Function 0x521000075188 'g' '_Bool (B)'
`-InitListExpr 0x52100009edc8 'B':'struct ExplicitThisInTemporary::B'
  `-CXXDefaultInitExpr 0x52100009ee20 'B *' has rewritten init
    `-CXXThisExpr 0x521000075008 'struct ExplicitThisInTemporary::B *' this

the CXXThisExpr here doesn't point to the current instance pointer of the frame (that doesn't even exist), but to the object created by the InitListExpr surrounding it (0x52100009edc8).

[AaronBallman]

In this code:

namespace ExplicitThisInTemporary {
 struct B { B *p = this; };
  constexpr bool g(B b) { return &b == b.p; }
  static_assert(g({}), "");
}

The AST for the static_assert expr is:

CallExpr 0x52100009ed98 '_Bool'
|-ImplicitCastExpr 0x52100009ed78 '_Bool (*)(B)' <FunctionToPointerDecay>
| `-DeclRefExpr 0x5210000754a0 '_Bool (B)' lvalue Function 0x521000075188 'g' '_Bool (B)'
`-InitListExpr 0x52100009edc8 'B':'struct ExplicitThisInTemporary::B'
  `-CXXDefaultInitExpr 0x52100009ee20 'B *' has rewritten init
    `-CXXThisExpr 0x521000075008 'struct ExplicitThisInTemporary::B *' this

the CXXThisExpr here doesn't point to the current instance pointer of the frame (that doesn't even exist), but to the object created by the InitListExpr surrounding it (0x52100009edc8).

I believe that is correct. The InitListExpr is responsible for creating and initializing the B object (https://eel.is/c++draft/dcl.init#aggr-5) and this would refer to that object (https://eel.is/c++draft/expr.prim.this#4).

CC @zygoloid @hubert-reinterpretcast @cor3ntin @shafik for some additional opinions.

[tbaederr]

Fixed differently by #122871