https://github.com/isuckatcs updated https://github.com/llvm/llvm-project/pull/67722
>From fb0bf2fcc9d873ce9cf4269cfb6de8786ac6f343 Mon Sep 17 00:00:00 2001 From: isuckatcs <65320245+isucka...@users.noreply.github.com> Date: Thu, 28 Sep 2023 20:43:23 +0200 Subject: [PATCH 1/3] [clang] Fix a crash from nested ArrayInitLoopExpr When visiting an ArrayInitLoopExpr, clang creates a temporary variable for the source array, however in a nested AILE this variable is created multiple times, in every iteration as they have the same AST node and clang is unable to differentiate them. Fixes #57135 --- clang/lib/AST/ExprConstant.cpp | 11 ++++++----- .../nested-array-init-loop-in-lambda-capture.cpp | 13 +++++++++++++ 2 files changed, 19 insertions(+), 5 deletions(-) create mode 100644 clang/test/AST/nested-array-init-loop-in-lambda-capture.cpp diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp index fea06b97259fe31..a1f5c262ca10be3 100644 --- a/clang/lib/AST/ExprConstant.cpp +++ b/clang/lib/AST/ExprConstant.cpp @@ -10967,19 +10967,20 @@ bool ArrayExprEvaluator::VisitArrayInitLoopExpr(const ArrayInitLoopExpr *E) { LValue Subobject = This; Subobject.addArray(Info, E, CAT); - bool Success = true; for (EvalInfo::ArrayInitLoopIndex Index(Info); Index != Elements; ++Index) { if (!EvaluateInPlace(Result.getArrayInitializedElt(Index), Info, Subobject, E->getSubExpr()) || !HandleLValueArrayAdjustment(Info, E, Subobject, CAT->getElementType(), 1)) { - if (!Info.noteFailure()) - return false; - Success = false; + + // There's no need to try and evaluate the rest, as those are the exact + // same expressions. + std::ignore = Info.noteFailure(); + return false; } } - return Success; + return true; } bool ArrayExprEvaluator::VisitCXXConstructExpr(const CXXConstructExpr *E) { diff --git a/clang/test/AST/nested-array-init-loop-in-lambda-capture.cpp b/clang/test/AST/nested-array-init-loop-in-lambda-capture.cpp new file mode 100644 index 000000000000000..82d27380b637d03 --- /dev/null +++ b/clang/test/AST/nested-array-init-loop-in-lambda-capture.cpp @@ -0,0 +1,13 @@ +// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++17 -verify %s +// RUN: %clang_cc1 -std=c++17 -verify=ref %s + +// ref-no-diagnostics +// expected-no-diagnostics + +void used_to_crash() { + int s[2][2]; + + int arr[4]; + + arr[0] = [s] { return s[0][0]; }(); +} >From 19c09fb7656ba3b2e170802adfd397dd9aa9fec9 Mon Sep 17 00:00:00 2001 From: isuckatcs <65320245+isucka...@users.noreply.github.com> Date: Thu, 28 Sep 2023 21:26:40 +0200 Subject: [PATCH 2/3] wrapping the temporary into a scope --- clang/lib/AST/ExprConstant.cpp | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp index a1f5c262ca10be3..12b4d6b22c2c2bc 100644 --- a/clang/lib/AST/ExprConstant.cpp +++ b/clang/lib/AST/ExprConstant.cpp @@ -10950,6 +10950,9 @@ bool ArrayExprEvaluator::VisitCXXParenListOrInitListExpr( } bool ArrayExprEvaluator::VisitArrayInitLoopExpr(const ArrayInitLoopExpr *E) { + + FullExpressionRAII Scope(Info); + LValue CommonLV; if (E->getCommonExpr() && !Evaluate(Info.CurrentCall->createTemporary( @@ -10967,20 +10970,19 @@ bool ArrayExprEvaluator::VisitArrayInitLoopExpr(const ArrayInitLoopExpr *E) { LValue Subobject = This; Subobject.addArray(Info, E, CAT); + bool Success = true; for (EvalInfo::ArrayInitLoopIndex Index(Info); Index != Elements; ++Index) { if (!EvaluateInPlace(Result.getArrayInitializedElt(Index), Info, Subobject, E->getSubExpr()) || !HandleLValueArrayAdjustment(Info, E, Subobject, CAT->getElementType(), 1)) { - - // There's no need to try and evaluate the rest, as those are the exact - // same expressions. - std::ignore = Info.noteFailure(); - return false; + if (!Info.noteFailure()) + return false; + Success = false; } } - return true; + return Success; } bool ArrayExprEvaluator::VisitCXXConstructExpr(const CXXConstructExpr *E) { >From 3fc829678775798938e232b8da2c4ddef827bc85 Mon Sep 17 00:00:00 2001 From: isuckatcs <65320245+isucka...@users.noreply.github.com> Date: Fri, 29 Sep 2023 07:30:12 +0200 Subject: [PATCH 3/3] addressed comments --- clang/lib/AST/ExprConstant.cpp | 16 +++++++++++--- clang/test/AST/Interp/cxx20.cpp | 37 +++++++++++++++++++++++++++++++-- 2 files changed, 48 insertions(+), 5 deletions(-) diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp index 12b4d6b22c2c2bc..1c0fa3ad2b6fb3e 100644 --- a/clang/lib/AST/ExprConstant.cpp +++ b/clang/lib/AST/ExprConstant.cpp @@ -10950,9 +10950,6 @@ bool ArrayExprEvaluator::VisitCXXParenListOrInitListExpr( } bool ArrayExprEvaluator::VisitArrayInitLoopExpr(const ArrayInitLoopExpr *E) { - - FullExpressionRAII Scope(Info); - LValue CommonLV; if (E->getCommonExpr() && !Evaluate(Info.CurrentCall->createTemporary( @@ -10972,6 +10969,16 @@ bool ArrayExprEvaluator::VisitArrayInitLoopExpr(const ArrayInitLoopExpr *E) { bool Success = true; for (EvalInfo::ArrayInitLoopIndex Index(Info); Index != Elements; ++Index) { + // C++ [class.temporary]/5 + // There are four contexts in which temporaries are destroyed at a different + // point than the end of the full-expression. [...] The second context is + // when a copy constructor is called to copy an element of an array while + // the entire array is copied [...]. In either case, if the constructor has + // one or more default arguments, the destruction of every temporary created + // in a default argument is sequenced before the construction of the next + // array element, if any. + FullExpressionRAII Scope(Info); + if (!EvaluateInPlace(Result.getArrayInitializedElt(Index), Info, Subobject, E->getSubExpr()) || !HandleLValueArrayAdjustment(Info, E, Subobject, @@ -10980,6 +10987,9 @@ bool ArrayExprEvaluator::VisitArrayInitLoopExpr(const ArrayInitLoopExpr *E) { return false; Success = false; } + + // Make sure we run the destructors too. + Scope.destroy(); } return Success; diff --git a/clang/test/AST/Interp/cxx20.cpp b/clang/test/AST/Interp/cxx20.cpp index df08bb75199d86a..03a1159f65f1747 100644 --- a/clang/test/AST/Interp/cxx20.cpp +++ b/clang/test/AST/Interp/cxx20.cpp @@ -1,5 +1,5 @@ -// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++20 -verify %s -// RUN: %clang_cc1 -std=c++20 -verify=ref %s +// RUN: %clang_cc1 -fcxx-exceptions -fexperimental-new-constant-interpreter -std=c++20 -verify %s +// RUN: %clang_cc1 -fcxx-exceptions -std=c++20 -verify=ref %s void test_alignas_operand() { alignas(8) char dummy; @@ -646,3 +646,36 @@ namespace ImplicitFunction { // expected-error {{not an integral constant expression}} \ // expected-note {{in call to 'callMe()'}} } + +// FIXME: Interp should also be able to evaluate this snippet properly. +namespace ConstexprArrayInitLoopExprDestructors +{ + struct Highlander { + int *p = 0; + constexpr Highlander() {} + constexpr void set(int *p) { this->p = p; ++*p; if (*p != 1) throw "there can be only one"; } // expected-note {{not valid in a constant expression}} + constexpr ~Highlander() { --*p; } + }; + + struct X { + int *p; + constexpr X(int *p) : p(p) {} + constexpr X(const X &x, Highlander &&h = Highlander()) : p(x.p) { + h.set(p); // expected-note {{in call to '&Highlander()->set(&n)'}} + } + }; + + constexpr int f() { + int n = 0; + X x[3] = {&n, &n, &n}; + auto [a, b, c] = x; // expected-note {{in call to 'X(x[0], Highlander())'}} + return n; + } + + static_assert(f() == 0); // expected-error {{not an integral constant expression}} \ + // expected-note {{in call to 'f()'}} + + int main() { + return f(); + } +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits