Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion clang/lib/AST/ByteCode/Pointer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a comment here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was going to try to explain this... but then I realized the getSource() check isn't actually doing anything, so I just deleted this whole block of code.

return A.getSource() == B.getSource();
}

if (A.StorageKind != B.StorageKind)
return false;
Expand Down
141 changes: 81 additions & 60 deletions clang/lib/AST/ExprConstant.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -1907,6 +1865,54 @@ namespace {
}
}

void SubobjectDesignator::adjustIndex(EvalInfo &Info, const Expr *E, APSInt N,
LValue &LV) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
void SubobjectDesignator::adjustIndex(EvalInfo &Info, const Expr *E, APSInt N,
LValue &LV) {
void SubobjectDesignator::adjustIndex(EvalInfo &Info, const Expr *E, APSInt N,
const 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,
Expand Down Expand Up @@ -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.
Expand All @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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 *>();

Expand Down
38 changes: 36 additions & 2 deletions clang/test/SemaCXX/constant-expression-p2280r4.cpp
Original file line number Diff line number Diff line change
@@ -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));

Expand Down Expand Up @@ -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]);
}