Skip to content

Commit 761595e

Browse files
authored
Merge pull request #25831 from slavapestov/fix-forward-capture-analysis
Fix forward capture analysis
2 parents 200eb30 + 44142cd commit 761595e

File tree

15 files changed

+340
-370
lines changed

15 files changed

+340
-370
lines changed

include/swift/AST/AnyFunctionRef.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,12 @@ class AnyFunctionRef {
116116
return TheFunction.dyn_cast<AbstractClosureExpr*>();
117117
}
118118

119+
bool isDeferBody() const {
120+
if (auto *fd = dyn_cast_or_null<FuncDecl>(getAbstractFunctionDecl()))
121+
return fd->isDeferBody();
122+
return false;
123+
}
124+
119125
/// Return true if this closure is passed as an argument to a function and is
120126
/// known not to escape from that function. In this case, captures can be
121127
/// more efficient.

include/swift/AST/CaptureInfo.h

Lines changed: 14 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#define SWIFT_AST_CAPTURE_INFO_H
1515

1616
#include "swift/Basic/LLVM.h"
17+
#include "swift/Basic/SourceLoc.h"
1718
#include "swift/AST/TypeAlignments.h"
1819
#include "llvm/ADT/ArrayRef.h"
1920
#include "llvm/ADT/PointerIntPair.h"
@@ -44,8 +45,9 @@ class CapturedValue {
4445

4546
private:
4647
Storage Value;
48+
SourceLoc Loc;
4749

48-
explicit CapturedValue(Storage V) : Value(V) {}
50+
explicit CapturedValue(Storage V, SourceLoc Loc) : Value(V), Loc(Loc) {}
4951

5052
public:
5153
friend struct llvm::DenseMapInfo<CapturedValue>;
@@ -61,12 +63,14 @@ class CapturedValue {
6163
IsNoEscape = 1 << 1
6264
};
6365

64-
CapturedValue(llvm::PointerUnion<ValueDecl*, OpaqueValueExpr*> Ptr,
65-
unsigned Flags)
66-
: Value(Ptr, Flags) {}
66+
CapturedValue(ValueDecl *Val, unsigned Flags, SourceLoc Loc)
67+
: Value(Val, Flags), Loc(Loc) {}
68+
69+
CapturedValue(OpaqueValueExpr *Val, unsigned Flags)
70+
: Value(Val, Flags), Loc(SourceLoc()) {}
6771

6872
static CapturedValue getDynamicSelfMetadata() {
69-
return CapturedValue((ValueDecl *)nullptr, 0);
73+
return CapturedValue((ValueDecl *)nullptr, 0, SourceLoc());
7074
}
7175

7276
bool isDirect() const { return Value.getInt() & IsDirect; }
@@ -80,7 +84,9 @@ class CapturedValue {
8084
CapturedValue mergeFlags(CapturedValue cv) {
8185
assert(Value.getPointer() == cv.Value.getPointer() &&
8286
"merging flags on two different value decls");
83-
return CapturedValue(Value.getPointer(), getFlags() & cv.getFlags());
87+
return CapturedValue(
88+
Storage(Value.getPointer(), getFlags() & cv.getFlags()),
89+
Loc);
8490
}
8591

8692
ValueDecl *getDecl() const {
@@ -95,49 +101,13 @@ class CapturedValue {
95101
return Value.getPointer().dyn_cast<OpaqueValueExpr *>();
96102
}
97103

98-
unsigned getFlags() const { return Value.getInt(); }
99-
100-
bool operator==(CapturedValue RHS) const {
101-
return Value == RHS.Value;
102-
}
103-
104-
bool operator!=(CapturedValue RHS) const {
105-
return Value != RHS.Value;
106-
}
104+
SourceLoc getLoc() const { return Loc; }
107105

108-
bool operator<(CapturedValue RHS) const {
109-
return Value < RHS.Value;
110-
}
106+
unsigned getFlags() const { return Value.getInt(); }
111107
};
112108

113109
} // end swift namespace
114110

115-
namespace llvm {
116-
117-
template <> struct DenseMapInfo<swift::CapturedValue> {
118-
using CapturedValue = swift::CapturedValue;
119-
120-
using PtrIntPairDenseMapInfo = DenseMapInfo<CapturedValue::Storage>;
121-
122-
static inline swift::CapturedValue getEmptyKey() {
123-
return CapturedValue{PtrIntPairDenseMapInfo::getEmptyKey()};
124-
}
125-
126-
static inline CapturedValue getTombstoneKey() {
127-
return CapturedValue{PtrIntPairDenseMapInfo::getTombstoneKey()};
128-
}
129-
130-
static unsigned getHashValue(const CapturedValue &Val) {
131-
return PtrIntPairDenseMapInfo::getHashValue(Val.Value);
132-
}
133-
134-
static bool isEqual(const CapturedValue &LHS, const CapturedValue &RHS) {
135-
return PtrIntPairDenseMapInfo::isEqual(LHS.Value, RHS.Value);
136-
}
137-
};
138-
139-
} // end llvm namespace
140-
141111
namespace swift {
142112

143113
class DynamicSelfType;

include/swift/AST/DiagnosticsSIL.def

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,14 @@ ERROR(unsupported_c_function_pointer_conversion,none,
116116
ERROR(objc_selector_malformed,none,"the type ObjectiveC.Selector is malformed",
117117
())
118118

119+
// Capture before declaration diagnostics.
120+
ERROR(capture_before_declaration,none,
121+
"closure captures %0 before it is declared", (Identifier))
122+
ERROR(capture_before_declaration_defer,none,
123+
"'defer' block captures %0 before it is declared", (Identifier))
124+
NOTE(captured_value_declared_here,none,
125+
"captured value declared here", ())
126+
119127
// Invalid escaping capture diagnostics.
120128
ERROR(escaping_inout_capture,none,
121129
"escaping closure captures 'inout' parameter %0", (Identifier))

include/swift/AST/DiagnosticsSema.def

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3016,14 +3016,6 @@ ERROR(method_call_in_closure_without_explicit_self,none,
30163016
ERROR(implicit_use_of_self_in_closure,none,
30173017
"implicit use of 'self' in closure; use 'self.' to make"
30183018
" capture semantics explicit", ())
3019-
ERROR(capture_before_declaration,none,
3020-
"cannot capture %0 before it is declared", (Identifier))
3021-
ERROR(transitive_capture_before_declaration,none,
3022-
"cannot capture %0, which would use %1 before it is declared",
3023-
(Identifier, Identifier))
3024-
NOTE(transitive_capture_through_here,none,
3025-
"%0, declared here, captures %1",
3026-
(Identifier, Identifier))
30273019

30283020
WARNING(recursive_accessor_reference,none,
30293021
"attempting to %select{access|modify}1 %0 within its own "

include/swift/SIL/TypeLowering.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -568,8 +568,6 @@ struct SILConstantInfo {
568568

569569
/// Different ways in which a function can capture context.
570570
enum class CaptureKind {
571-
/// No context arguments are necessary.
572-
None,
573571
/// A local value captured as a mutable box.
574572
Box,
575573
/// A local value captured as a single pointer to storage (formed with

lib/Parse/ParseStmt.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -966,7 +966,7 @@ ParserResult<Stmt> Parser::parseStmtDefer() {
966966
StaticSpellingKind::None,
967967
/*FuncLoc=*/ SourceLoc(),
968968
name,
969-
/*NameLoc=*/ SourceLoc(),
969+
/*NameLoc=*/ PreviousLoc,
970970
/*Throws=*/ false, /*ThrowsLoc=*/ SourceLoc(),
971971
/*generic params*/ nullptr,
972972
params,

lib/SIL/SILFunctionType.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -773,8 +773,6 @@ lowerCaptureContextParameters(SILModule &M, AnyFunctionRef function,
773773
expansion);
774774
auto loweredTy = loweredTL.getLoweredType();
775775
switch (Types.getDeclCaptureKind(capture, expansion)) {
776-
case CaptureKind::None:
777-
break;
778776
case CaptureKind::Constant: {
779777
// Constants are captured by value.
780778
ParameterConvention convention;

lib/SIL/TypeLowering.cpp

Lines changed: 29 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -91,44 +91,36 @@ static bool hasSingletonMetatype(CanType instanceType) {
9191
CaptureKind TypeConverter::getDeclCaptureKind(CapturedValue capture,
9292
ResilienceExpansion expansion) {
9393
auto decl = capture.getDecl();
94-
if (auto *var = dyn_cast<VarDecl>(decl)) {
95-
assert(var->hasStorage() &&
96-
"should not have attempted to directly capture this variable");
97-
98-
// If this is a non-address-only stored 'let' constant, we can capture it
99-
// by value. If it is address-only, then we can't load it, so capture it
100-
// by its address (like a var) instead.
101-
if (var->isImmutable() &&
102-
(!SILModuleConventions(M).useLoweredAddresses() ||
103-
!getTypeLowering(var->getType(), expansion).isAddressOnly()))
104-
return CaptureKind::Constant;
105-
106-
// In-out parameters are captured by address.
107-
if (var->isInOut()) {
108-
return CaptureKind::StorageAddress;
109-
}
110-
111-
// Reference storage types can appear in a capture list, which means
112-
// we might allocate boxes to store the captures. However, those boxes
113-
// have the same lifetime as the closure itself, so we must capture
114-
// the box itself and not the payload, even if the closure is noescape,
115-
// otherwise they will be destroyed when the closure is formed.
116-
if (var->getType()->is<ReferenceStorageType>()) {
117-
return CaptureKind::Box;
118-
}
119-
120-
// If we're capturing into a non-escaping closure, we can generally just
121-
// capture the address of the value as no-escape.
122-
return capture.isNoEscape() ?
123-
CaptureKind::StorageAddress : CaptureKind::Box;
94+
auto *var = cast<VarDecl>(decl);
95+
assert(var->hasStorage() &&
96+
"should not have attempted to directly capture this variable");
97+
98+
// If this is a non-address-only stored 'let' constant, we can capture it
99+
// by value. If it is address-only, then we can't load it, so capture it
100+
// by its address (like a var) instead.
101+
if (var->isImmutable() &&
102+
(!SILModuleConventions(M).useLoweredAddresses() ||
103+
!getTypeLowering(var->getType(), expansion).isAddressOnly()))
104+
return CaptureKind::Constant;
105+
106+
// In-out parameters are captured by address.
107+
if (var->isInOut()) {
108+
return CaptureKind::StorageAddress;
124109
}
125-
126-
// "Captured" local types require no context.
127-
if (isa<TypeAliasDecl>(decl) || isa<GenericTypeParamDecl>(decl) ||
128-
isa<AssociatedTypeDecl>(decl))
129-
return CaptureKind::None;
130-
131-
llvm_unreachable("function-like captures should have been lowered away");
110+
111+
// Reference storage types can appear in a capture list, which means
112+
// we might allocate boxes to store the captures. However, those boxes
113+
// have the same lifetime as the closure itself, so we must capture
114+
// the box itself and not the payload, even if the closure is noescape,
115+
// otherwise they will be destroyed when the closure is formed.
116+
if (var->getType()->is<ReferenceStorageType>()) {
117+
return CaptureKind::Box;
118+
}
119+
120+
// If we're capturing into a non-escaping closure, we can generally just
121+
// capture the address of the value as no-escape.
122+
return capture.isNoEscape() ?
123+
CaptureKind::StorageAddress : CaptureKind::Box;
132124
}
133125

134126
using RecursiveProperties = TypeLowering::RecursiveProperties;

0 commit comments

Comments
 (0)