Skip to content

Conversation

@a-tarasyuk
Copy link
Member

@a-tarasyuk a-tarasyuk commented Oct 16, 2024

Fixes #112140


CXXConstructExpr 0x14209e580 'const S':'const struct S' contains-errors 'void (const int &)' list
`-CXXDefaultArgExpr 0x14209e500 'const int' contains-errors
  `-RecoveryExpr 0x14209daf0 'const int' contains-errors

This change resolves an issue with evaluating ArrayFiller initializers in dependent contexts, especially when they involve a RecoveryExpr. In certain cases, ArrayFiller initializers containing a RecoveryExpr from earlier errors are incorrectly passed to EvaluateInPlace, causing evaluation failures when they are value-dependent.

When this is the case, the initializer is processed through EvaluateDependentExpr, which prevents unnecessary evaluation attempts and ensures proper handling of value-dependent initializers in ArrayFillers.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Oct 16, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 16, 2024

@llvm/pr-subscribers-clang

Author: Oleksandr T. (a-tarasyuk)

Changes

Fixes #112140


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

3 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+1)
  • (modified) clang/lib/AST/ExprConstant.cpp (+3)
  • (modified) clang/test/SemaCXX/constant-expression-cxx11.cpp (+10)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 817e3abef8d566..1cbf95f0b034d4 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -517,6 +517,7 @@ Bug Fixes to C++ Support
   certain situations. (#GH47400), (#GH90896)
 - Fix erroneous templated array size calculation leading to crashes in generated code. (#GH41441)
 - During the lookup for a base class name, non-type names are ignored. (#GH16855)
+- Fixed an assertion failure when evaluating constant expressions in value-dependent initializers (#GH112140)
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 52a7f5778ce6d2..62d88f0f82b760 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -11536,6 +11536,9 @@ bool ArrayExprEvaluator::VisitCXXParenListOrInitListExpr(
   LValue Subobject = This;
   Subobject.addArray(Info, ExprToVisit, CAT);
   auto Eval = [&](const Expr *Init, unsigned ArrayIndex) {
+    if (Init->isValueDependent())
+      return EvaluateDependentExpr(Init, Info);
+
     if (!EvaluateInPlace(Result.getArrayInitializedElt(ArrayIndex), Info,
                          Subobject, Init) ||
         !HandleLValueArrayAdjustment(Info, Init, Subobject,
diff --git a/clang/test/SemaCXX/constant-expression-cxx11.cpp b/clang/test/SemaCXX/constant-expression-cxx11.cpp
index e2ea984b37cd04..e01aaff1459e88 100644
--- a/clang/test/SemaCXX/constant-expression-cxx11.cpp
+++ b/clang/test/SemaCXX/constant-expression-cxx11.cpp
@@ -2564,3 +2564,13 @@ GH50055::E2 GlobalInitNotCE2 = GH50055::testDefaultArgForParam(); // ok, not a c
 constexpr GH50055::E2 GlobalInitCE = (GH50055::E2)-1;
 // expected-error@-1 {{constexpr variable 'GlobalInitCE' must be initialized by a constant expression}}
 // expected-note@-2 {{integer value -1 is outside the valid range of values [0, 7] for the enumeration type 'E2'}}
+
+namespace GH {
+struct S {
+  constexpr S(const int &a = ) { } // expected-error {{expected expression}}
+};
+
+void foo() {
+  constexpr S s[2] = { }; // expected-error {{constexpr variable 's' must be initialized by a constant expression}}
+}
+}

Copy link
Contributor

@cor3ntin cor3ntin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM modulo comment.
Thanks!

@a-tarasyuk a-tarasyuk requested a review from shafik October 19, 2024 13:30
@cor3ntin
Copy link
Contributor

There is a description so I'll merge. Shafik can review post commit if needs be.
Thanks!

@cor3ntin cor3ntin merged commit 61a456b into llvm:main Oct 24, 2024
9 checks passed
Copy link
Collaborator

@shafik shafik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you for adding more details to the summary

@frobtech frobtech mentioned this pull request Oct 25, 2024
NoumanAmir657 pushed a commit to NoumanAmir657/llvm-project that referenced this pull request Nov 4, 2024
…essions (llvm#112612)

Fixes llvm#112140

--- 

```
CXXConstructExpr 0x14209e580 'const S':'const struct S' contains-errors 'void (const int &)' list
`-CXXDefaultArgExpr 0x14209e500 'const int' contains-errors
  `-RecoveryExpr 0x14209daf0 'const int' contains-errors
```

This change resolves an issue with evaluating `ArrayFiller` initializers
in _dependent_ contexts, especially when they involve a `RecoveryExpr`.
In certain cases, `ArrayFiller` initializers containing a `RecoveryExpr`
from earlier errors are incorrectly passed to `EvaluateInPlace`, causing
evaluation failures when they are value-dependent.

When this is the case, the initializer is processed through
`EvaluateDependentExpr`, which prevents unnecessary evaluation attempts
and ensures proper handling of value-dependent initializers in
`ArrayFillers`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[clang++] Assertion `!E->isValueDependent()' failed in (anonymous namespace)::ArrayExprEvaluator::VisitCXXParenListOrInitListExpr

4 participants