https://github.com/efriedma-quic created
https://github.com/llvm/llvm-project/pull/151053
6a60f18997d62b0e2842a921fcb6beb3e52ed823 fixed the primary issue of
dereferences, but there are some expressions that depend on the identity of the
pointed-to object without actually accessing it. Handle those cases.
Also, while I'm here, fix a crash in interpreter mode comparing typeid to
nullptr.
>From 43e441c0559294f0e4d85cf67bea480140b2e9d1 Mon Sep 17 00:00:00 2001
From: Eli Friedman <efrie...@quicinc.com>
Date: Mon, 28 Jul 2025 15:47:33 -0700
Subject: [PATCH] [clang] Followup for constexpr-unknown potential constant
expressions.
6a60f18997d62b0e2842a921fcb6beb3e52ed823 fixed the primary issue of
dereferences, but there are some expressions that depend on the
identity of the pointed-to object without actually accessing it. Handle
those cases.
Also, while I'm here, fix a crash in interpreter mode comparing typeid
to nullptr.
---
clang/lib/AST/ByteCode/Pointer.cpp | 5 +-
clang/lib/AST/ExprConstant.cpp | 141 ++++++++++--------
.../SemaCXX/constant-expression-p2280r4.cpp | 38 ++++-
3 files changed, 121 insertions(+), 63 deletions(-)
diff --git a/clang/lib/AST/ByteCode/Pointer.cpp
b/clang/lib/AST/ByteCode/Pointer.cpp
index 4019b74669282..d94251996ba16 100644
--- a/clang/lib/AST/ByteCode/Pointer.cpp
+++ b/clang/lib/AST/ByteCode/Pointer.cpp
@@ -563,8 +563,11 @@ bool Pointer::hasSameBase(const Pointer &A, const Pointer
&B) {
if (A.isTypeidPointer() && B.isTypeidPointer())
return true;
- if (A.isIntegralPointer() || B.isIntegralPointer())
+ if (A.isIntegralPointer() || B.isIntegralPointer()) {
+ if (A.isTypeidPointer() || B.isTypeidPointer())
+ return false;
return A.getSource() == B.getSource();
+ }
if (A.StorageKind != B.StorageKind)
return false;
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 9808298a1b1d0..373cf14859bbd 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -465,49 +465,7 @@ namespace {
void diagnosePointerArithmetic(EvalInfo &Info, const Expr *E,
const APSInt &N);
/// Add N to the address of this subobject.
- void adjustIndex(EvalInfo &Info, const Expr *E, APSInt N) {
- if (Invalid || !N) return;
- uint64_t TruncatedN = N.extOrTrunc(64).getZExtValue();
- if (isMostDerivedAnUnsizedArray()) {
- diagnoseUnsizedArrayPointerArithmetic(Info, E);
- // Can't verify -- trust that the user is doing the right thing (or if
- // not, trust that the caller will catch the bad behavior).
- // FIXME: Should we reject if this overflows, at least?
- Entries.back() = PathEntry::ArrayIndex(
- Entries.back().getAsArrayIndex() + TruncatedN);
- return;
- }
-
- // [expr.add]p4: For the purposes of these operators, a pointer to a
- // nonarray object behaves the same as a pointer to the first element of
- // an array of length one with the type of the object as its element
type.
- bool IsArray = MostDerivedPathLength == Entries.size() &&
- MostDerivedIsArrayElement;
- uint64_t ArrayIndex = IsArray ? Entries.back().getAsArrayIndex()
- : (uint64_t)IsOnePastTheEnd;
- uint64_t ArraySize =
- IsArray ? getMostDerivedArraySize() : (uint64_t)1;
-
- if (N < -(int64_t)ArrayIndex || N > ArraySize - ArrayIndex) {
- // Calculate the actual index in a wide enough type, so we can include
- // it in the note.
- N = N.extend(std::max<unsigned>(N.getBitWidth() + 1, 65));
- (llvm::APInt&)N += ArrayIndex;
- assert(N.ugt(ArraySize) && "bounds check failed for in-bounds index");
- diagnosePointerArithmetic(Info, E, N);
- setInvalid();
- return;
- }
-
- ArrayIndex += TruncatedN;
- assert(ArrayIndex <= ArraySize &&
- "bounds check succeeded for out-of-bounds index");
-
- if (IsArray)
- Entries.back() = PathEntry::ArrayIndex(ArrayIndex);
- else
- IsOnePastTheEnd = (ArrayIndex != 0);
- }
+ void adjustIndex(EvalInfo &Info, const Expr *E, APSInt N, LValue &LV);
};
/// A scope at the end of which an object can need to be destroyed.
@@ -1799,7 +1757,7 @@ namespace {
Offset = CharUnits::fromQuantity(Offset64 + ElemSize64 * Index64);
if (checkNullPointer(Info, E, CSK_ArrayIndex))
- Designator.adjustIndex(Info, E, Index);
+ Designator.adjustIndex(Info, E, Index, *this);
clearIsNullPointer();
}
void adjustOffset(CharUnits N) {
@@ -1907,6 +1865,54 @@ namespace {
}
}
+void SubobjectDesignator::adjustIndex(EvalInfo &Info, const Expr *E, APSInt N,
+ LValue &LV) {
+ if (Invalid || !N)
+ return;
+ uint64_t TruncatedN = N.extOrTrunc(64).getZExtValue();
+ if (isMostDerivedAnUnsizedArray()) {
+ diagnoseUnsizedArrayPointerArithmetic(Info, E);
+ // Can't verify -- trust that the user is doing the right thing (or if
+ // not, trust that the caller will catch the bad behavior).
+ // FIXME: Should we reject if this overflows, at least?
+ Entries.back() =
+ PathEntry::ArrayIndex(Entries.back().getAsArrayIndex() + TruncatedN);
+ return;
+ }
+
+ // [expr.add]p4: For the purposes of these operators, a pointer to a
+ // nonarray object behaves the same as a pointer to the first element of
+ // an array of length one with the type of the object as its element type.
+ bool IsArray =
+ MostDerivedPathLength == Entries.size() && MostDerivedIsArrayElement;
+ uint64_t ArrayIndex =
+ IsArray ? Entries.back().getAsArrayIndex() : (uint64_t)IsOnePastTheEnd;
+ uint64_t ArraySize = IsArray ? getMostDerivedArraySize() : (uint64_t)1;
+
+ if (N < -(int64_t)ArrayIndex || N > ArraySize - ArrayIndex) {
+ if (!Info.checkingPotentialConstantExpression() ||
+ !LV.AllowConstexprUnknown) {
+ // Calculate the actual index in a wide enough type, so we can include
+ // it in the note.
+ N = N.extend(std::max<unsigned>(N.getBitWidth() + 1, 65));
+ (llvm::APInt &)N += ArrayIndex;
+ assert(N.ugt(ArraySize) && "bounds check failed for in-bounds index");
+ diagnosePointerArithmetic(Info, E, N);
+ }
+ setInvalid();
+ return;
+ }
+
+ ArrayIndex += TruncatedN;
+ assert(ArrayIndex <= ArraySize &&
+ "bounds check succeeded for out-of-bounds index");
+
+ if (IsArray)
+ Entries.back() = PathEntry::ArrayIndex(ArrayIndex);
+ else
+ IsOnePastTheEnd = (ArrayIndex != 0);
+}
+
static bool Evaluate(APValue &Result, EvalInfo &Info, const Expr *E);
static bool EvaluateInPlace(APValue &Result, EvalInfo &Info,
const LValue &This, const Expr *E,
@@ -5126,12 +5132,18 @@ static bool HandleBaseToDerivedCast(EvalInfo &Info,
const CastExpr *E,
if (const PointerType *PT = TargetQT->getAs<PointerType>())
TargetQT = PT->getPointeeType();
- // Check this cast lands within the final derived-to-base subobject path.
- if (D.MostDerivedPathLength + E->path_size() > D.Entries.size()) {
- Info.CCEDiag(E, diag::note_constexpr_invalid_downcast)
- << D.MostDerivedType << TargetQT;
+ auto InvalidCast = [&]() {
+ if (!Info.checkingPotentialConstantExpression() ||
+ !Result.AllowConstexprUnknown) {
+ Info.CCEDiag(E, diag::note_constexpr_invalid_downcast)
+ << D.MostDerivedType << TargetQT;
+ }
return false;
- }
+ };
+
+ // Check this cast lands within the final derived-to-base subobject path.
+ if (D.MostDerivedPathLength + E->path_size() > D.Entries.size())
+ return InvalidCast();
// Check the type of the final cast. We don't need to check the path,
// since a cast can only be formed if the path is unique.
@@ -5142,11 +5154,8 @@ static bool HandleBaseToDerivedCast(EvalInfo &Info,
const CastExpr *E,
FinalType = D.MostDerivedType->getAsCXXRecordDecl();
else
FinalType = getAsBaseClass(D.Entries[NewEntriesSize - 1]);
- if (FinalType->getCanonicalDecl() != TargetType->getCanonicalDecl()) {
- Info.CCEDiag(E, diag::note_constexpr_invalid_downcast)
- << D.MostDerivedType << TargetQT;
- return false;
- }
+ if (FinalType->getCanonicalDecl() != TargetType->getCanonicalDecl())
+ return InvalidCast();
// Truncate the lvalue to the appropriate derived class.
return CastToDerivedClass(Info, E, Result, TargetType, NewEntriesSize);
@@ -6104,12 +6113,15 @@ static bool checkDynamicType(EvalInfo &Info, const Expr
*E, const LValue &This,
} else if (Polymorphic) {
// Conservatively refuse to perform a polymorphic operation if we would
// not be able to read a notional 'vptr' value.
- APValue Val;
- This.moveInto(Val);
- QualType StarThisType =
- Info.Ctx.getLValueReferenceType(This.Designator.getType(Info.Ctx));
- Info.FFDiag(E, diag::note_constexpr_polymorphic_unknown_dynamic_type)
- << AK << Val.getAsString(Info.Ctx, StarThisType);
+ if (!Info.checkingPotentialConstantExpression() ||
+ !This.AllowConstexprUnknown) {
+ APValue Val;
+ This.moveInto(Val);
+ QualType StarThisType =
+ Info.Ctx.getLValueReferenceType(This.Designator.getType(Info.Ctx));
+ Info.FFDiag(E, diag::note_constexpr_polymorphic_unknown_dynamic_type)
+ << AK << Val.getAsString(Info.Ctx, StarThisType);
+ }
return false;
}
return true;
@@ -14554,6 +14566,11 @@ EvaluateComparisonBinaryOperator(EvalInfo &Info, const
BinaryOperator *E,
// Reject differing bases from the normal codepath; we special-case
// comparisons to null.
if (!HasSameBase(LHSValue, RHSValue)) {
+ // Bail out early if we're checking potential constant expression.
+ // Otherwise, prefer to diagnose other issues.
+ if (Info.checkingPotentialConstantExpression() &&
+ (LHSValue.AllowConstexprUnknown || RHSValue.AllowConstexprUnknown))
+ return false;
auto DiagComparison = [&] (unsigned DiagID, bool Reversed = false) {
std::string LHS = LHSValue.toString(Info.Ctx, E->getLHS()->getType());
std::string RHS = RHSValue.toString(Info.Ctx, E->getRHS()->getType());
@@ -14874,6 +14891,10 @@ bool IntExprEvaluator::VisitBinaryOperator(const
BinaryOperator *E) {
// Reject differing bases from the normal codepath; we special-case
// comparisons to null.
if (!HasSameBase(LHSValue, RHSValue)) {
+ if (Info.checkingPotentialConstantExpression() &&
+ (LHSValue.AllowConstexprUnknown || RHSValue.AllowConstexprUnknown))
+ return false;
+
const Expr *LHSExpr = LHSValue.Base.dyn_cast<const Expr *>();
const Expr *RHSExpr = RHSValue.Base.dyn_cast<const Expr *>();
diff --git a/clang/test/SemaCXX/constant-expression-p2280r4.cpp
b/clang/test/SemaCXX/constant-expression-p2280r4.cpp
index 312a77830420b..78e2e17016280 100644
--- a/clang/test/SemaCXX/constant-expression-p2280r4.cpp
+++ b/clang/test/SemaCXX/constant-expression-p2280r4.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -std=c++23 -verify=expected,nointerpreter %s
-// RUN: %clang_cc1 -std=c++23 -verify=expected,interpreter %s
-fexperimental-new-constant-interpreter
+// RUN: %clang_cc1 -std=c++23 -verify=expected,nointerpreter
-Winvalid-constexpr %s
+// RUN: %clang_cc1 -std=c++23 -verify=expected,interpreter %s
-fexperimental-new-constant-interpreter -Winvalid-constexpr
using size_t = decltype(sizeof(0));
@@ -397,3 +397,37 @@ namespace GH150015 {
// nointerpreter-note
{{comparison of addresses of subobjects of different base classes has
unspecified value}} \
// interpreter-note
{{initializer of 'z' is unknown}}
}
+
+namespace InvalidConstexprFn {
+ // Make sure we don't trigger -Winvalid-constexpr incorrectly.
+ constexpr bool same_address(const int &a, const int &b) { return &a == &b; }
+ constexpr int next_element(const int &p) { return (&p)[2]; }
+
+ struct Base {};
+ struct Derived : Base { int n; };
+ constexpr int get_derived_member(const Base& b) { return static_cast<const
Derived&>(b).n; }
+
+ struct PolyBase {
+ constexpr virtual int get() const { return 0; }
+ };
+ struct PolyDerived : PolyBase {
+ constexpr int get() const override { return 1; }
+ };
+ constexpr int virtual_call(const PolyBase& b) { return b.get(); }
+ constexpr auto* type(const PolyBase& b) { return &typeid(b); }
+ // FIXME: Intepreter doesn't support constexpr dynamic_cast yet.
+ constexpr const void* dyncast(const PolyBase& b) { return dynamic_cast<const
void*>(&b); } // interpreter-error {{constexpr function never produces a
constant expression}} \
+
// interpreter-note 2 {{subexpression not valid in a constant
expression}}
+ constexpr int sub(const int (&a)[], const int (&b)[]) { return a-b; }
+ constexpr const int* add(const int &a) { return &a+3; }
+
+ constexpr int arr[3]{0, 1, 2};
+ static_assert(same_address(arr[1], arr[1]));
+ static_assert(next_element(arr[0]) == 2);
+ static_assert(get_derived_member(Derived{}) == 0);
+ static_assert(virtual_call(PolyDerived{}) == 1);
+ static_assert(type(PolyDerived{}) != nullptr);
+ static_assert(dyncast(PolyDerived{}) != nullptr); // interpreter-error
{{static assertion expression is not an integral constant expression}}
interpreter-note {{in call}}
+ static_assert(sub(arr, arr) == 0);
+ static_assert(add(arr[0]) == &arr[3]);
+}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
