llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Yihan Wang (yronglin)
<details>
<summary>Changes</summary>
A default member initializer can refer to previously initialized subobjects, so
`this` has to denote something while it is evaluated. A discarded composite
prvalue had no result object, so the interpreter rejected code the legacy
evaluator accepts:
```cpp
struct A { int &x; constexpr ~A() { x = 0; } };
struct B { int &x; const A &a = A{x}; };
constexpr int f() { int x = 1; B{x}; return x; }
constexpr int r = f(); // error: implicit use of 'this' pointer
static_assert(r == 0); // FXIME: 'r' shouble be 0, it's a bug in clang, see
https://github.com/llvm/llvm-project/issues/85601.
```
https://godbolt.org/z/rb993Ef9d
This patch changes to allocate a result object for such an expression and
record an InitLink to it, letting an `ExprWithCleanups` establish its
full-expression scope first. Also emit `StartInit`/`EndInit` around a
materialized temporary and around a top-level result object, so a
const-qualified composite stays writable while it is under construction.
---
Full diff: https://github.com/llvm/llvm-project/pull/219952.diff
3 Files Affected:
- (modified) clang/lib/AST/ByteCode/Compiler.cpp (+60-2)
- (modified) clang/lib/AST/ByteCode/Compiler.h (+6)
- (modified) clang/test/AST/ByteCode/records.cpp (+24-2)
``````````diff
diff --git a/clang/lib/AST/ByteCode/Compiler.cpp
b/clang/lib/AST/ByteCode/Compiler.cpp
index c182639ea07f8..cf4945a3f2ff6 100644
--- a/clang/lib/AST/ByteCode/Compiler.cpp
+++ b/clang/lib/AST/ByteCode/Compiler.cpp
@@ -50,6 +50,22 @@ static bool isSideEffectFree(const Expr *E) {
return false;
}
+static bool containsDefaultInitExpr(const Expr *E) {
+ class Finder final : public ConstDynamicRecursiveASTVisitor {
+ public:
+ Finder() { ShouldVisitImplicitCode = true; }
+
+ bool VisitCXXDefaultInitExpr(const CXXDefaultInitExpr *) override {
+ Found = true;
+ return true;
+ }
+
+ bool Found = false;
+ } F;
+ F.TraverseStmt(E);
+ return F.Found;
+}
+
/// Scope chain managing the variable lifetimes.
template <class Emitter> class VariableScope {
public:
@@ -265,7 +281,9 @@ template <class Emitter> class InitStackScope final {
public:
InitStackScope(Compiler<Emitter> *Ctx, bool Active)
: Ctx(Ctx), OldValue(Ctx->InitStackActive), Active(Active) {
- Ctx->InitStackActive = Active;
+ // An explicit initializer nested in a default member initializer still
+ // needs the surrounding default initializer's `this` reconstruction.
+ Ctx->InitStackActive = OldValue || Active;
if (Active)
Ctx->InitStack.push_back(InitLink::DIE());
}
@@ -3475,6 +3493,9 @@ bool Compiler<Emitter>::VisitExprWithCleanups(const
ExprWithCleanups *E) {
LocalScope<Emitter> ES(this, ScopeKind::FullExpression);
const Expr *SubExpr = E->getSubExpr();
+ if (DiscardResult && this->discardNeedsResultObject(SubExpr))
+ return this->discardIntoResultObject(SubExpr) && ES.destroyLocals(E);
+
return this->delegate(SubExpr) && ES.destroyLocals(E);
}
@@ -3541,8 +3562,12 @@ bool Compiler<Emitter>::VisitMaterializeTemporaryExpr(
// Non-primitive values.
if (!this->emitGetPtrGlobal(*GlobalIndex, E))
return false;
+ if (!this->emitStartInit(E))
+ return false;
if (!this->visitInitializer(Inner))
return false;
+ if (!this->emitEndInit(E))
+ return false;
if (IsStatic) {
assert(TempDecl);
return this->emitInitGlobalTempComp(TempDecl, E);
@@ -3585,7 +3610,11 @@ bool Compiler<Emitter>::VisitMaterializeTemporaryExpr(
if (!this->emitGetPtrLocal(*LocalIndex, E))
return false;
- return this->visitInitializer(Inner);
+ if (!this->emitStartInit(E))
+ return false;
+ if (!this->visitInitializer(Inner))
+ return false;
+ return this->emitEndInit(E);
}
return false;
}
@@ -4887,7 +4916,30 @@ bool Compiler<Emitter>::VisitStmtExpr(const StmtExpr *E)
{
return BS.destroyLocals();
}
+template <class Emitter>
+bool Compiler<Emitter>::discardNeedsResultObject(const Expr *E) const {
+ return !E->isGLValue() && !canClassify(E->getType()) &&
+ containsDefaultInitExpr(E);
+}
+
+template <class Emitter>
+bool Compiler<Emitter>::discardIntoResultObject(const Expr *E) {
+ UnsignedOrNone LocalIndex =
+ allocateLocal(E, QualType(), ScopeKind::FullExpression);
+ if (!LocalIndex)
+ return false;
+ InitLinkScope<Emitter> ILS(this, InitLink::Temp(*LocalIndex));
+ if (!this->emitGetPtrLocal(*LocalIndex, E))
+ return false;
+ return this->visitInitializerPop(E);
+}
+
template <class Emitter> bool Compiler<Emitter>::discard(const Expr *E) {
+ // Let an ExprWithCleanups establish its full-expression scope first; it
+ // allocates the result object itself.
+ if (!isa<ExprWithCleanups>(E) && this->discardNeedsResultObject(E))
+ return this->discardIntoResultObject(E);
+
OptionScope<Emitter> Scope(this, /*NewDiscardResult=*/true,
/*NewInitializing=*/false, /*ToLValue=*/false);
return this->Visit(E);
@@ -5451,8 +5503,14 @@ bool Compiler<Emitter>::visitExpr(const Expr *E, bool
DestroyToplevelScope) {
if (!this->emitGetPtrLocal(*LocalOffset, E))
return false;
+ // A const-qualified result object is writable while it is being
+ // initialized, just like an object evaluated through visitVarDecl().
+ if (!this->emitStartInit(E))
+ return false;
if (!visitInitializer(E))
return false;
+ if (!this->emitEndInit(E))
+ return false;
// We are destroying the locals AFTER the Ret op.
// The Ret op needs to copy the (alive) values, but the
// destructors may still turn the entire expression invalid.
diff --git a/clang/lib/AST/ByteCode/Compiler.h
b/clang/lib/AST/ByteCode/Compiler.h
index f34809cd0f14c..6e22f57749e43 100644
--- a/clang/lib/AST/ByteCode/Compiler.h
+++ b/clang/lib/AST/ByteCode/Compiler.h
@@ -321,6 +321,12 @@ class Compiler : public
ConstStmtVisitor<Compiler<Emitter>, bool>,
bool visitAsLValue(const Expr *E);
/// Evaluates an expression for side effects and discards the result.
bool discard(const Expr *E);
+ /// Whether discarding \p E still requires a result object: a composite
+ /// prvalue whose default member initializer may refer to previously
+ /// initialized subobjects, so that `this` has something to denote.
+ bool discardNeedsResultObject(const Expr *E) const;
+ /// Allocate that result object and initialize \p E into it.
+ bool discardIntoResultObject(const Expr *E);
/// Just pass evaluation on to \p E. This leaves all the parsing flags
/// intact.
bool delegate(const Expr *E);
diff --git a/clang/test/AST/ByteCode/records.cpp
b/clang/test/AST/ByteCode/records.cpp
index 36b5cb62fe95f..fc49be7b17332 100644
--- a/clang/test/AST/ByteCode/records.cpp
+++ b/clang/test/AST/ByteCode/records.cpp
@@ -1302,11 +1302,33 @@ namespace {
};
constexpr int a() {
int x = 1;
- int f = B{x}.x;
+ {
+ B b{x};
+ }
+ return x;
+ }
+ static_assert(a() == 0);
+
+ /// Before the result object was allocated, this could not be evaluated at
+ /// all. The temporary B is not destroyed until the end of the enclosing
+ /// full-expression, so x still reads 1 here, matching ExprConstant.
+ constexpr int discarded() {
+ int x = 1;
B{x}; // both-warning {{expression result unused}}
+ return x;
+ }
+ static_assert(discarded() == 1);
- return 1;
+ /// A const-qualified composite result is writable while under construction.
+ constexpr int decrement(int &x) {
+ return --x;
}
+ struct DMIConstComposite {
+ int a;
+ int b = decrement(a);
+ };
+ constexpr DMIConstComposite c{1};
+ static_assert(c.a == 0);
}
#endif
``````````
</details>
https://github.com/llvm/llvm-project/pull/219952
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
