diff --git a/lib/SILGen/SILGenDecl.cpp b/lib/SILGen/SILGenDecl.cpp index 65332d83edbe0..fb544dac416d4 100644 --- a/lib/SILGen/SILGenDecl.cpp +++ b/lib/SILGen/SILGenDecl.cpp @@ -40,6 +40,8 @@ #include "llvm/Support/ErrorHandling.h" #include +#define DEBUG_TYPE "SILGenDecl" + using namespace swift; using namespace Lowering; @@ -808,6 +810,12 @@ class LetValueInitialization : public Initialization { lowering->getLoweredType().isMoveOnly(/*orWrapped=*/false); } +// auto parentInit = vd->getParentInitializer(); +// if (isa(parentInit)) { +// auto fn = cast(parentInit)->getSemanticFn(); +// isUninitialized = isa(fn); +// } + // Make sure that we have a non-address only type when binding a // @_noImplicitCopy let. if (lowering->isAddressOnly() && vd->isNoImplicitCopy()) { @@ -816,6 +824,10 @@ class LetValueInitialization : public Initialization { } if (needsTemporaryBuffer) { + LLVM_DEBUG({ + llvm::dbgs() << "JQ: need tmp buffer\n"; + vd->dump(llvm::dbgs()); + }); bool lexicalLifetimesEnabled = SGF.getASTContext().SILOpts.supportsLexicalLifetimes(SGF.getModule()); auto lifetime = SGF.F.getLifetime(vd, lowering->getLoweredType()); @@ -824,8 +836,12 @@ class LetValueInitialization : public Initialization { address = SGF.emitTemporaryAllocation(vd, lowering->getLoweredType(), DoesNotHaveDynamicLifetime, isLexical, IsFromVarDecl); - if (isUninitialized) + + // Ensure DI always checks this to avoid cases where an address-only + // value is referenced in a closure that is part of its initializer. + if (false || isUninitialized) address = SGF.B.createMarkUninitializedVar(vd, address); + DestroyCleanup = SGF.enterDormantTemporaryCleanup(address, *lowering); SGF.VarLocs[vd] = SILGenFunction::VarLoc(address, SILAccessEnforcement::Unknown); @@ -834,7 +850,7 @@ class LetValueInitialization : public Initialization { // inactive until the variable is initialized: if control flow exits the // before the value is bound, we don't want to destroy the value. // - // Cleanups are required for all lexically scoped variables to delimite + // Cleanups are required for all lexically scoped variables to delimit // the variable scope, even if the cleanup does nothing. SGF.Cleanups.pushCleanupInState( CleanupState::Dormant, vd); diff --git a/lib/SILGen/SILGenFunction.cpp b/lib/SILGen/SILGenFunction.cpp index e39ebbfc0b8e5..4596f2bf91bdc 100644 --- a/lib/SILGen/SILGenFunction.cpp +++ b/lib/SILGen/SILGenFunction.cpp @@ -590,6 +590,16 @@ void SILGenFunction::emitCaptures(SILLocation loc, break; } + LLVM_DEBUG({ + llvm::dbgs() << "JQ: emit captures running\n"; + for (auto capture : captureInfo.getCaptures()) { + llvm::dbgs() << " cap: '" << capture.getDecl()->getName() << "'\n"; + } + llvm::dbgs() + << " canGuarantee: '" << canGuarantee << "'\n" + << " capCanEscape: '" << captureCanEscape << "'\n"; + }); + auto expansion = getTypeExpansionContext(); for (auto capture : captureInfo.getCaptures()) { @@ -609,6 +619,7 @@ void SILGenFunction::emitCaptures(SILLocation loc, if (capture.isOpaqueValue() || capture.isPackElement()) { capturedArgs.push_back( emitRValueAsSingleValue(capture.getExpr()).ensurePlusOne(*this, loc)); + LLVM_DEBUG(llvm::dbgs() << "JQ: cap emit early ret: opaque || pack\n"); continue; } @@ -632,6 +643,52 @@ void SILGenFunction::emitCaptures(SILLocation loc, auto valueType = FunctionDC->mapTypeIntoContext( interfaceType->getReferenceStorageReferent()); + LLVM_DEBUG( + { + llvm::dbgs() << "=== DEBUG: VarLocs contents for capture of '" + << vd->getBaseIdentifier() << "' ===\n"; + llvm::dbgs() << "Total entries in VarLocs: " << VarLocs.size() << "\n"; + + for (auto &entry : VarLocs) { + auto *var = entry.first; + auto &loc = entry.second; + + llvm::dbgs() << " - Variable: " << var->getBaseIdentifier() << "\n"; + // llvm::errs() << " Type: " << var->getType() << "\n"; + llvm::dbgs() << " Value type: " << loc.value->getType() << "\n"; + llvm::dbgs() << " Value kind: "; + + if (isa(loc.value)) { + llvm::dbgs() << "SILUndef (UNINITIALIZED)\n"; + } else if (isa(loc.value)) { + llvm::dbgs() << "SILArgument\n"; + } else if (isa(loc.value)) { + llvm::dbgs() << "AllocStackInst\n"; + } else if (isa(loc.value)) { + llvm::dbgs() << "AllocBoxInst\n"; + } else { + llvm::dbgs() << "some other inst\n"; + } + + if (loc.box) { + llvm::dbgs() << " Has box: yes\n"; + } + + llvm::dbgs() << "\n"; + } + + llvm::dbgs() << "Looking for variable: " << vd->getBaseIdentifier() << "\n"; + auto found = VarLocs.find(vd); + if (found == VarLocs.end()) { + llvm::dbgs() << " Result: NOT FOUND in VarLocs\n"; + } else { + llvm::dbgs() << " Result: FOUND in VarLocs\n"; + llvm::dbgs() << " Value is undef: " + << (isa(found->second.value) ? "YES" : "NO") << "\n"; + } + llvm::dbgs() << "===================================\n\n"; + }); + // // If we haven't emitted the captured value yet, we're forming a closure // to a local function before all of its captures have been emitted. Eg, @@ -695,6 +752,12 @@ void SILGenFunction::emitCaptures(SILLocation loc, // expansion context without opaque archetype substitution. auto getAddressValue = [&](SILValue entryValue, bool forceCopy, bool forLValue) -> SILValue { + LLVM_DEBUG({ + llvm::dbgs() << "JQ: get addr value, force copy: " << forceCopy + << ", for lval: " << forLValue << "\n"; + entryValue->getDefiningInstruction()->print(llvm::dbgs()); + }); + if (!SGM.M.useLoweredAddresses() && !forLValue && !isPack) { // In opaque values mode, addresses aren't used except by lvalues. auto &lowering = getTypeLowering(entryValue->getType()); @@ -775,6 +838,11 @@ void SILGenFunction::emitCaptures(SILLocation loc, auto &Entry = found->second; auto val = Entry.value; + LLVM_DEBUG({ + auto capKind = SGM.Types.getDeclCaptureKind(capture, expansion); + llvm::dbgs() << "JQ: cap kind:: " << (unsigned)capKind << "\n"; + }); + switch (SGM.Types.getDeclCaptureKind(capture, expansion)) { case CaptureKind::Constant: { assert(!isPack); @@ -855,6 +923,12 @@ void SILGenFunction::emitCaptures(SILLocation loc, } capturedArgs.push_back(ManagedValue::forOwnedAddressRValue( addr, CleanupHandle::invalid())); + +// LLVM_DEBUG({ +// llvm::dbgs() << "JQ: adding escape to mark for: \n"; +// val->getDefiningInstruction()->print(llvm::dbgs()); +// }); +// escapesToMark.push_back(val); } break; } diff --git a/lib/SILOptimizer/Mandatory/DIMemoryUseCollector.cpp b/lib/SILOptimizer/Mandatory/DIMemoryUseCollector.cpp index 3ca1f6c06661c..ed26d0f3b57ee 100644 --- a/lib/SILOptimizer/Mandatory/DIMemoryUseCollector.cpp +++ b/lib/SILOptimizer/Mandatory/DIMemoryUseCollector.cpp @@ -823,6 +823,27 @@ void ElementUseCollector::collectUses(SILValue Pointer, unsigned BaseEltNo) { continue; } + if (false && (isa(User) || + isa(User))) { + LLVM_DEBUG({ + llvm::dbgs() << "JQ: DI found unchecked addr cast:\n" + << Op; + }); + + DIUseKind Kind; + Kind = DIUseKind::Initialization; +// if (Op->getOperandNumber() == 0) +// Kind = DIUseKind::Load; +// else +// Kind = InStructSubElement ? DIUseKind::PartialStore +// : DIUseKind::Initialization; + + // TODO: what about enum sub elt? do we need to even special case things here? + + addElementUses(BaseEltNo, PointeeType, User, Kind); + continue; + } + // Look through mark_unresolved_non_copyable_value. To us, it is not // interesting. if (auto *mmi = dyn_cast(User)) { diff --git a/lib/Sema/TypeCheckCaptures.cpp b/lib/Sema/TypeCheckCaptures.cpp index be0054a709067..8ba29616fcd76 100644 --- a/lib/Sema/TypeCheckCaptures.cpp +++ b/lib/Sema/TypeCheckCaptures.cpp @@ -289,6 +289,27 @@ class FindCapturedVars : public ASTWalker { return Action::SkipChildren(PEE); } + bool isExprContainedIn(Expr *inner, Expr *outer) { + class ContainmentChecker : public ASTWalker { + Expr *target; + public: + bool found = false; + ContainmentChecker(Expr *target) : target(target) {} + + PreWalkResult walkToExprPre(Expr *E) override { + if (E == target) { + found = true; + return Action::Stop(); + } + return Action::Continue(E); + } + }; + + ContainmentChecker checker(inner); + outer->walk(checker); + return checker.found; + } + PreWalkResult walkToDeclRefExpr(DeclRefExpr *DRE) { auto *D = DRE->getDecl(); @@ -300,6 +321,41 @@ class FindCapturedVars : public ASTWalker { if (D->getBaseName() == Context.Id_dollarInterpolation) return Action::SkipNode(DRE); + auto isCapturingFromOwnInitializer = [&]() -> bool { +// return false; + + // assume synthesized code is okay? + if (D->isImplicit() || DRE->isImplicit()) + return false; + + auto *var = dyn_cast(D); + if (!var) { + return false; + } + + auto *pbd = var->getParentPatternBinding(); + if (!pbd) { + return false; + } + + unsigned index = pbd->getPatternEntryIndexForVarDecl(var); + auto *initExpr = pbd->getInit(index); + if (!initExpr) { + return false; + } + + auto loc = DRE->getLoc(); + if (!loc.isValid()) + return false; + + auto srcRange = initExpr->getSourceRange(); + if (!srcRange.isValid()) + return false; + +// return isExprContainedIn(DRE, initExpr); + return initExpr->getSourceRange().contains(DRE->getLoc()); + }; + // DC is the DeclContext where D was defined // CurDC is the DeclContext where D was referenced auto DC = D->getDeclContext(); @@ -321,6 +377,14 @@ class FindCapturedVars : public ASTWalker { if (isa(D)) return Action::SkipNode(DRE); + if (isCapturingFromOwnInitializer()) { + Context.Diags.diagnose(DRE->getLoc(), + diag::use_local_before_declaration + ,DRE->getDecl()->createNameRef()); + Context.Diags.diagnose(DRE->getDecl()->getLoc(), diag::default_value_declared_here); + return Action::SkipNode(DRE); + } + // A local reference is not a capture. if (CurDC == DC || isa(CurDC)) return Action::SkipNode(DRE); @@ -389,6 +453,14 @@ class FindCapturedVars : public ASTWalker { if (TmpDC == nullptr) return Action::SkipNode(DRE); +// if (isCapturingFromOwnInitializer()) { +// Context.Diags.diagnose(DRE->getLoc(), +// diag::use_local_before_declaration +// ,DRE->getDecl()->createNameRef()); +// Context.Diags.diagnose(DRE->getDecl()->getLoc(), diag::default_value_declared_here); +// return Action::SkipNode(DRE); +// } + // Only capture var decls at global scope. Other things can be captured // if they are local. if (!isa(D) && !D->isLocalCapture()) diff --git a/test/SILOptimizer/definite_init_address_only_let.swift b/test/SILOptimizer/definite_init_address_only_let.swift index 3500154159ad5..8f404309f2733 100644 --- a/test/SILOptimizer/definite_init_address_only_let.swift +++ b/test/SILOptimizer/definite_init_address_only_let.swift @@ -39,3 +39,34 @@ func quz(a: Bool, t: T) { return } } + +// https://github.com/swiftlang/swift/issues/84909 + +func uninit_closure_reference() { + func passthrough(_ a: () -> Any) -> Any { a() } + + // let initMe = passthrough { initMe } + // ezxpected-error @-1 {{constant 'initMe' used before being initialized}} + // ezxpected-note @-2 {{defined here}} + + // let inline = { () -> Any in inline }() + // ezxpected-error @-1 {{constant 'inline' used before being initialized}} + // ezxpected-note @-2 {{defined here}} + + // these should not regress + func castAny(_ a: Any) { + let directUncond = a as! Int + _ = directUncond + + let directCond = a as? Int + _ = directCond + + let twoPhaseUncond: Int + twoPhaseUncond = a as! Int + _ = twoPhaseUncond + + let twoPhasCond: Int? + twoPhasCond = a as? Int + _ = twoPhasCond + } +} diff --git a/test/Sema/diag_variable_used_in_initial.swift b/test/Sema/diag_variable_used_in_initial.swift index b79c798877a13..2e7f4a20a3538 100644 --- a/test/Sema/diag_variable_used_in_initial.swift +++ b/test/Sema/diag_variable_used_in_initial.swift @@ -58,3 +58,36 @@ func localContext() { } } } + +// https://github.com/swiftlang/swift/issues/84909 + +func uninit_closure_reference() { + func passthrough(_ a: () -> Any) -> Any { a() } + + let initMe = passthrough { initMe } + // expected-error @-1 {{use of local variable 'initMe' before its declaration}} + // expected-note @-2 {{default value declared here}} + + let inline = // expected-note {{default value declared here}} + { + () -> Any in + inline // expected-error {{use of local variable 'inline' before its declaration}} + }() + + // these should not regress + func castAny(_ a: Any) { + let directUncond = a as! Int + _ = directUncond + + let directCond = a as? Int + _ = directCond + + let twoPhaseUncond: Int + twoPhaseUncond = a as! Int + _ = twoPhaseUncond + + let twoPhaseCond: Int? + twoPhaseCond = a as? Int + _ = twoPhaseCond + } +}