llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Timm Baeder (tbaederr) <details> <summary>Changes</summary> When revisiting `const T& range = f(T());`, we create do mark the variable for `range` as constexpr-unknown, but not the temporary variable we create for `T()`. Change that. Then we also need to ignore constexpr-unknown pointers in `CheckInvoke()`. --- Full diff: https://github.com/llvm/llvm-project/pull/187918.diff 5 Files Affected: - (modified) clang/lib/AST/ByteCode/Compiler.cpp (+23-22) - (modified) clang/lib/AST/ByteCode/Compiler.h (+6-8) - (modified) clang/lib/AST/ByteCode/Interp.cpp (+3-3) - (modified) clang/test/AST/ByteCode/codegen-constexpr-unknown.cpp (+1-4) - (modified) clang/test/AST/ByteCode/cxx26.cpp (+11) ``````````diff diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp index a881d77a73cbd..bc8b7921295f9 100644 --- a/clang/lib/AST/ByteCode/Compiler.cpp +++ b/clang/lib/AST/ByteCode/Compiler.cpp @@ -4956,15 +4956,14 @@ template <class Emitter> unsigned Compiler<Emitter>::allocateLocalPrimitive(DeclTy &&Src, PrimType Ty, bool IsConst, bool IsVolatile, - ScopeKind SC, - bool IsConstexprUnknown) { + ScopeKind SC) { // FIXME: There are cases where Src.is<Expr*>() is wrong, e.g. // (int){12} in C. Consider using Expr::isTemporaryObject() instead // or isa<MaterializeTemporaryExpr>(). Descriptor *D = P.createDescriptor(Src, Ty, nullptr, Descriptor::InlineDescMD, IsConst, isa<const Expr *>(Src), /*IsMutable=*/false, IsVolatile); - D->IsConstexprUnknown = IsConstexprUnknown; + D->IsConstexprUnknown = this->VariablesAreConstexprUnknown; Scope::Local Local = this->createLocal(D); if (auto *VD = dyn_cast_if_present<ValueDecl>(Src.dyn_cast<const Decl *>())) Locals.insert({VD, Local}); @@ -4974,8 +4973,7 @@ unsigned Compiler<Emitter>::allocateLocalPrimitive(DeclTy &&Src, PrimType Ty, template <class Emitter> UnsignedOrNone Compiler<Emitter>::allocateLocal(DeclTy &&Src, QualType Ty, - ScopeKind SC, - bool IsConstexprUnknown) { + ScopeKind SC) { const ValueDecl *Key = nullptr; const Expr *Init = nullptr; bool IsTemporary = false; @@ -4997,7 +4995,7 @@ UnsignedOrNone Compiler<Emitter>::allocateLocal(DeclTy &&Src, QualType Ty, Init); if (!D) return std::nullopt; - D->IsConstexprUnknown = IsConstexprUnknown; + D->IsConstexprUnknown = this->VariablesAreConstexprUnknown; Scope::Local Local = this->createLocal(D); if (Key) @@ -5100,11 +5098,9 @@ bool Compiler<Emitter>::visitExpr(const Expr *E, bool DestroyToplevelScope) { } template <class Emitter> -VarCreationState Compiler<Emitter>::visitDecl(const VarDecl *VD, - bool IsConstexprUnknown) { +VarCreationState Compiler<Emitter>::visitDecl(const VarDecl *VD) { - auto R = this->visitVarDecl(VD, VD->getInit(), /*Toplevel=*/true, - IsConstexprUnknown); + auto R = this->visitVarDecl(VD, VD->getInit(), /*Toplevel=*/true); if (R.notCreated()) return R; @@ -5188,9 +5184,9 @@ bool Compiler<Emitter>::visitDeclAndReturn(const VarDecl *VD, const Expr *Init, } template <class Emitter> -VarCreationState -Compiler<Emitter>::visitVarDecl(const VarDecl *VD, const Expr *Init, - bool Toplevel, bool IsConstexprUnknown) { +VarCreationState Compiler<Emitter>::visitVarDecl(const VarDecl *VD, + const Expr *Init, + bool Toplevel) { // We don't know what to do with these, so just return false. if (VD->getType().isNull()) return false; @@ -5254,8 +5250,7 @@ Compiler<Emitter>::visitVarDecl(const VarDecl *VD, const Expr *Init, if (VarT) { unsigned Offset = this->allocateLocalPrimitive( VD, *VarT, VD->getType().isConstQualified(), - VD->getType().isVolatileQualified(), ScopeKind::Block, - IsConstexprUnknown); + VD->getType().isVolatileQualified(), ScopeKind::Block); if (!Init) return true; @@ -5273,8 +5268,8 @@ Compiler<Emitter>::visitVarDecl(const VarDecl *VD, const Expr *Init, return this->emitSetLocal(*VarT, Offset, VD); } // Local composite variables. - if (UnsignedOrNone Offset = this->allocateLocal( - VD, VD->getType(), ScopeKind::Block, IsConstexprUnknown)) { + if (UnsignedOrNone Offset = + this->allocateLocal(VD, VD->getType(), ScopeKind::Block)) { if (!Init) return true; @@ -7475,10 +7470,13 @@ bool Compiler<Emitter>::visitDeclRef(const ValueDecl *D, const Expr *E) { } // In case we need to re-visit a declaration. - auto revisit = [&](const VarDecl *VD) -> bool { + auto revisit = [&](const VarDecl *VD, + bool IsConstexprUnknown = true) -> bool { + llvm::SaveAndRestore CURS(this->VariablesAreConstexprUnknown, + IsConstexprUnknown); if (!this->emitPushCC(VD->hasConstantInitialization(), E)) return false; - auto VarState = this->visitDecl(VD, /*IsConstexprUnknown=*/true); + auto VarState = this->visitDecl(VD); if (!this->emitPopCC(E)) return false; @@ -7526,7 +7524,7 @@ bool Compiler<Emitter>::visitDeclRef(const ValueDecl *D, const Expr *E) { if (!Ctx.getLangOpts().CPlusPlus) { if (VD->getAnyInitializer() && DeclType.isConstant(Ctx.getASTContext()) && !VD->isWeak()) - return revisit(VD); + return revisit(VD, DeclType->isPointerType()); return this->emitDummyPtr(D, E); } @@ -7560,11 +7558,14 @@ bool Compiler<Emitter>::visitDeclRef(const ValueDecl *D, const Expr *E) { if (VD->isLocalVarDecl() && typeShouldBeVisited(DeclType) && VD->getInit() && !VD->getInit()->isValueDependent()) { if (VD->evaluateValue()) { + bool IsConstexprUnknown = !DeclType.isConstant(Ctx.getASTContext()) && + !DeclType->isReferenceType(); // Revisit the variable declaration, but make sure it's associated with a // different evaluation, so e.g. mutable reads don't work on it. EvalIDScope _(Ctx); - return revisit(VD); - } + return revisit(VD, IsConstexprUnknown); + } else if (Ctx.getLangOpts().CPlusPlus26 && IsReference) + return revisit(VD, true); if (IsReference) return this->emitInvalidDeclRef(cast<DeclRefExpr>(E), diff --git a/clang/lib/AST/ByteCode/Compiler.h b/clang/lib/AST/ByteCode/Compiler.h index 717928dc1fbbd..aa601337a9383 100644 --- a/clang/lib/AST/ByteCode/Compiler.h +++ b/clang/lib/AST/ByteCode/Compiler.h @@ -307,10 +307,8 @@ class Compiler : public ConstStmtVisitor<Compiler<Emitter>, bool>, bool delegate(const Expr *E); /// Creates and initializes a variable from the given decl. VarCreationState visitVarDecl(const VarDecl *VD, const Expr *Init, - bool Toplevel = false, - bool IsConstexprUnknown = false); - VarCreationState visitDecl(const VarDecl *VD, - bool IsConstexprUnknown = false); + bool Toplevel = false); + VarCreationState visitDecl(const VarDecl *VD); /// Visit an APValue. bool visitAPValue(const APValue &Val, PrimType ValType, const Expr *E); bool visitAPValueInitializer(const APValue &Val, const Expr *E, QualType T); @@ -330,13 +328,11 @@ class Compiler : public ConstStmtVisitor<Compiler<Emitter>, bool>, /// Creates a local primitive value. unsigned allocateLocalPrimitive(DeclTy &&Decl, PrimType Ty, bool IsConst, bool IsVolatile = false, - ScopeKind SC = ScopeKind::Block, - bool IsConstexprUnknown = false); + ScopeKind SC = ScopeKind::Block); /// Allocates a space storing a local given its type. UnsignedOrNone allocateLocal(DeclTy &&Decl, QualType Ty = QualType(), - ScopeKind = ScopeKind::Block, - bool IsConstexprUnknown = false); + ScopeKind = ScopeKind::Block); UnsignedOrNone allocateTemporary(const Expr *E); private: @@ -454,6 +450,8 @@ class Compiler : public ConstStmtVisitor<Compiler<Emitter>, bool>, bool InStmtExpr = false; bool ToLValue = false; + bool VariablesAreConstexprUnknown = false; + /// Flag inidicating if we're initializing an already created /// variable. This is set in visitInitializer(). bool Initializing = false; diff --git a/clang/lib/AST/ByteCode/Interp.cpp b/clang/lib/AST/ByteCode/Interp.cpp index 360291ef4e9da..12207c55b4589 100644 --- a/clang/lib/AST/ByteCode/Interp.cpp +++ b/clang/lib/AST/ByteCode/Interp.cpp @@ -928,9 +928,9 @@ bool CheckStore(InterpState &S, CodePtr OpPC, const Pointer &Ptr, } static bool CheckInvoke(InterpState &S, CodePtr OpPC, const Pointer &Ptr) { - if (!CheckLive(S, OpPC, Ptr, AK_MemberCall)) - return false; - if (!Ptr.isDummy()) { + if (!Ptr.isDummy() && !isConstexprUnknown(Ptr)) { + if (!CheckLive(S, OpPC, Ptr, AK_MemberCall)) + return false; if (!CheckExtern(S, OpPC, Ptr)) return false; if (!CheckRange(S, OpPC, Ptr, AK_MemberCall)) diff --git a/clang/test/AST/ByteCode/codegen-constexpr-unknown.cpp b/clang/test/AST/ByteCode/codegen-constexpr-unknown.cpp index d5312c09d0bd8..8b70f5a4251ad 100644 --- a/clang/test/AST/ByteCode/codegen-constexpr-unknown.cpp +++ b/clang/test/AST/ByteCode/codegen-constexpr-unknown.cpp @@ -1,5 +1,5 @@ // RUN: %clang_cc1 -triple x86_64-linux -emit-llvm -fcxx-exceptions -o - %s | FileCheck %s --check-prefix=CHECK -// RUN: %clang_cc1 -triple x86_64-linux -emit-llvm -fcxx-exceptions -o - %s -fexperimental-new-constant-interpreter -DINTERP | FileCheck %s --check-prefix=CHECK,INTERP +// RUN: %clang_cc1 -triple x86_64-linux -emit-llvm -fcxx-exceptions -o - %s -fexperimental-new-constant-interpreter -DINTERP | FileCheck %s --check-prefix=CHECK /// CodeGenFunction::ConstantFoldsToSimpleInteger() for the if condition /// needs to succeed and return true. @@ -7,8 +7,6 @@ /// variable to the topmost scope, otherwise we will pick the call scope /// of to_address and de-allocate the local variable at the end of the /// to_address call. -/// FIXME: This is not currently correct since we still mark p as -/// constexpr-unknown and then reject it when comparing. extern void abort2(); constexpr const int* to_address(const int *a) { return a; @@ -23,7 +21,6 @@ void rightscope() { // CHECK-NEXT: entry: // CHECK-NEXT: %p = alloca i32 // CHECK-NEXT: store i32 0, ptr %p -// INTERP-NEXT: call noundef ptr @_Z10to_addressPKi(ptr noundef %p) /// In the if expression below, the read from s.i should fail. diff --git a/clang/test/AST/ByteCode/cxx26.cpp b/clang/test/AST/ByteCode/cxx26.cpp index 8035a2640ff5f..d4349d84996cc 100644 --- a/clang/test/AST/ByteCode/cxx26.cpp +++ b/clang/test/AST/ByteCode/cxx26.cpp @@ -62,3 +62,14 @@ namespace ExplicitThisInBacktrace { static_assert(test()); // both-error {{not an integral constant expression}} \ // both-note {{in call to}} } + +namespace ConstexprUnknownNestedVariables { + struct T { constexpr int a() const { return 42; } }; + constexpr const T& f(const T& t) noexcept { return t; } + constexpr int f() { + const T& range = f(T()); + return [&] consteval { return range.a(); }(); + } + + static_assert(f() == 42); +} `````````` </details> https://github.com/llvm/llvm-project/pull/187918 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
