Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
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
20 changes: 18 additions & 2 deletions lib/SILGen/SILGenDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@
#include "llvm/Support/ErrorHandling.h"
#include <iterator>

#define DEBUG_TYPE "SILGenDecl"

using namespace swift;
using namespace Lowering;

Expand Down Expand Up @@ -808,6 +810,12 @@ class LetValueInitialization : public Initialization {
lowering->getLoweredType().isMoveOnly(/*orWrapped=*/false);
}

// auto parentInit = vd->getParentInitializer();
// if (isa<CallExpr>(parentInit)) {
// auto fn = cast<CallExpr>(parentInit)->getSemanticFn();
// isUninitialized = isa<AbstractClosureExpr>(fn);
// }

// Make sure that we have a non-address only type when binding a
// @_noImplicitCopy let.
if (lowering->isAddressOnly() && vd->isNoImplicitCopy()) {
Expand All @@ -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());
Expand All @@ -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);
Expand All @@ -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<DestroyLocalVariable>(
CleanupState::Dormant, vd);
Expand Down
74 changes: 74 additions & 0 deletions lib/SILGen/SILGenFunction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand All @@ -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;
}

Expand All @@ -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<SILUndef>(loc.value)) {
llvm::dbgs() << "SILUndef (UNINITIALIZED)\n";
} else if (isa<SILArgument>(loc.value)) {
llvm::dbgs() << "SILArgument\n";
} else if (isa<AllocStackInst>(loc.value)) {
llvm::dbgs() << "AllocStackInst\n";
} else if (isa<AllocBoxInst>(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<SILUndef>(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,
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}
Expand Down
21 changes: 21 additions & 0 deletions lib/SILOptimizer/Mandatory/DIMemoryUseCollector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -823,6 +823,27 @@ void ElementUseCollector::collectUses(SILValue Pointer, unsigned BaseEltNo) {
continue;
}

if (false && (isa<UnconditionalCheckedCastAddrInst>(User) ||
isa<CheckedCastAddrBranchInst>(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<MarkUnresolvedNonCopyableValueInst>(User)) {
Expand Down
72 changes: 72 additions & 0 deletions lib/Sema/TypeCheckCaptures.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<Expr *> 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<Expr *> walkToDeclRefExpr(DeclRefExpr *DRE) {
auto *D = DRE->getDecl();

Expand All @@ -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<VarDecl>(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();
Expand All @@ -321,6 +377,14 @@ class FindCapturedVars : public ASTWalker {
if (isa<TypeDecl>(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<TopLevelCodeDecl>(CurDC))
return Action::SkipNode(DRE);
Expand Down Expand Up @@ -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<VarDecl>(D) && !D->isLocalCapture())
Expand Down
31 changes: 31 additions & 0 deletions test/SILOptimizer/definite_init_address_only_let.swift
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,34 @@ func quz<T>(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
}
}
33 changes: 33 additions & 0 deletions test/Sema/diag_variable_used_in_initial.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}