-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[clang][bytecode] Don't ignore discarded ArraySubScriptExprs #137526
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
Conversation
We need to evaluate them since the index might be out of bounds.
@llvm/pr-subscribers-clang Author: Timm Baeder (tbaederr) ChangesWe need to evaluate them since the index might be out of bounds. Full diff: https://github.com/llvm/llvm-project/pull/137526.diff 2 Files Affected:
diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp
index 58fe2c184cf3f..a9610835c81e6 100644
--- a/clang/lib/AST/ByteCode/Compiler.cpp
+++ b/clang/lib/AST/ByteCode/Compiler.cpp
@@ -1695,9 +1695,6 @@ bool Compiler<Emitter>::VisitArraySubscriptExpr(const ArraySubscriptExpr *E) {
const Expr *Index = E->getIdx();
const Expr *Base = E->getBase();
- if (DiscardResult)
- return this->discard(LHS) && this->discard(RHS);
-
// C++17's rules require us to evaluate the LHS first, regardless of which
// side is the base.
bool Success = true;
@@ -1728,7 +1725,11 @@ bool Compiler<Emitter>::VisitArraySubscriptExpr(const ArraySubscriptExpr *E) {
return false;
}
- return this->emitArrayElemPtrPop(*IndexT, E);
+ if (!this->emitArrayElemPtrPop(*IndexT, E))
+ return false;
+ if (DiscardResult)
+ return this->emitPopPtr(E);
+ return true;
}
template <class Emitter>
diff --git a/clang/test/AST/ByteCode/arrays.cpp b/clang/test/AST/ByteCode/arrays.cpp
index 934c3a3e7aff1..2dd51c2fa6711 100644
--- a/clang/test/AST/ByteCode/arrays.cpp
+++ b/clang/test/AST/ByteCode/arrays.cpp
@@ -1,6 +1,6 @@
-// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify=expected,both %s
-// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++20 -verify=expected,both %s
-// RUN: %clang_cc1 -verify=ref,both %s
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify=expected,both %s
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify=expected,both -std=c++20 %s
+// RUN: %clang_cc1 -verify=ref,both %s
// RUN: %clang_cc1 -verify=ref,both -std=c++20 %s
constexpr int m = 3;
@@ -771,3 +771,11 @@ namespace OnePastEndDiag {
constexpr int k = a(foo + 2); // both-error {{must be initialized by a constant expression}} \
// both-note {{in call to 'a(&foo[2])'}}
}
+
+namespace DiscardedSubScriptExpr {
+ constexpr bool foo() { // both-error {{never produces a constant expression}}
+ int a[2] = {};
+ (void)a[3]; // both-note {{cannot refer to element 3 of array of 2 elements in a constant expression}}
+ return true;
+ }
+}
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/59/builds/16742 Here is the relevant piece of the build log for the reference
|
…7526) We need to evaluate them since the index might be out of bounds.
…7526) We need to evaluate them since the index might be out of bounds.
…7526) We need to evaluate them since the index might be out of bounds.
…7526) We need to evaluate them since the index might be out of bounds.
…7526) We need to evaluate them since the index might be out of bounds.
We need to evaluate them since the index might be out of bounds.