Skip to content

[Clang] prevent assertion failure in value-dependent initializer expressions #112612

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Oct 24, 2024

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
4 participants