Skip to content

[4.2] DI: Consider capture of lets to be a read-only use. #17111

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 11, 2018
Merged
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
25 changes: 24 additions & 1 deletion lib/SILOptimizer/Mandatory/DIMemoryUseCollectorOwnership.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -877,9 +877,32 @@ void ElementUseCollector::collectUses(SILValue Pointer, unsigned BaseEltNo) {
addElementUses(BaseEltNo, PointeeType, User, DIUseKind::IndirectIn);
continue;


// If this is an @inout parameter, it is like both a load and store.
case ParameterConvention::Indirect_Inout:
case ParameterConvention::Indirect_InoutAliasable: {
// FIXME: The @inout_aliasable convention is used for indirect captures
// of both 'let' and 'var' variables. Using a more specific convention
// for 'let' properties like @in_guaranteed unfortunately exposes bugs
// elsewhere in the pipeline. A 'let' capture cannot really be mutated
// by the callee, and this is enforced by sema, so we can consider it
// a nonmutating use.
bool isLet = true;

for (unsigned i = 0; i < TheMemory.NumElements; ++i) {
if (!TheMemory.isElementLetProperty(i)) {
isLet = false;
break;
}
}

if (isLet) {
addElementUses(BaseEltNo, PointeeType, User, DIUseKind::IndirectIn);
continue;
}

LLVM_FALLTHROUGH;
}
case ParameterConvention::Indirect_Inout: {
// If we're in the initializer for a struct, and this is a call to a
// mutating method, we model that as an escape of self. If an
// individual sub-member is passed as inout, then we model that as an
Expand Down
9 changes: 5 additions & 4 deletions lib/Sema/ConstraintSystem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -873,6 +873,7 @@ std::pair<Type, Type>
ConstraintSystem::getTypeOfReference(ValueDecl *value,
FunctionRefKind functionRefKind,
ConstraintLocatorBuilder locator,
DeclContext *useDC,
const DeclRefExpr *base) {
if (value->getDeclContext()->isTypeContext() && isa<FuncDecl>(value)) {
// Unqualified lookup can find operator names within nominal types.
Expand Down Expand Up @@ -937,7 +938,7 @@ ConstraintSystem::getTypeOfReference(ValueDecl *value,
// Unqualified reference to a type.
if (auto typeDecl = dyn_cast<TypeDecl>(value)) {
// Resolve the reference to this type declaration in our current context.
auto type = TC.resolveTypeInContext(typeDecl, nullptr, DC,
auto type = TC.resolveTypeInContext(typeDecl, nullptr, useDC,
TypeResolutionFlags::InExpression,
/*isSpecialized=*/false);

Expand All @@ -958,7 +959,7 @@ ConstraintSystem::getTypeOfReference(ValueDecl *value,

// Determine the type of the value, opening up that type if necessary.
bool wantInterfaceType = !varDecl->getDeclContext()->isLocalContext();
Type valueType = TC.getUnopenedTypeOfReference(varDecl, Type(), DC, base,
Type valueType = TC.getUnopenedTypeOfReference(varDecl, Type(), useDC, base,
wantInterfaceType);

assert(!valueType->hasUnboundGenericType() &&
Expand Down Expand Up @@ -1222,7 +1223,7 @@ ConstraintSystem::getTypeOfMemberReference(

// If the base is a module type, just use the type of the decl.
if (baseObjTy->is<ModuleType>()) {
return getTypeOfReference(value, functionRefKind, locator, base);
return getTypeOfReference(value, functionRefKind, locator, useDC, base);
}

// Don't open existentials when accessing typealias members of
Expand Down Expand Up @@ -1633,7 +1634,7 @@ void ConstraintSystem::resolveOverload(ConstraintLocator *locator,
} else {
std::tie(openedFullType, refType)
= getTypeOfReference(choice.getDecl(),
choice.getFunctionRefKind(), locator);
choice.getFunctionRefKind(), locator, useDC);
}

// For a non-subscript declaration found via dynamic lookup, strip
Expand Down
1 change: 1 addition & 0 deletions lib/Sema/ConstraintSystem.h
Original file line number Diff line number Diff line change
Expand Up @@ -2188,6 +2188,7 @@ class ConstraintSystem {
ValueDecl *decl,
FunctionRefKind functionRefKind,
ConstraintLocatorBuilder locator,
DeclContext *useDC,
const DeclRefExpr *base = nullptr);

/// \brief Retrieve the type of a reference to the given value declaration,
Expand Down
12 changes: 4 additions & 8 deletions test/SILGen/closures.swift
Original file line number Diff line number Diff line change
Expand Up @@ -380,13 +380,11 @@ func closeOverLetLValue() {
// CHECK: [[TMP_CLASS_ADDR:%.*]] = alloc_stack $ClassWithIntProperty, let, name "a", argno 1
// CHECK: [[COPY_ARG:%.*]] = copy_value [[ARG]]
// CHECK: store [[COPY_ARG]] to [init] [[TMP_CLASS_ADDR]] : $*ClassWithIntProperty
// CHECK: [[LOADED_CLASS:%.*]] = load [copy] [[TMP_CLASS_ADDR]] : $*ClassWithIntProperty
// CHECK: [[BORROWED_LOADED_CLASS:%.*]] = begin_borrow [[LOADED_CLASS]]
// CHECK: [[BORROWED_LOADED_CLASS:%.*]] = load_borrow [[TMP_CLASS_ADDR]] : $*ClassWithIntProperty
// CHECK: [[INT_IN_CLASS_ADDR:%.*]] = ref_element_addr [[BORROWED_LOADED_CLASS]] : $ClassWithIntProperty, #ClassWithIntProperty.x
// CHECK: [[ACCESS:%.*]] = begin_access [read] [dynamic] [[INT_IN_CLASS_ADDR]] : $*Int
// CHECK: [[INT_IN_CLASS:%.*]] = load [trivial] [[ACCESS]] : $*Int
// CHECK: end_borrow [[BORROWED_LOADED_CLASS]] from [[LOADED_CLASS]]
// CHECK: destroy_value [[LOADED_CLASS]]
// CHECK: end_borrow [[BORROWED_LOADED_CLASS]] from [[TMP_CLASS_ADDR]]
// CHECK: destroy_addr [[TMP_CLASS_ADDR]] : $*ClassWithIntProperty
// CHECK: dealloc_stack %1 : $*ClassWithIntProperty
// CHECK: return [[INT_IN_CLASS]]
Expand All @@ -397,10 +395,8 @@ func closeOverLetLValue() {
// GUARANTEED: [[TMP:%.*]] = alloc_stack $ClassWithIntProperty
// GUARANTEED: [[COPY:%.*]] = copy_value %0 : $ClassWithIntProperty
// GUARANTEED: store [[COPY]] to [init] [[TMP]] : $*ClassWithIntProperty
// GUARANTEED: [[LOADED_COPY:%.*]] = load [copy] [[TMP]]
// GUARANTEED: [[BORROWED:%.*]] = begin_borrow [[LOADED_COPY]]
// GUARANTEED: end_borrow [[BORROWED]] from [[LOADED_COPY]]
// GUARANTEED: destroy_value [[LOADED_COPY]]
// GUARANTEED: [[BORROWED:%.*]] = load_borrow [[TMP]]
// GUARANTEED: end_borrow [[BORROWED]] from [[TMP]]
// GUARANTEED: destroy_addr [[TMP]]
// GUARANTEED: } // end sil function '$S8closures18closeOverLetLValueyyFSiyXEfU_'

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
@@ -0,0 +1,31 @@
// RUN: %target-swift-frontend -emit-sil -verify %s

func foo<T>(a: Bool, t: T) {
let x: T
defer { print(x) }

x = t
return
}

func bar<T>(a: Bool, t: T) {
let x: T // expected-note {{defined here}}
defer { print(x) } //expected-error{{constant 'x' used before being initialized}}

if a {
x = t
return
}
}

func bas<T>(a: Bool, t: T) {
let x: T
defer { print(x) }

if a {
x = t
return
}

x = t
}
5 changes: 0 additions & 5 deletions test/SILOptimizer/definite_init_diagnostics.swift
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,6 @@ func test2() {
}
b4 = 7

let b5: Any
b5 = "x"
{ takes_inout_any(&b5) }() // expected-error {{immutable value 'b5' must not be passed inout}}
({ takes_inout_any(&b5) })() // expected-error {{immutable value 'b5' must not be passed inout}}

// Structs
var s1 : SomeStruct
s1 = SomeStruct() // ok
Expand Down
13 changes: 13 additions & 0 deletions test/expr/closure/let.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// RUN: %target-swift-frontend -typecheck -verify %s

func frob(x: inout Int) {}

func foo() {
let x: Int // expected-note* {{}}

x = 0

_ = { frob(x: x) }() // expected-error{{'x' is a 'let'}}
_ = { x = 0 }() // expected-error{{'x' is a 'let'}}
_ = { frob(x: x); x = 0 }() // expected-error 2 {{'x' is a 'let'}}
}