llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Timm Baeder (tbaederr) <details> <summary>Changes</summary> 1) Global variables as well as dummies can not be marked constexpr-unknown. There is a subtlety here with global variables: we can't register it as constexpr-unknown and later figure out that it actually _isn't_. 2) Add a `GetRefGlobal` op similar to the existing `GetRefLocal`. 3) Reject constexpr-unknown values in `CmpHelperEQ<Pointer>` 4) Diagnose constexpr-unknown values in `GetTypeidPtr` --- Patch is 24.66 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/196334.diff 9 Files Affected: - (modified) clang/lib/AST/ByteCode/Compiler.cpp (+10-6) - (modified) clang/lib/AST/ByteCode/Compiler.h (+1-1) - (modified) clang/lib/AST/ByteCode/Interp.cpp (+18-4) - (modified) clang/lib/AST/ByteCode/Interp.h (+44-7) - (modified) clang/lib/AST/ByteCode/Opcodes.td (+1) - (modified) clang/lib/AST/ByteCode/Pointer.cpp (+7-10) - (modified) clang/lib/AST/ByteCode/Program.cpp (+11-5) - (modified) clang/lib/AST/ByteCode/Program.h (+4-2) - (modified) clang/test/SemaCXX/constant-expression-p2280r4.cpp (+26-38) ``````````diff diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp index 11c5478a5c748..f9d13b42ac0bd 100644 --- a/clang/lib/AST/ByteCode/Compiler.cpp +++ b/clang/lib/AST/ByteCode/Compiler.cpp @@ -5270,7 +5270,8 @@ VarCreationState Compiler<Emitter>::visitVarDecl(const VarDecl *VD, return checkDecl(); // The previous attempt at initialization might've been unsuccessful, // so let's try this one. - } else if ((GlobalIndex = P.createGlobal(VD, Init))) { + } else if ((GlobalIndex = + P.createGlobal(VD, Init, VariablesAreConstexprUnknown))) { } else { return false; } @@ -7617,7 +7618,9 @@ bool Compiler<Emitter>::visitDeclRef(const ValueDecl *D, const Expr *E) { if (IsReference) { if (!Ctx.getLangOpts().CPlusPlus11) return this->emitGetGlobal(classifyPrim(E), *GlobalIndex, E); - return this->emitGetGlobalUnchecked(classifyPrim(E), *GlobalIndex, E); + if (!Ctx.getLangOpts().CPlusPlus23) + return this->emitGetGlobalUnchecked(classifyPrim(E), *GlobalIndex, E); + return this->emitGetRefGlobal(*GlobalIndex, E); } return this->emitGetPtrGlobal(*GlobalIndex, E); @@ -7702,7 +7705,7 @@ bool Compiler<Emitter>::visitDeclRef(const ValueDecl *D, const Expr *E) { true); return this->visitDeclRef(D, E); } - return revisit(VD); + return revisit(VD, !VD->isConstexpr() && DeclType->isReferenceType()); } // FIXME: The evaluateValue() check here is a little ridiculous, since @@ -7726,7 +7729,8 @@ bool Compiler<Emitter>::visitDeclRef(const ValueDecl *D, const Expr *E) { /*InitializerFailed=*/true, E); } - return this->emitDummyPtr(D, E); + return this->emitDummyPtr( + D, E, Ctx.getLangOpts().CPlusPlus23 && DeclType->isReferenceType()); } template <class Emitter> @@ -8034,9 +8038,9 @@ bool Compiler<Emitter>::emitDestructionPop(const Descriptor *Desc, /// Create a dummy pointer for the given decl (or expr) and /// push a pointer to it on the stack. template <class Emitter> -bool Compiler<Emitter>::emitDummyPtr(const DeclTy &D, const Expr *E) { +bool Compiler<Emitter>::emitDummyPtr(const DeclTy &D, const Expr *E, bool CU) { assert(!DiscardResult && "Should've been checked before"); - unsigned DummyID = P.getOrCreateDummy(D); + unsigned DummyID = P.getOrCreateDummy(D, CU); if (!this->emitGetPtrGlobal(DummyID, E)) return false; diff --git a/clang/lib/AST/ByteCode/Compiler.h b/clang/lib/AST/ByteCode/Compiler.h index de6ea524897a0..98eba30415632 100644 --- a/clang/lib/AST/ByteCode/Compiler.h +++ b/clang/lib/AST/ByteCode/Compiler.h @@ -417,7 +417,7 @@ class Compiler : public ConstStmtVisitor<Compiler<Emitter>, bool>, const BinaryOperator *E); bool emitRecordDestructionPop(const Record *R, SourceInfo Loc); bool emitDestructionPop(const Descriptor *Desc, SourceInfo Loc); - bool emitDummyPtr(const DeclTy &D, const Expr *E); + bool emitDummyPtr(const DeclTy &D, const Expr *E, bool CU = false); bool emitFloat(const APFloat &F, const Expr *E); unsigned collectBaseOffset(const QualType BaseType, const QualType DerivedType); diff --git a/clang/lib/AST/ByteCode/Interp.cpp b/clang/lib/AST/ByteCode/Interp.cpp index 30b54f5dab236..ad840dc4fd404 100644 --- a/clang/lib/AST/ByteCode/Interp.cpp +++ b/clang/lib/AST/ByteCode/Interp.cpp @@ -397,7 +397,8 @@ bool CheckExtern(InterpState &S, CodePtr OpPC, const Pointer &Ptr) { return false; const auto *VD = Ptr.getDeclDesc()->asValueDecl(); - diagnoseNonConstVariable(S, OpPC, VD); + if (!Ptr.isConstexprUnknown() || !S.checkingPotentialConstantExpression()) + diagnoseNonConstVariable(S, OpPC, VD); return false; } @@ -1539,9 +1540,7 @@ bool GetPtrDerivedPop(InterpState &S, CodePtr OpPC, uint32_t Off, bool NullOK, return true; } - if (!CheckSubobject(S, OpPC, Ptr, CSK_Derived)) - return false; - if (!CheckDowncast(S, OpPC, Ptr, Off)) + if (isConstexprUnknown(Ptr)) return false; if (!Ptr.getFieldDesc()->isRecord()) { @@ -1549,6 +1548,11 @@ bool GetPtrDerivedPop(InterpState &S, CodePtr OpPC, uint32_t Off, bool NullOK, return true; } + if (!CheckSubobject(S, OpPC, Ptr, CSK_Derived)) + return false; + if (!CheckDowncast(S, OpPC, Ptr, Off)) + return false; + const Record *TargetRecord = Ptr.atFieldSub(Off).getRecord(); assert(TargetRecord); @@ -2399,6 +2403,16 @@ bool GetTypeidPtr(InterpState &S, CodePtr OpPC, const Type *TypeInfoType) { if (!P.isBlockPointer()) return false; + if (P.isConstexprUnknown()) { + QualType DynamicType = P.getType(); + const Expr *E = S.Current->getExpr(OpPC); + APValue V = P.toAPValue(S.getASTContext()); + QualType TT = S.getASTContext().getLValueReferenceType(DynamicType); + S.FFDiag(E, diag::note_constexpr_polymorphic_unknown_dynamic_type) + << AK_TypeId << V.getAsString(S.getASTContext(), TT); + return false; + } + // Pick the most-derived type. CanQualType T = P.getDeclPtr().getType()->getCanonicalTypeUnqualified(); // ... unless we're currently constructing this object. diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h index 4a550fdd63bfb..d9cd7489b30f6 100644 --- a/clang/lib/AST/ByteCode/Interp.h +++ b/clang/lib/AST/ByteCode/Interp.h @@ -1277,6 +1277,15 @@ inline bool CmpHelperEQ<Pointer>(InterpState &S, CodePtr OpPC, CompareFn Fn) { } } + // p == nullptr or nullptr == p. + if (RHS.isZero() || LHS.isZero()) { + S.Stk.push<BoolT>(BoolT::from(Fn(ComparisonCategoryResult::Unordered))); + return true; + } + + assert(!LHS.isZero()); + assert(!RHS.isZero()); + if (!S.inConstantContext()) { if (isConstexprUnknown(LHS) || isConstexprUnknown(RHS)) return false; @@ -1309,27 +1318,26 @@ inline bool CmpHelperEQ<Pointer>(InterpState &S, CodePtr OpPC, CompareFn Fn) { } // Otherwise we need to do a bunch of extra checks before returning Unordered. - if (LHS.isOnePastEnd() && !RHS.isOnePastEnd() && !RHS.isZero() && - RHS.isBlockPointer() && RHS.getOffset() == 0) { + if (LHS.isOnePastEnd() && !RHS.isOnePastEnd() && RHS.isBlockPointer() && + RHS.getOffset() == 0) { const SourceInfo &Loc = S.Current->getSource(OpPC); S.FFDiag(Loc, diag::note_constexpr_pointer_comparison_past_end) << LHS.toDiagnosticString(S.getASTContext()); return false; } - if (RHS.isOnePastEnd() && !LHS.isOnePastEnd() && !LHS.isZero() && - LHS.isBlockPointer() && LHS.getOffset() == 0) { + if (RHS.isOnePastEnd() && !LHS.isOnePastEnd() && LHS.isBlockPointer() && + LHS.getOffset() == 0) { const SourceInfo &Loc = S.Current->getSource(OpPC); S.FFDiag(Loc, diag::note_constexpr_pointer_comparison_past_end) << RHS.toDiagnosticString(S.getASTContext()); return false; } - bool BothNonNull = !LHS.isZero() && !RHS.isZero(); // Reject comparisons to literals. for (const auto &P : {LHS, RHS}) { if (P.isZero()) continue; - if (BothNonNull && P.pointsToLiteral()) { + if (P.pointsToLiteral()) { const Expr *E = P.getDeclDesc()->asExpr(); if (isa<StringLiteral>(E)) { const SourceInfo &Loc = S.Current->getSource(OpPC); @@ -1343,7 +1351,7 @@ inline bool CmpHelperEQ<Pointer>(InterpState &S, CodePtr OpPC, CompareFn Fn) { << P.toDiagnosticString(S.getASTContext()); return false; } - } else if (BothNonNull && P.isIntegralPointer()) { + } else if (P.isIntegralPointer()) { const SourceInfo &Loc = S.Current->getSource(OpPC); S.FFDiag(Loc, diag::note_constexpr_pointer_constant_comparison) << LHS.toDiagnosticString(S.getASTContext()) @@ -1360,6 +1368,15 @@ inline bool CmpHelperEQ<Pointer>(InterpState &S, CodePtr OpPC, CompareFn Fn) { return false; } + if (LHS.isConstexprUnknown() || RHS.isConstexprUnknown()) { + if (!S.checkingPotentialConstantExpression()) + S.FFDiag(S.Current->getSource(OpPC), + diag::note_constexpr_pointer_comparison_unspecified) + << LHS.toDiagnosticString(S.getASTContext()) + << RHS.toDiagnosticString(S.getASTContext()); + return false; + } + S.Stk.push<BoolT>(BoolT::from(Fn(ComparisonCategoryResult::Unordered))); return true; } @@ -1977,6 +1994,22 @@ inline bool GetRefLocal(InterpState &S, CodePtr OpPC, uint32_t I) { return handleReference(S, OpPC, LocalBlock); } +inline bool GetRefGlobal(InterpState &S, CodePtr OpPC, uint32_t I) { + Block *B = S.P.getGlobal(I); + + if (isConstexprUnknown(B)) { + S.Stk.push<Pointer>(B); + return true; + } + + const auto &Desc = B->getBlockDesc<GlobalInlineDescriptor>(); + if (Desc.InitState != GlobalInitState::Initialized) + return DiagnoseUninitialized(S, OpPC, B->isExtern(), B, AK_Read); + + S.Stk.push<Pointer>(B->deref<Pointer>()); + return true; +} + inline bool CheckRefInit(InterpState &S, CodePtr OpPC) { const Pointer &Ptr = S.Stk.peek<Pointer>(); return CheckRange(S, OpPC, Ptr, AK_Read); @@ -2076,6 +2109,10 @@ inline bool CheckNull(InterpState &S, CodePtr OpPC) { inline bool VirtBaseHelper(InterpState &S, CodePtr OpPC, const RecordDecl *Decl, const Pointer &Ptr) { + if (!Ptr.isBlockPointer()) + return false; + if (!Ptr.getFieldDesc()->isRecord()) + return false; Pointer Base = Ptr.stripBaseCasts(); const Record::Base *VirtBase = Base.getRecord()->getVirtualBase(Decl); S.Stk.push<Pointer>(Base.atField(VirtBase->Offset)); diff --git a/clang/lib/AST/ByteCode/Opcodes.td b/clang/lib/AST/ByteCode/Opcodes.td index 3fb25a5fa0884..9123719ec7f8e 100644 --- a/clang/lib/AST/ByteCode/Opcodes.td +++ b/clang/lib/AST/ByteCode/Opcodes.td @@ -325,6 +325,7 @@ def GetPtrLocal : OffsetOpcode { def GetRefLocal : OffsetOpcode { bit HasCustomEval = 1; } +def GetRefGlobal : OffsetOpcode; def CheckRefInit : Opcode {} diff --git a/clang/lib/AST/ByteCode/Pointer.cpp b/clang/lib/AST/ByteCode/Pointer.cpp index 851e9d5742ccb..51ef1401c3603 100644 --- a/clang/lib/AST/ByteCode/Pointer.cpp +++ b/clang/lib/AST/ByteCode/Pointer.cpp @@ -230,11 +230,6 @@ APValue Pointer::toAPValue(const ASTContext &ASTCtx) const { return ASTCtx.toCharUnitsFromBits(Layout.getFieldOffset(FieldIndex)); }; - bool UsePath = true; - if (const ValueDecl *VD = getDeclDesc()->asValueDecl(); - VD && VD->getType()->isReferenceType()) - UsePath = false; - // Build the path into the object. bool OnePastEnd = isOnePastEnd() && !isZeroSizeArray(); Pointer Ptr = *this; @@ -318,10 +313,9 @@ APValue Pointer::toAPValue(const ASTContext &ASTCtx) const { // Just invert the order of the elements. std::reverse(Path.begin(), Path.end()); - if (UsePath) - return APValue(Base, Offset, Path, OnePastEnd); - - return APValue(Base, Offset, APValue::NoLValuePath()); + auto Result = APValue(Base, Offset, Path, OnePastEnd); + Result.setConstexprUnknown(isConstexprUnknown()); + return Result; } void Pointer::print(llvm::raw_ostream &OS) const { @@ -437,7 +431,10 @@ std::string Pointer::toDiagnosticString(const ASTContext &Ctx) const { if (isIntegralPointer()) return (Twine("&(") + Twine(asIntPointer().Value + Offset) + ")").str(); - return toAPValue(Ctx).getAsString(Ctx, getType()); + QualType Ty = getType(); + if (Ty->isLValueReferenceType()) + Ty = Ty->getPointeeType(); + return toAPValue(Ctx).getAsString(Ctx, Ty); } bool Pointer::isInitialized() const { diff --git a/clang/lib/AST/ByteCode/Program.cpp b/clang/lib/AST/ByteCode/Program.cpp index d49903e36d34a..6711c61a307fa 100644 --- a/clang/lib/AST/ByteCode/Program.cpp +++ b/clang/lib/AST/ByteCode/Program.cpp @@ -122,7 +122,7 @@ UnsignedOrNone Program::getOrCreateGlobal(const ValueDecl *VD, return std::nullopt; } -unsigned Program::getOrCreateDummy(const DeclTy &D) { +unsigned Program::getOrCreateDummy(const DeclTy &D, bool IsConstexprUnknown) { assert(D); // Dedup blocks since they are immutable and pointers cannot be compared. if (auto It = DummyVariables.find(D.getOpaqueValue()); @@ -152,6 +152,8 @@ unsigned Program::getOrCreateDummy(const DeclTy &D) { if (!Desc) Desc = allocateDescriptor(D); + Desc->IsConstexprUnknown = IsConstexprUnknown; + assert(Desc); // Allocate a block for storage. @@ -168,7 +170,8 @@ unsigned Program::getOrCreateDummy(const DeclTy &D) { return I; } -UnsignedOrNone Program::createGlobal(const ValueDecl *VD, const Expr *Init) { +UnsignedOrNone Program::createGlobal(const ValueDecl *VD, const Expr *Init, + bool IsConstexprUnknown) { bool IsStatic, IsExtern; bool IsWeak = VD->isWeak(); if (const auto *Var = dyn_cast<VarDecl>(VD)) { @@ -185,8 +188,8 @@ UnsignedOrNone Program::createGlobal(const ValueDecl *VD, const Expr *Init) { // Register all previous declarations as well. For extern blocks, just replace // the index with the new variable. - UnsignedOrNone Idx = - createGlobal(VD, VD->getType(), IsStatic, IsExtern, IsWeak, Init); + UnsignedOrNone Idx = createGlobal(VD, VD->getType(), IsStatic, IsExtern, + IsWeak, IsConstexprUnknown, Init); if (!Idx) return std::nullopt; @@ -237,7 +240,8 @@ UnsignedOrNone Program::createGlobal(const Expr *E, QualType ExprType) { if (auto Idx = getGlobal(E)) return Idx; if (auto Idx = createGlobal(E, ExprType, /*IsStatic=*/true, - /*IsExtern=*/false, /*IsWeak=*/false)) { + /*IsExtern=*/false, /*IsWeak=*/false, + /*IsConstexprUnknown=*/false)) { GlobalIndices[E] = *Idx; return *Idx; } @@ -246,6 +250,7 @@ UnsignedOrNone Program::createGlobal(const Expr *E, QualType ExprType) { UnsignedOrNone Program::createGlobal(const DeclTy &D, QualType Ty, bool IsStatic, bool IsExtern, bool IsWeak, + bool IsConstexprUnknown, const Expr *Init) { // Create a descriptor for the global. Descriptor *Desc; @@ -261,6 +266,7 @@ UnsignedOrNone Program::createGlobal(const DeclTy &D, QualType Ty, if (!Desc) return std::nullopt; + Desc->IsConstexprUnknown = IsConstexprUnknown; // Allocate a block for storage. unsigned I = Globals.size(); diff --git a/clang/lib/AST/ByteCode/Program.h b/clang/lib/AST/ByteCode/Program.h index e3ec0c07736a3..c98c5c0e51a24 100644 --- a/clang/lib/AST/ByteCode/Program.h +++ b/clang/lib/AST/ByteCode/Program.h @@ -88,10 +88,11 @@ class Program final { const Expr *Init = nullptr); /// Returns or creates a dummy value for unknown declarations. - unsigned getOrCreateDummy(const DeclTy &D); + unsigned getOrCreateDummy(const DeclTy &D, bool IsConstexprUnknown = false); /// Creates a global and returns its index. - UnsignedOrNone createGlobal(const ValueDecl *VD, const Expr *Init); + UnsignedOrNone createGlobal(const ValueDecl *VD, const Expr *Init, + bool IsConstexprUnknown = false); /// Creates a global from a lifetime-extended temporary. UnsignedOrNone createGlobal(const Expr *E, QualType ExprType); @@ -169,6 +170,7 @@ class Program final { UnsignedOrNone createGlobal(const DeclTy &D, QualType Ty, bool IsStatic, bool IsExtern, bool IsWeak, + bool IsConstexprUnknown, const Expr *Init = nullptr); /// Reference to the VM context. diff --git a/clang/test/SemaCXX/constant-expression-p2280r4.cpp b/clang/test/SemaCXX/constant-expression-p2280r4.cpp index f4a8fbcbbe427..319c6c842e8f8 100644 --- a/clang/test/SemaCXX/constant-expression-p2280r4.cpp +++ b/clang/test/SemaCXX/constant-expression-p2280r4.cpp @@ -50,12 +50,11 @@ void splash(Swim& swam) { // expected-note {{declared here}} } extern Swim dc; -extern Swim& trident; // interpreter-note {{declared here}} +extern Swim& trident; constexpr auto& sandeno = typeid(dc); // ok: can only be typeid(Swim) constexpr auto& gallagher = typeid(trident); // expected-error {{constexpr variable 'gallagher' must be initialized by a constant expression}} \ - // nointerpreter-note {{typeid applied to object 'trident' whose dynamic type is not constant}} \ - // interpreter-note {{initializer of 'trident' is unknown}} + // expected-note {{typeid applied to object 'trident' whose dynamic type is not constant}} namespace explicitThis { struct C { @@ -266,9 +265,9 @@ namespace param_reference { namespace dropped_note { extern int &x; // expected-note {{declared here}} - constexpr int f() { return x; } // nointerpreter-note {{read of non-constexpr variable 'x'}} \ - // interpreter-note {{initializer of 'x' is unknown}} - constexpr int y = f(); // expected-error {{constexpr variable 'y' must be initialized by a constant expression}} expected-note {{in call to 'f()'}} + constexpr int f() { return x; } // expected-note {{read of non-constexpr variable 'x'}} + constexpr int y = f(); // expected-error {{constexpr variable 'y' must be initialized by a constant expression}} \ + // expected-note {{in call to 'f()'}} } namespace dynamic { @@ -289,10 +288,10 @@ namespace unsized_array { constexpr int t1 = a - a; constexpr int t2 = a - b; // expected-error {{constexpr variable 't2' must be initialized by a constant expression}} \ // nointerpreter-note {{arithmetic involving unrelated objects '&a[0]' and '&b[0]' has unspecified value}} \ - // interpreter-note {{arithmetic involving unrelated objects 'a' and 'b' has unspecified value}} + // interpreter-note {{arithmetic involving unrelated objects '&a' and '&b' has unspecified value}} constexpr int t3 = a - &c[2]; // expected-error {{constexpr variable 't3' must be initialized by a constant expression}} \ - // nointerpreter-note {{arithmetic involving unrelated objects '&a[0]' and '&c[2]' has unspecified value}} \ - // interpreter-note {{arithmetic involving unrelated objects 'a' and '*((char*)&c + 8)' has unspecified value}} + // nointerpreter-note {{arithmetic involving unrelated objects '&a[0]' and '&c[2]' has unspecified value}} \ + // interpreter-note {{arithmetic involving unrelated objects '&a' and '&c[2]' has unspecified value}} } } @@ -300,24 +299,19 @@ namespace casting { struct A {}; struct B : A {}; struct C : A {}; - extern A &a; // interpreter-note {{declared here}} - extern B &b; // expected-note {{declared here}} interpreter-note 2 {{declared here}} + extern A &a; + extern B &b; // nointerpreter-note {{declared here}} constexpr B &t1 = (B&)a; // expected-error {{must be initialized by a constant expression}} \ - // nointerpreter-note {{cannot cast object of dynamic type 'A' to type 'B'}} \ - // interpreter-note {{initializer of 'a' is unknown}} + // nointerpreter-note {{cannot cast object of dynamic type 'A' to type 'B'}} constexpr B &t2 = (B&)(A&)b; // expected-error {{must be initialized by a constant expression}} \ - // nointerpreter-note {{initializer of 'b' is not a constant expression}} \ - // interpreter-note {{initializer of 'b' is unknown}} - // FIXME: interpreter incorrectly rejects. - constexpr bool t3 = &b + 1 == &(B&)(A&)b; // interpreter-error {{must be initialized by a constant expression}} \ - // interpreter-note {{initializer of 'b' is unknown}} + // nointerpreter-note {{initializer of 'b' is not a constant expression}} + constexpr bool t3 = &b + 1 == &(B&)(A&)b; // interpreter-error {{constexpr variable 't3' must be initialized by a constant expression}} ... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/196334 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
