[Clang] Implement P2280R4 Using unknown pointers and references in constant expressions #95474
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Shafik Yaghmour (shafik) ChangesP2280R4 allows the use of references in pointers of unknown origins in a constant expression context but only in specific cases that could be constant expressions. We track whether a variable is a constexpr unknown in a constant expression by setting a flag in either APValue or LValue and using this flag to prevent using unknown values in places where it is not allowed. Patch is 21.02 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/95474.diff 6 Files Affected:
diff --git a/clang/include/clang/AST/APValue.h b/clang/include/clang/AST/APValue.h
index c4206b73b1156..6352348107a64 100644
--- a/clang/include/clang/AST/APValue.h
+++ b/clang/include/clang/AST/APValue.h
@@ -249,6 +249,7 @@ class APValue {
struct NoLValuePath {};
struct UninitArray {};
struct UninitStruct {};
+ struct ConstexprUnknown {};
template <typename Impl> friend class clang::serialization::BasicReaderBase;
friend class ASTImporter;
@@ -256,6 +257,7 @@ class APValue {
private:
ValueKind Kind;
+ bool AllowConstexprUnknown = false;
struct ComplexAPSInt {
APSInt Real, Imag;
@@ -314,53 +316,69 @@ class APValue {
DataType Data;
public:
- APValue() : Kind(None) {}
- explicit APValue(APSInt I) : Kind(None) {
+ bool allowConstexprUnknown() const { return AllowConstexprUnknown; }
+
+ void setConstexprUnknown() { AllowConstexprUnknown = true; }
+
+ APValue() : Kind(None), AllowConstexprUnknown(false) {}
+ explicit APValue(APSInt I) : Kind(None), AllowConstexprUnknown(false) {
MakeInt(); setInt(std::move(I));
}
- explicit APValue(APFloat F) : Kind(None) {
+ explicit APValue(APFloat F) : Kind(None), AllowConstexprUnknown(false) {
MakeFloat(); setFloat(std::move(F));
}
- explicit APValue(APFixedPoint FX) : Kind(None) {
+ explicit APValue(APFixedPoint FX) : Kind(None), AllowConstexprUnknown(false) {
MakeFixedPoint(std::move(FX));
}
- explicit APValue(const APValue *E, unsigned N) : Kind(None) {
+ explicit APValue(const APValue *E, unsigned N)
+ : Kind(None), AllowConstexprUnknown(false) {
MakeVector(); setVector(E, N);
}
- APValue(APSInt R, APSInt I) : Kind(None) {
+ APValue(APSInt R, APSInt I) : Kind(None), AllowConstexprUnknown(false) {
MakeComplexInt(); setComplexInt(std::move(R), std::move(I));
}
- APValue(APFloat R, APFloat I) : Kind(None) {
+ APValue(APFloat R, APFloat I) : Kind(None), AllowConstexprUnknown(false) {
MakeComplexFloat(); setComplexFloat(std::move(R), std::move(I));
}
APValue(const APValue &RHS);
APValue(APValue &&RHS);
APValue(LValueBase B, const CharUnits &O, NoLValuePath N,
bool IsNullPtr = false)
- : Kind(None) {
+ : Kind(None), AllowConstexprUnknown(false) {
MakeLValue(); setLValue(B, O, N, IsNullPtr);
}
APValue(LValueBase B, const CharUnits &O, ArrayRef<LValuePathEntry> Path,
bool OnePastTheEnd, bool IsNullPtr = false)
- : Kind(None) {
+ : Kind(None), AllowConstexprUnknown(false) {
MakeLValue(); setLValue(B, O, Path, OnePastTheEnd, IsNullPtr);
}
- APValue(UninitArray, unsigned InitElts, unsigned Size) : Kind(None) {
+
+ APValue(LValueBase B, ConstexprUnknown, const CharUnits &O,
+ bool IsNullPtr = false)
+ : Kind(None), AllowConstexprUnknown(true) {
+ MakeLValue();
+ setLValue(B, O, NoLValuePath{}, IsNullPtr);
+ }
+
+ APValue(UninitArray, unsigned InitElts, unsigned Size)
+ : Kind(None), AllowConstexprUnknown(false) {
MakeArray(InitElts, Size);
}
- APValue(UninitStruct, unsigned B, unsigned M) : Kind(None) {
+ APValue(UninitStruct, unsigned B, unsigned M)
+ : Kind(None), AllowConstexprUnknown(false) {
MakeStruct(B, M);
}
explicit APValue(const FieldDecl *D, const APValue &V = APValue())
- : Kind(None) {
+ : Kind(None), AllowConstexprUnknown(false) {
MakeUnion(); setUnion(D, V);
}
APValue(const ValueDecl *Member, bool IsDerivedMember,
- ArrayRef<const CXXRecordDecl*> Path) : Kind(None) {
+ ArrayRef<const CXXRecordDecl *> Path)
+ : Kind(None), AllowConstexprUnknown(false) {
MakeMemberPointer(Member, IsDerivedMember, Path);
}
- APValue(const AddrLabelExpr* LHSExpr, const AddrLabelExpr* RHSExpr)
- : Kind(None) {
+ APValue(const AddrLabelExpr *LHSExpr, const AddrLabelExpr *RHSExpr)
+ : Kind(None), AllowConstexprUnknown(false) {
MakeAddrLabelDiff(); setAddrLabelDiff(LHSExpr, RHSExpr);
}
static APValue IndeterminateValue() {
diff --git a/clang/lib/AST/APValue.cpp b/clang/lib/AST/APValue.cpp
index d8e33ff421c06..f7841cb6556de 100644
--- a/clang/lib/AST/APValue.cpp
+++ b/clang/lib/AST/APValue.cpp
@@ -308,7 +308,8 @@ APValue::UnionData::~UnionData () {
delete Value;
}
-APValue::APValue(const APValue &RHS) : Kind(None) {
+APValue::APValue(const APValue &RHS)
+ : Kind(None), AllowConstexprUnknown(RHS.AllowConstexprUnknown) {
switch (RHS.getKind()) {
case None:
case Indeterminate:
@@ -379,13 +380,17 @@ APValue::APValue(const APValue &RHS) : Kind(None) {
}
}
-APValue::APValue(APValue &&RHS) : Kind(RHS.Kind), Data(RHS.Data) {
+APValue::APValue(APValue &&RHS)
+ : Kind(RHS.Kind), AllowConstexprUnknown(RHS.AllowConstexprUnknown),
+ Data(RHS.Data) {
RHS.Kind = None;
}
APValue &APValue::operator=(const APValue &RHS) {
if (this != &RHS)
*this = APValue(RHS);
+
+ AllowConstexprUnknown = RHS.AllowConstexprUnknown;
return *this;
}
@@ -395,6 +400,7 @@ APValue &APValue::operator=(APValue &&RHS) {
DestroyDataAndMakeUninit();
Kind = RHS.Kind;
Data = RHS.Data;
+ AllowConstexprUnknown = RHS.AllowConstexprUnknown;
RHS.Kind = None;
}
return *this;
@@ -426,6 +432,7 @@ void APValue::DestroyDataAndMakeUninit() {
else if (Kind == AddrLabelDiff)
((AddrLabelDiffData *)(char *)&Data)->~AddrLabelDiffData();
Kind = None;
+ AllowConstexprUnknown = false;
}
bool APValue::needsCleanup() const {
@@ -468,6 +475,7 @@ bool APValue::needsCleanup() const {
void APValue::swap(APValue &RHS) {
std::swap(Kind, RHS.Kind);
std::swap(Data, RHS.Data);
+ std::swap(AllowConstexprUnknown, RHS.AllowConstexprUnknown);
}
/// Profile the value of an APInt, excluding its bit-width.
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 7178f081d9cf3..9a427d908e4c9 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -1609,8 +1609,11 @@ namespace {
SubobjectDesignator Designator;
bool IsNullPtr : 1;
bool InvalidBase : 1;
+ // P2280R4 track if we have an unknown reference or pointer.
+ bool AllowConstexprUnknown = false;
const APValue::LValueBase getLValueBase() const { return Base; }
+ bool allowConstexprUnknown() const { return AllowConstexprUnknown; }
CharUnits &getLValueOffset() { return Offset; }
const CharUnits &getLValueOffset() const { return Offset; }
SubobjectDesignator &getLValueDesignator() { return Designator; }
@@ -1628,6 +1631,8 @@ namespace {
V = APValue(Base, Offset, Designator.Entries,
Designator.IsOnePastTheEnd, IsNullPtr);
}
+ if (AllowConstexprUnknown)
+ V.setConstexprUnknown();
}
void setFrom(ASTContext &Ctx, const APValue &V) {
assert(V.isLValue() && "Setting LValue from a non-LValue?");
@@ -1636,6 +1641,7 @@ namespace {
InvalidBase = false;
Designator = SubobjectDesignator(Ctx, V);
IsNullPtr = V.isNullPointer();
+ AllowConstexprUnknown = V.allowConstexprUnknown();
}
void set(APValue::LValueBase B, bool BInvalid = false) {
@@ -1653,6 +1659,7 @@ namespace {
InvalidBase = BInvalid;
Designator = SubobjectDesignator(getType(B));
IsNullPtr = false;
+ AllowConstexprUnknown = false;
}
void setNull(ASTContext &Ctx, QualType PointerTy) {
@@ -1662,6 +1669,7 @@ namespace {
InvalidBase = false;
Designator = SubobjectDesignator(PointerTy->getPointeeType());
IsNullPtr = true;
+ AllowConstexprUnknown = false;
}
void setInvalid(APValue::LValueBase B, unsigned I = 0) {
@@ -3300,6 +3308,11 @@ static bool HandleLValueComplexElement(EvalInfo &Info, const Expr *E,
static bool evaluateVarDeclInit(EvalInfo &Info, const Expr *E,
const VarDecl *VD, CallStackFrame *Frame,
unsigned Version, APValue *&Result) {
+ // P2280R4 If we have a reference type and we are in C++23 allow unknown
+ // references and pointers.
+ bool AllowConstexprUnknown =
+ Info.getLangOpts().CPlusPlus23 && VD->getType()->isReferenceType();
+
APValue::LValueBase Base(VD, Frame ? Frame->Index : 0, Version);
// If this is a local variable, dig out its value.
@@ -3334,7 +3347,9 @@ static bool evaluateVarDeclInit(EvalInfo &Info, const Expr *E,
return true;
}
- if (isa<ParmVarDecl>(VD)) {
+ // P2280R4 struck the restriction that variable of referene type lifetime
+ // should begin within the evaluation of E
+ if (isa<ParmVarDecl>(VD) && !AllowConstexprUnknown) {
// Assume parameters of a potential constant expression are usable in
// constant expressions.
if (!Info.checkingPotentialConstantExpression() ||
@@ -3358,7 +3373,9 @@ static bool evaluateVarDeclInit(EvalInfo &Info, const Expr *E,
// FIXME: We should eventually check whether the variable has a reachable
// initializing declaration.
const Expr *Init = VD->getAnyInitializer(VD);
- if (!Init) {
+ // P2280R4 struck the restriction that variable of referene type should have
+ // a preceding initialization.
+ if (!Init && !AllowConstexprUnknown) {
// Don't diagnose during potential constant expression checking; an
// initializer might be added later.
if (!Info.checkingPotentialConstantExpression()) {
@@ -3369,7 +3386,9 @@ static bool evaluateVarDeclInit(EvalInfo &Info, const Expr *E,
return false;
}
- if (Init->isValueDependent()) {
+ // P2280R4 struck the initialization requirement for variables of reference
+ // type so we can no longer assume we have an Init.
+ if (Init && Init->isValueDependent()) {
// The DeclRefExpr is not value-dependent, but the variable it refers to
// has a value-dependent initializer. This should only happen in
// constant-folding cases, where the variable is not actually of a suitable
@@ -3388,7 +3407,9 @@ static bool evaluateVarDeclInit(EvalInfo &Info, const Expr *E,
// Check that we can fold the initializer. In C++, we will have already done
// this in the cases where it matters for conformance.
- if (!VD->evaluateValue()) {
+ // P2280R4 struck the initialization requirement for variables of reference
+ // type so we can no longer assume we have an Init.
+ if (Init && !VD->evaluateValue()) {
Info.FFDiag(E, diag::note_constexpr_var_init_non_constant, 1) << VD;
NoteLValueLocation(Info, Base);
return false;
@@ -3420,6 +3441,15 @@ static bool evaluateVarDeclInit(EvalInfo &Info, const Expr *E,
}
Result = VD->getEvaluatedValue();
+
+ // P2280R4 If we don't have a value because this is a reference that was not
+ // initialized or whose lifetime began within E then create a value with as
+ // a ConstexprUnknown status.
+ if (AllowConstexprUnknown) {
+ if (!Result) {
+ Result = new APValue(Base, APValue::ConstexprUnknown{}, CharUnits::One());
+ }
+ }
return true;
}
@@ -3700,6 +3730,11 @@ findSubobject(EvalInfo &Info, const Expr *E, const CompleteObject &Obj,
const FieldDecl *LastField = nullptr;
const FieldDecl *VolatileField = nullptr;
+ // P2280R4 If we have an unknown referene or pointer and we don't have a
+ // value then bail out.
+ if (O->allowConstexprUnknown() && !O->hasValue())
+ return false;
+
// Walk the designator's path to find the subobject.
for (unsigned I = 0, N = Sub.Entries.size(); /**/; ++I) {
// Reading an indeterminate value is undefined, but assigning over one is OK.
@@ -5732,6 +5767,12 @@ struct CheckDynamicTypeHandler {
/// dynamic type.
static bool checkDynamicType(EvalInfo &Info, const Expr *E, const LValue &This,
AccessKinds AK, bool Polymorphic) {
+ // P2280R4 We are not allowed to invoke a virtual function whose dynamic type
+ // us constexpr-unknown, so stop early and let this fail later on if we
+ // attempt to do so.
+ if (This.allowConstexprUnknown())
+ return true;
+
if (This.Designator.Invalid)
return false;
@@ -5804,7 +5845,13 @@ static std::optional<DynamicType> ComputeDynamicType(EvalInfo &Info,
// If we don't have an lvalue denoting an object of class type, there is no
// meaningful dynamic type. (We consider objects of non-class type to have no
// dynamic type.)
- if (!checkDynamicType(Info, E, This, AK, true))
+ if (!checkDynamicType(Info, E, This, AK,
+ (AK == AK_TypeId
+ ? (E->getType()->isReferenceType() ? true : false)
+ : true)))
+ return std::nullopt;
+
+ if (This.Designator.Invalid)
return std::nullopt;
// Refuse to compute a dynamic type in the presence of virtual bases. This
@@ -8539,7 +8586,8 @@ static bool HandleLambdaCapture(EvalInfo &Info, const Expr *E, LValue &Result,
const ParmVarDecl *Self = MD->getParamDecl(0);
if (Self->getType()->isReferenceType()) {
APValue *RefValue = Info.getParamSlot(Info.CurrentCall->Arguments, Self);
- Result.setFrom(Info.Ctx, *RefValue);
+ if (!RefValue->allowConstexprUnknown() || RefValue->hasValue())
+ Result.setFrom(Info.Ctx, *RefValue);
} else {
const ParmVarDecl *VD = Info.CurrentCall->Arguments.getOrigParam(Self);
CallStackFrame *Frame =
@@ -8595,7 +8643,10 @@ bool LValueExprEvaluator::VisitDeclRefExpr(const DeclRefExpr *E) {
bool LValueExprEvaluator::VisitVarDecl(const Expr *E, const VarDecl *VD) {
-
+ // P2280R4 if we are in C++23 track if we have an unknown reference or
+ // pointer.
+ bool AllowConstexprUnknown =
+ Info.getLangOpts().CPlusPlus23 && VD->getType()->isReferenceType();
// If we are within a lambda's call operator, check whether the 'VD' referred
// to within 'E' actually represents a lambda-capture that maps to a
// data-member/field within the closure object, and if so, evaluate to the
@@ -8665,10 +8716,23 @@ bool LValueExprEvaluator::VisitVarDecl(const Expr *E, const VarDecl *VD) {
if (!V->hasValue()) {
// FIXME: Is it possible for V to be indeterminate here? If so, we should
// adjust the diagnostic to say that.
- if (!Info.checkingPotentialConstantExpression())
+ // P2280R4 If we are have a variable that is unknown reference or pointer
+ // it may not have a value but still be usable later on so do not diagnose.
+ if (!Info.checkingPotentialConstantExpression() && !AllowConstexprUnknown)
Info.FFDiag(E, diag::note_constexpr_use_uninit_reference);
+
+ // P2280R4 If we are have a variable that is unknown reference or pointer
+ // try to recover it from the frame and set the result accordingly.
+ if (VD->getType()->isReferenceType() && AllowConstexprUnknown) {
+ if (Frame) {
+ Result.set({VD, Frame->Index, Version});
+ return true;
+ }
+ return Success(VD);
+ }
return false;
}
+
return Success(*V, E);
}
@@ -11486,7 +11550,10 @@ class IntExprEvaluator
}
bool Success(const APValue &V, const Expr *E) {
- if (V.isLValue() || V.isAddrLabelDiff() || V.isIndeterminate()) {
+ // P2280R4 if we have an unknown reference or pointer allow further
+ // evaluation of the value.
+ if (V.isLValue() || V.isAddrLabelDiff() || V.isIndeterminate() ||
+ V.allowConstexprUnknown()) {
Result = V;
return true;
}
diff --git a/clang/test/SemaCXX/constant-expression-cxx11.cpp b/clang/test/SemaCXX/constant-expression-cxx11.cpp
index efb391ba0922d..684a6a242bf71 100644
--- a/clang/test/SemaCXX/constant-expression-cxx11.cpp
+++ b/clang/test/SemaCXX/constant-expression-cxx11.cpp
@@ -1419,9 +1419,9 @@ namespace ConvertedConstantExpr {
// useless note and instead just point to the non-constant subexpression.
enum class E {
em = m,
- en = n, // expected-error {{not a constant expression}} expected-note {{initializer of 'n' is unknown}}
+ en = n, // cxx23-note {{initializer of 'n' is not a constant expression}} expected-error {{enumerator value is not a constant expression}} cxx11_20-note {{initializer of 'n' is unknown}}
eo = (m + // expected-error {{not a constant expression}}
- n // expected-note {{initializer of 'n' is unknown}}
+ n // cxx23-note {{initializer of 'n' is not a constant expression}} cxx11_20-note {{initializer of 'n' is unknown}}
),
eq = reinterpret_cast<long>((int*)0) // expected-error {{not a constant expression}} expected-note {{reinterpret_cast}}
};
@@ -1961,7 +1961,8 @@ namespace ConstexprConstructorRecovery {
namespace Lifetime {
void f() {
- constexpr int &n = n; // expected-error {{constant expression}} expected-note {{use of reference outside its lifetime}} expected-warning {{not yet bound to a value}}
+ constexpr int &n = n; // expected-error {{constant expression}} cxx23-note {{reference to 'n' is not a constant expression}} cxx23-note {{address of non-static constexpr variable 'n' may differ}} expected-warning {{not yet bound to a value}}
+ // cxx11_20-note@-1 {{use of reference outside its lifetime is not allowed in a constant expression}}
constexpr int m = m; // expected-error {{constant expression}} expected-note {{read of object outside its lifetime}}
}
@@ -2381,15 +2382,15 @@ namespace array_size {
template<typename T> void f1(T t) {
constexpr int k = t.size();
}
- template<typename T> void f2(const T &t) { // expected-note 2{{declared here}}
- constexpr int k = t.size(); // expected-error 2{{constant}} expected-note 2{{function parameter 't' with unknown value cannot be used in a constant expression}}
+ template<typename T> void f2(const T &t) { // cxx11_20-note 2{{declared here}}
+ constexpr int k = t.size(); // cxx11_20-error 2{{constexpr variable 'k' must be initialized by a constant expression}} cxx11_20-note 2{{function parameter 't' with unknown value cannot be used in a constant expression}}
}
template<typename T> void f3(const T &t) {
constexpr int k = T::size();
}
void g(array<3> a) {
f1(a);
- f2(a); // expected-note {{instantiation of}}
+ f2(a); // cxx11_20-note {{in instantiation of function template}}
f3(a);
}
@@ -2398,8 +2399,9 @@ namespace array_size {
};
void h(array_nonstatic<3> a) {
f1(a);
- f2(a); // expected-note {{instantiation of}}
+ f2(a); // cxx11_20-note {{instantiation of}}
}
+ //static_assert(f2(array_size::array<3>{}));
}
namespace flexible_array {
diff --git a/clang/test/SemaCXX/constant-expression-cxx2a.cpp b/clang/test/SemaCXX/constant-expression-cxx2a.cpp
index e4d97dcb73562..3910886a36346 100644
--- a/clang/test/SemaCXX/constant-expression-cxx2a.cpp
+++ b/clang/test/SemaCXX/constant-expression-cxx2a.cpp
@@ -308,8 +308,7 @@ namespace TypeId {
static_assert(&B2().ti1 == &typeid(B));
static_assert(&B2().ti2 == &typeid(B2));
extern B2 extern_b2;
- // expected-note@+1 {{typeid applied to object 'extern_b2' whose dynamic type is not constant}}
- static_assert(&typeid(extern_b2) == &typeid(B2)); // expected-error {{constant expression}}
+ static_assert(&typeid(extern_b2) == &typeid(B2));
constexpr B2 b2;
constexpr const B &b1 = b2;
diff --git a/clang/test/SemaCXX/constant-expression-p2280r4.cpp b/clang/test/SemaCXX/constant-expression-p2280r4.cpp
new file mode 100644
index 0000000000000..be41eed88c494
--- /dev/null
+++ b/clang/test/SemaCXX/constant-expression-p2280r4.cpp
@@ -0,0 +1,54 @@
+// RUN: %clang_cc1 -std=c++23 -verify %s
+
+using size_t = decltype(sizeof(0));
+
+namespace std {
+struct type_info {
+ const char* name() const noexcept(true);
+};
+}
+
+template <typename T, size_t N>
+constexpr size_t array_size(T (&)[N]) {
+ return N;
+}
+
+void use_array(int const (&gold_medal_mel)[2]) {
+ constexpr auto gold = array_size(gold_medal_mel); // ok
+}
+
+constexpr auto olympic_mile() {
+ const int ledecky = 1500;
+ return []{ return ledecky; };
+}
+static_assert(olympic_mile()() == 1500); // ok
+
+struct Swim {
+ constexpr int phelps() { return 28; }
+ virtual constexpr int lochte() { return 12; }
+ int coughlin = 12;
+};
+
+constexpr int how_many(Swim& swam) {
+ Swim* p = &swam;
+ return (p + 1 - 1)->phelps();
+}
+
+void splash(Swim& swam) {
+ static_assert(swam.phelps() == 28); // ok
+ static_assert((&swam)->phelps() == 28); // ok
+ Swim* pswam = &swam; // expected-note {{declared here}}
+ static_assert(pswam->phelps() ==...
[truncated]
|
clang/lib/AST/ExprConstant.cpp
Outdated
Comment on lines
3311
to
3314
| // P2280R4 If we have a reference type and we are in C++23 allow unknown | ||
| // references and pointers. | ||
| bool AllowConstexprUnknown = | ||
| Info.getLangOpts().CPlusPlus23 && VD->getType()->isReferenceType(); |
There was a problem hiding this comment.
This is a DR per N4916 (ditto below).
Suggested change
| // P2280R4 If we have a reference type and we are in C++23 allow unknown | |
| // references and pointers. | |
| bool AllowConstexprUnknown = | |
| Info.getLangOpts().CPlusPlus23 && VD->getType()->isReferenceType(); | |
| // P2280R4 If we have a reference type allow unknown references and pointers. | |
| bool AllowConstexprUnknown = VD->getType()->isReferenceType(); |
There was a problem hiding this comment.
Additionally, I would prefer references to the c++ standard wording rather than the paper
Comment on lines
-1964
to
+1965
| constexpr int &n = n; // expected-error {{constant expression}} expected-note {{use of reference outside its lifetime}} expected-warning {{not yet bound to a value}} | ||
| constexpr int &n = n; // expected-error {{constant expression}} cxx23-note {{reference to 'n' is not a constant expression}} cxx23-note {{address of non-static constexpr variable 'n' may differ}} expected-warning {{not yet bound to a value}} | ||
| // cxx11_20-note@-1 {{use of reference outside its lifetime is not allowed in a constant expression}} |
There was a problem hiding this comment.
This looks strange. The old message looks better (see also CWG453) and the new one is possibly misleading as we perhaps shoudn't say the address of a reference variable.
Fznamznon
reviewed
Jun 14, 2024
cor3ntin
reviewed
Jun 14, 2024
There was a problem hiding this comment.
Thanks a lot Shafik!
I like the direction, it looks less complex than i thought it would be
I agree with @frederick-vs-ja that some diagnostics seem to have regressed.
Maybe a variable should not treated as constexpr-unknown if it's not within its lifetime
I'd like you to go over all the related issues we have (including the ones we closed for duplication), to make sure they work as expected. i think they make up for a good corpus of unit tests
We will need changelog / status page update
Just to point it out, there are a few unresolved core issues
https://cplusplus.github.io/CWG/issues/2740.html
https://cplusplus.github.io/CWG/issues/2633.html
clang/lib/AST/ExprConstant.cpp
Outdated
Comment on lines
3311
to
3314
| // P2280R4 If we have a reference type and we are in C++23 allow unknown | ||
| // references and pointers. | ||
| bool AllowConstexprUnknown = | ||
| Info.getLangOpts().CPlusPlus23 && VD->getType()->isReferenceType(); |
There was a problem hiding this comment.
Additionally, I would prefer references to the c++ standard wording rather than the paper
|
Thank you work on this! I really like this feature. Will this PR also provide support for the new constant interpreter? The new constant interpreter seems to already support some unknown constexpr features (FYI https://godbolt.org/z/xTYhGEfxT). It has the concept of |
I think that can be a separate PR @tbaederr |
AaronBallman
reviewed
Jul 8, 2024
clang/include/clang/AST/APValue.h
Outdated
|
|
||
| template <typename Impl> friend class clang::serialization::BasicReaderBase; | ||
| friend class ASTImporter; | ||
| friend class ASTNodeImporter; | ||
|
|
||
| private: | ||
| ValueKind Kind; | ||
| bool AllowConstexprUnknown = false; |
There was a problem hiding this comment.
Would it make sense to use a bit-field here so we can pack this bit together with the Kind?
There was a problem hiding this comment.
Initializing a bit-field inline is a C++20 extension and so I don't think I can use it.
I set AllowConstexprUnknown everyplace I thought it mattered in APValue but I obtain different results. It means I am missing someplace where APValue is being initialized. I can try again and see if I can hunt down the places I missed but it left me a bit spooked that it was not obvious.
…nstant expressions P2280R4 allows the use of references in pointers of unknown origins in a constant expression context but only in specific cases that could be constant expressions. We track whether a variable is a constexpr unknown in a constant expression by setting a flag in either APValue or LValue and using this flag to prevent using unknown values in places where it is not allowed. In `evaluateVarDeclInit` we may need to create a new `APValue` to track the unknown referene or pointer and we track that `APValue` in the `CallStackFrame`. Fixes: llvm#63139 llvm#63117
shafik
force-pushed
the
impl_unknown_pointers_and_references_in_constant_expr
branch
from
7583d32 to
69b09ea
Compare
|
Shafik contacted me offline about a libc++ test failure this PR causes. The test in question is struct memory_resource {
virtual ~memory_resource();
bool is_equal(const memory_resource &) const noexcept;
};
inline bool operator==(const memory_resource &__lhs, const memory_resource &__rhs) {
return &__lhs == &__rhs || __lhs.is_equal(__rhs);
// ^^^^^^^^^^^^^^^^ frontend built from this branch believes that this is never true
}
void __assert_fail();
struct TestResourceImp : memory_resource {
memory_resource do_is_equal_other;
virtual bool do_is_equal() noexcept {
return dynamic_cast<TestResourceImp *>(&do_is_equal_other);
}
};
int main() {
memory_resource mr1, mr2;
mr1 != mr2 ? void() : __assert_fail();
}Clang built from this branch produces the following IR for ; Function Attrs: mustprogress noinline nounwind optnone uwtable
define linkonce_odr dso_local noundef zeroext i1 @_ZeqRK15memory_resourceS1_(ptr noundef nonnull align 8 dereferenceable(8) %__lhs, ptr noundef nonnull align 8 dereferenceable(8) %__rhs) #1 comdat {
entry:
%__lhs.addr = alloca ptr, align 8
%__rhs.addr = alloca ptr, align 8
store ptr %__lhs, ptr %__lhs.addr, align 8
store ptr %__rhs, ptr %__rhs.addr, align 8
%0 = load ptr, ptr %__lhs.addr, align 8
%1 = load ptr, ptr %__rhs.addr, align 8
%call = call noundef zeroext i1 @_ZNK15memory_resource8is_equalERKS_(ptr noundef nonnull align 8 dereferenceable(8) %0, ptr noundef nonnull align 8 dereferenceable(8) %1) #4
ret i1 %call
}Whereas a recent nighly build ( ; Function Attrs: mustprogress noinline nounwind optnone uwtable
define linkonce_odr dso_local noundef zeroext i1 @_ZeqRK15memory_resourceS1_(ptr noundef nonnull align 8 dereferenceable(8) %__lhs, ptr noundef nonnull align 8 dereferenceable(8) %__rhs) #1 comdat {
entry:
%__lhs.addr = alloca ptr, align 8
%__rhs.addr = alloca ptr, align 8
store ptr %__lhs, ptr %__lhs.addr, align 8
store ptr %__rhs, ptr %__rhs.addr, align 8
%0 = load ptr, ptr %__lhs.addr, align 8
%1 = load ptr, ptr %__rhs.addr, align 8
%cmp = icmp eq ptr %0, %1
br i1 %cmp, label %lor.end, label %lor.rhs
lor.rhs: ; preds = %entry
%2 = load ptr, ptr %__lhs.addr, align 8
%3 = load ptr, ptr %__rhs.addr, align 8
%call = call noundef zeroext i1 @_ZNK15memory_resource8is_equalERKS_(ptr noundef nonnull align 8 dereferenceable(8) %2, ptr noundef nonnull align 8 dereferenceable(8) %3) #4
br label %lor.end
lor.end: ; preds = %lor.rhs, %entry
%4 = phi i1 [ true, %entry ], [ %call, %lor.rhs ]
ret i1 %4
} |
… and pointers - Fix handling for comparing unknown pointers - Add more tests
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
…es and pointers it did not have to handle this case before.
cor3ntin
reviewed
Jul 30, 2024
updating comments to refer to the standard when possible
tbaederr
reviewed
Sep 4, 2024
|
Just a |
|
Fixed in 28c819c |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/56/builds/16819 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/60/builds/17619 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/153/builds/20475 Here is the relevant piece of the build log for the reference |
This was referenced Jan 23, 2025
github-actions bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Jan 26, 2025
…set when ceating APValue (#124478) When implmenting P2280R4 here: llvm/llvm-project#95474 When creating the APValue to store and constexprUnknown value I used an offset of CharUnits::One() but it should have been CharUnits::Zero(). This change just adjusts that value.
|
clang started issuing a warning after this PR was merged when compiling the following code: Is the warning valid? Is the change in clang's behavior intentional? |
What do you have |
|
Is passing |
|
Looks like a bug to me. This warning presumably ought to be checking whether it's valid to read / write to the given location using something like |
|
Hey, I'm seeing a compiler crash after this PR. https://pastila.nl/?000c2db4/648c000755344e7c43e24c25dd06a571#dyq1EG+tE1OVC+RrCFJoNw== Here is the commit I had to make to get it building again. |
|
I am seeing a runtime error after this, when using C++23, which looks like a regression. Testcase: // A.hpp
#include <iostream>
struct MyStruct {
double m0{-9999.0};
int m1{-12345};
};
constexpr MyStruct default_val;
auto compute_area(double l, const MyStruct &val = default_val) -> double {
std::cout << "compute_area : val.m0 : " << val.m0 << "\n";
std::cout << "compute_area : val.m1 : " << val.m1 << " \n";
if (val.m1 != -12345) {
return val.m0 * l;
}
return -9999;
}
// main.cpp
//#include "A.hpp"
#include <cassert>
#include <iostream>
auto main() -> int {
MyStruct in_val{.m0 = 2.0, .m1 = 1};
std::cout << "main : in_val.m0 : " << in_val.m0 << "\n";
std::cout << "main : in_val.m1 : " << in_val.m1 << " \n\n";
double val = compute_area(1.0, in_val);
std::cout << "result = " << val << std::endl;
assert(val == 2.0);
return 0;
}Before this landed, it runs ok: After this landed, the assert fails: (original report: emscripten-core/emscripten#24325) |
|
|
Should And this function should return -1 regardless of what the standard is because |
We evaluate the operand of
If that mode doesn't already permit using unknown pointers and references in all language modes, then we should change it so that it does. |
But there isn't much we can do with unknown pointers or references, is there? We can't have |
|
@kripken somewhat reduced: struct MyStruct {
double m0{-9999.0};
int m1{-12345};
};
constexpr MyStruct default_val;
auto compute_area(double l, const MyStruct &val = default_val) -> double {
if (val.m1 == 1)
return 2.0;
return 0;
}
#include <cassert>
auto main() -> int {
MyStruct in_val{.m0 = 2.0, .m1 = 1};
double val = compute_area(1.0, in_val);
assert(val == 2.0);
} |
It depends on the mode parameter in |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
P2280R4 allows the use of references in pointers of unknown origins in a constant expression context but only in specific cases that could be constant expressions.
We track whether a variable is a constexpr unknown in a constant expression by setting a flag in either APValue or LValue and using this flag to prevent using unknown values in places where it is not allowed.
Fixes: #63139 #63117