Skip to content

Commit 0514248

Browse files
slavapestovslavapestov
authored
Merge pull request #28596 from slavapestov/fun-with-guard-let
Fix various problems with top-level 'guard' statements and captures
2 parents 06014e6 + 8214e18 commit 0514248

20 files changed

+219
-169
lines changed

‎include/swift/AST/Decl.h

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -399,7 +399,7 @@ class alignas(1 << DeclAlignInBits) Decl {
399399
HasSingleExpressionBody : 1
400400
);
401401

402-
SWIFT_INLINE_BITFIELD(FuncDecl, AbstractFunctionDecl, 1+1+2+1+1+2,
402+
SWIFT_INLINE_BITFIELD(FuncDecl, AbstractFunctionDecl, 1+1+2+1+1+2+1,
403403
/// Whether we've computed the 'static' flag yet.
404404
IsStaticComputed : 1,
405405

@@ -416,7 +416,12 @@ class alignas(1 << DeclAlignInBits) Decl {
416416
SelfAccessComputed : 1,
417417

418418
/// Backing bits for 'self' access kind.
419-
SelfAccess : 2
419+
SelfAccess : 2,
420+
421+
/// Whether this is a top-level function which should be treated
422+
/// as if it were in local context for the purposes of capture
423+
/// analysis.
424+
HasTopLevelLocalContextCaptures : 1
420425
);
421426

422427
SWIFT_INLINE_BITFIELD(AccessorDecl, FuncDecl, 4+1+1,
@@ -2708,6 +2713,10 @@ class ValueDecl : public Decl {
27082713
AccessSemantics getAccessSemanticsFromContext(const DeclContext *DC,
27092714
bool isAccessOnSelf) const;
27102715

2716+
/// Determines if a reference to this declaration from a nested function
2717+
/// should be treated like a capture of a local value.
2718+
bool isLocalCapture() const;
2719+
27112720
/// Print a reference to the given declaration.
27122721
std::string printRef() const;
27132722

@@ -6078,6 +6087,7 @@ class FuncDecl : public AbstractFunctionDecl {
60786087
Bits.FuncDecl.SelfAccessComputed = false;
60796088
Bits.FuncDecl.IsStaticComputed = false;
60806089
Bits.FuncDecl.IsStatic = false;
6090+
Bits.FuncDecl.HasTopLevelLocalContextCaptures = false;
60816091
}
60826092

60836093
private:
@@ -6235,6 +6245,12 @@ class FuncDecl : public AbstractFunctionDecl {
62356245
/// Perform basic checking to determine whether the @IBAction or
62366246
/// @IBSegueAction attribute can be applied to this function.
62376247
bool isPotentialIBActionTarget() const;
6248+
6249+
bool hasTopLevelLocalContextCaptures() const {
6250+
return Bits.FuncDecl.HasTopLevelLocalContextCaptures;
6251+
}
6252+
6253+
void setHasTopLevelLocalContextCaptures(bool hasCaptures=true);
62386254
};
62396255

62406256
/// This represents an accessor function, such as a getter or setter.

‎include/swift/Parse/Parser.h

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -856,11 +856,19 @@ class Parser {
856856
void consumeTopLevelDecl(ParserPosition BeginParserPosition,
857857
TopLevelCodeDecl *TLCD);
858858

859+
ParserStatus parseBraceItems(SmallVectorImpl<ASTNode> &Decls,
860+
BraceItemListKind Kind,
861+
BraceItemListKind ConditionalBlockKind,
862+
bool &IsFollowingGuard);
859863
ParserStatus parseBraceItems(SmallVectorImpl<ASTNode> &Decls,
860864
BraceItemListKind Kind =
861865
BraceItemListKind::Brace,
862866
BraceItemListKind ConditionalBlockKind =
863-
BraceItemListKind::Brace);
867+
BraceItemListKind::Brace) {
868+
bool IsFollowingGuard = false;
869+
return parseBraceItems(Decls, Kind, ConditionalBlockKind,
870+
IsFollowingGuard);
871+
}
864872
ParserResult<BraceStmt> parseBraceItemList(Diag<> ID);
865873

866874
//===--------------------------------------------------------------------===//
@@ -869,7 +877,7 @@ class Parser {
869877
/// Return true if parser is at the start of a decl or decl-import.
870878
bool isStartOfDecl();
871879

872-
bool parseTopLevel();
880+
void parseTopLevel();
873881

874882
/// Flags that control the parsing of declarations.
875883
enum ParseDeclFlags {

‎include/swift/SIL/TypeLowering.h

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -946,24 +946,6 @@ class TypeConverter {
946946
CaptureInfo getLoweredLocalCaptures(SILDeclRef fn);
947947
bool hasLoweredLocalCaptures(SILDeclRef fn);
948948

949-
#ifndef NDEBUG
950-
/// If \c false, \c childDC is in a context it cannot capture variables from,
951-
/// so it is expected that Sema may not have computed its \c CaptureInfo.
952-
///
953-
/// This call exists for use in assertions; do not use it to skip capture
954-
/// processing.
955-
static bool canCaptureFromParent(DeclContext *childDC) {
956-
// This call was added because Sema leaves the captures of functions that
957-
// cannot capture anything uncomputed.
958-
// TODO: Make Sema set them to CaptureInfo::empty() instead.
959-
960-
if (childDC)
961-
if (auto decl = childDC->getAsDecl())
962-
return decl->getDeclContext()->isLocalContext();
963-
return true;
964-
}
965-
#endif
966-
967949
enum class ABIDifference : uint8_t {
968950
// Types have compatible calling conventions and representations, so can
969951
// be trivially bitcast.

‎include/swift/Subsystems.h

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -119,18 +119,14 @@ namespace swift {
119119
///
120120
/// \param PersistentState if non-null the same PersistentState object can
121121
/// be used to resume parsing or parse delayed function bodies.
122-
///
123-
/// \return true if the parser found code with side effects.
124-
bool parseIntoSourceFile(SourceFile &SF, unsigned BufferID, bool *Done,
122+
void parseIntoSourceFile(SourceFile &SF, unsigned BufferID, bool *Done,
125123
SILParserState *SIL = nullptr,
126124
PersistentParserState *PersistentState = nullptr,
127125
bool DelayBodyParsing = true);
128126

129127
/// Parse a single buffer into the given source file, until the full source
130128
/// contents are parsed.
131-
///
132-
/// \return true if the parser found code with side effects.
133-
bool parseIntoSourceFileFull(SourceFile &SF, unsigned BufferID,
129+
void parseIntoSourceFileFull(SourceFile &SF, unsigned BufferID,
134130
PersistentParserState *PersistentState = nullptr,
135131
bool DelayBodyParsing = true);
136132

‎lib/AST/CaptureInfo.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ CaptureInfo CaptureInfo::empty() {
5757

5858
bool CaptureInfo::hasLocalCaptures() const {
5959
for (auto capture : getCaptures())
60-
if (capture.getDecl()->getDeclContext()->isLocalContext())
60+
if (capture.getDecl()->isLocalCapture())
6161
return true;
6262
return false;
6363
}
@@ -71,7 +71,7 @@ getLocalCaptures(SmallVectorImpl<CapturedValue> &Result) const {
7171

7272
// Filter out global variables.
7373
for (auto capture : getCaptures()) {
74-
if (!capture.getDecl()->getDeclContext()->isLocalContext())
74+
if (!capture.getDecl()->isLocalCapture())
7575
continue;
7676

7777
Result.push_back(capture);

‎lib/AST/Decl.cpp

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2866,6 +2866,16 @@ bool ValueDecl::isImplicitlyUnwrappedOptional() const {
28662866
false);
28672867
}
28682868

2869+
bool ValueDecl::isLocalCapture() const {
2870+
auto *dc = getDeclContext();
2871+
2872+
if (auto *fd = dyn_cast<FuncDecl>(this))
2873+
if (isa<SourceFile>(dc))
2874+
return fd->hasTopLevelLocalContextCaptures();
2875+
2876+
return dc->isLocalContext();
2877+
}
2878+
28692879
ArrayRef<ValueDecl *>
28702880
ValueDecl::getSatisfiedProtocolRequirements(bool Sorted) const {
28712881
// Dig out the nominal type.
@@ -7669,6 +7679,12 @@ bool FuncDecl::isPotentialIBActionTarget() const {
76697679
!isa<AccessorDecl>(this);
76707680
}
76717681

7682+
void FuncDecl::setHasTopLevelLocalContextCaptures(bool hasCaptures) {
7683+
assert(!hasCaptures || isa<SourceFile>(getDeclContext()));
7684+
7685+
Bits.FuncDecl.HasTopLevelLocalContextCaptures = hasCaptures;
7686+
}
7687+
76727688
Type TypeBase::getSwiftNewtypeUnderlyingType() {
76737689
auto structDecl = getStructOrBoundGenericStruct();
76747690
if (!structDecl)

‎lib/Immediate/REPL.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -187,12 +187,10 @@ typeCheckREPLInput(ModuleDecl *MostRecentModule, StringRef Name,
187187
REPLInputFile.addImports(ImportsWithOptions);
188188
}
189189

190-
bool FoundAnySideEffects = false;
191190
bool Done;
192191
do {
193-
FoundAnySideEffects |=
194-
parseIntoSourceFile(REPLInputFile, BufferID, &Done, nullptr,
195-
&PersistentState);
192+
parseIntoSourceFile(REPLInputFile, BufferID, &Done, nullptr,
193+
&PersistentState);
196194
} while (!Done);
197195
performTypeChecking(REPLInputFile);
198196
return REPLModule;

‎lib/Parse/ParseDecl.cpp

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ namespace {
189189
/// decl-sil [[only in SIL mode]
190190
/// decl-sil-stage [[only in SIL mode]
191191
/// \endverbatim
192-
bool Parser::parseTopLevel() {
192+
void Parser::parseTopLevel() {
193193
SF.ASTStage = SourceFile::Parsing;
194194

195195
// Prime the lexer.
@@ -246,17 +246,6 @@ bool Parser::parseTopLevel() {
246246
consumeToken();
247247
}
248248

249-
// If this is a Main source file, determine if we found code that needs to be
250-
// executed (this is used by the repl to know whether to compile and run the
251-
// newly parsed stuff).
252-
bool FoundTopLevelCodeToExecute = false;
253-
if (allowTopLevelCode()) {
254-
for (auto V : Items) {
255-
if (isa<TopLevelCodeDecl>(V.get<Decl*>()))
256-
FoundTopLevelCodeToExecute = true;
257-
}
258-
}
259-
260249
// Add newly parsed decls to the module.
261250
for (auto Item : Items) {
262251
if (auto *D = Item.dyn_cast<Decl*>()) {
@@ -279,8 +268,6 @@ bool Parser::parseTopLevel() {
279268
SyntaxContext->addToken(Tok, LeadingTrivia, TrailingTrivia);
280269
TokReceiver->finalize();
281270
}
282-
283-
return FoundTopLevelCodeToExecute;
284271
}
285272

286273
ParserResult<AvailableAttr> Parser::parseExtendedAvailabilitySpecList(

‎lib/Parse/ParseStmt.cpp

Lines changed: 26 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,8 @@ bool Parser::isTerminatorForBraceItemListKind(BraceItemListKind Kind,
202202
ArrayRef<ASTNode> ParsedDecls) {
203203
switch (Kind) {
204204
case BraceItemListKind::Brace:
205+
case BraceItemListKind::TopLevelCode:
206+
case BraceItemListKind::TopLevelLibrary:
205207
return false;
206208
case BraceItemListKind::Case: {
207209
if (Tok.is(tok::pound_if)) {
@@ -220,26 +222,6 @@ bool Parser::isTerminatorForBraceItemListKind(BraceItemListKind Kind,
220222
}
221223
return isAtStartOfSwitchCase(*this);
222224
}
223-
case BraceItemListKind::TopLevelCode:
224-
// When parsing the top level executable code for a module, if we parsed
225-
// some executable code, then we're done. We want to process (name bind,
226-
// type check, etc) decls one at a time to make sure that there are not
227-
// forward type references, etc. There is an outer loop around the parser
228-
// that will reinvoke the parser at the top level on each statement until
229-
// EOF. In contrast, it is ok to have forward references between classes,
230-
// functions, etc.
231-
for (auto I : ParsedDecls) {
232-
if (isa<TopLevelCodeDecl>(I.get<Decl*>()))
233-
// Only bail out if the next token is at the start of a line. If we
234-
// don't, then we may accidentally allow things like "a = 1 b = 4".
235-
// FIXME: This is really dubious. This will reject some things, but
236-
// allow other things we don't want.
237-
if (Tok.isAtStartOfLine())
238-
return true;
239-
}
240-
return false;
241-
case BraceItemListKind::TopLevelLibrary:
242-
return false;
243225
case BraceItemListKind::ActiveConditionalBlock:
244226
case BraceItemListKind::InactiveConditionalBlock:
245227
return Tok.isNot(tok::pound_else) && Tok.isNot(tok::pound_endif) &&
@@ -289,7 +271,8 @@ void Parser::consumeTopLevelDecl(ParserPosition BeginParserPosition,
289271
/// expr '=' expr
290272
ParserStatus Parser::parseBraceItems(SmallVectorImpl<ASTNode> &Entries,
291273
BraceItemListKind Kind,
292-
BraceItemListKind ConditionalBlockKind) {
274+
BraceItemListKind ConditionalBlockKind,
275+
bool &IsFollowingGuard) {
293276
bool isRootCtx = SyntaxContext->isRoot();
294277
SyntaxParsingContext ItemListContext(SyntaxContext,
295278
SyntaxKind::CodeBlockItemList);
@@ -382,7 +365,8 @@ ParserStatus Parser::parseBraceItems(SmallVectorImpl<ASTNode> &Entries,
382365
[&](SmallVectorImpl<ASTNode> &Elements, bool IsActive) {
383366
parseBraceItems(Elements, Kind, IsActive
384367
? BraceItemListKind::ActiveConditionalBlock
385-
: BraceItemListKind::InactiveConditionalBlock);
368+
: BraceItemListKind::InactiveConditionalBlock,
369+
IsFollowingGuard);
386370
});
387371
if (IfConfigResult.hasCodeCompletion() && isCodeCompletionFirstPass()) {
388372
consumeDecl(BeginParserPosition, None, IsTopLevel);
@@ -419,7 +403,18 @@ ParserStatus Parser::parseBraceItems(SmallVectorImpl<ASTNode> &Entries,
419403
ParserResult<Decl> DeclResult =
420404
parseDecl(IsTopLevel ? PD_AllowTopLevel : PD_Default,
421405
IsAtStartOfLineOrPreviousHadSemi,
422-
[&](Decl *D) {TmpDecls.push_back(D);});
406+
[&](Decl *D) {
407+
TmpDecls.push_back(D);
408+
409+
// Any function after a 'guard' statement is marked as
410+
// possibly having local captures. This allows SILGen
411+
// to correctly determine its capture list, since
412+
// otherwise it would be skipped because it is not
413+
// defined inside a local context.
414+
if (IsFollowingGuard)
415+
if (auto *FD = dyn_cast<FuncDecl>(D))
416+
FD->setHasTopLevelLocalContextCaptures();
417+
});
423418
BraceItemsStatus |= DeclResult;
424419
if (DeclResult.isParseError()) {
425420
NeedParseErrorRecovery = true;
@@ -469,6 +464,14 @@ ParserStatus Parser::parseBraceItems(SmallVectorImpl<ASTNode> &Entries,
469464
Result, Result.getEndLoc());
470465
TLCD->setBody(Brace);
471466
Entries.push_back(TLCD);
467+
468+
// A top-level 'guard' statement can introduce local bindings, so we
469+
// must mark all functions following one. This makes them behave
470+
// as if they were in local context for the purposes of capture
471+
// emission in SILGen.
472+
if (auto *stmt = Result.dyn_cast<Stmt *>())
473+
if (isa<GuardStmt>(stmt))
474+
IsFollowingGuard = true;
472475
}
473476
} else if (Tok.is(tok::kw_init) && isa<ConstructorDecl>(CurDeclContext)) {
474477
SourceLoc StartLoc = Tok.getLoc();

‎lib/ParseSIL/ParseSIL.cpp

Lines changed: 17 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -112,13 +112,13 @@ void PrettyStackTraceParser::print(llvm::raw_ostream &out) const {
112112
out << '\n';
113113
}
114114

115-
static bool parseIntoSourceFileImpl(SourceFile &SF,
116-
unsigned BufferID,
117-
bool *Done,
118-
SILParserState *SIL,
119-
PersistentParserState *PersistentState,
120-
bool FullParse,
121-
bool DelayBodyParsing) {
115+
static void parseIntoSourceFileImpl(SourceFile &SF,
116+
unsigned BufferID,
117+
bool *Done,
118+
SILParserState *SIL,
119+
PersistentParserState *PersistentState,
120+
bool FullParse,
121+
bool DelayBodyParsing) {
122122
assert((!FullParse || (SF.canBeParsedInFull() && !SIL)) &&
123123
"cannot parse in full with the given parameters!");
124124

@@ -149,40 +149,36 @@ static bool parseIntoSourceFileImpl(SourceFile &SF,
149149
llvm::SaveAndRestore<bool> S(P.IsParsingInterfaceTokens,
150150
SF.hasInterfaceHash());
151151

152-
bool FoundSideEffects = false;
153152
do {
154-
bool hasSideEffects = P.parseTopLevel();
155-
FoundSideEffects = FoundSideEffects || hasSideEffects;
153+
P.parseTopLevel();
156154
*Done = P.Tok.is(tok::eof);
157155
} while (FullParse && !*Done);
158156

159157
if (STreeCreator) {
160158
auto rawNode = P.finalizeSyntaxTree();
161159
STreeCreator->acceptSyntaxRoot(rawNode, SF);
162160
}
163-
164-
return FoundSideEffects;
165161
}
166162

167-
bool swift::parseIntoSourceFile(SourceFile &SF,
163+
void swift::parseIntoSourceFile(SourceFile &SF,
168164
unsigned BufferID,
169165
bool *Done,
170166
SILParserState *SIL,
171167
PersistentParserState *PersistentState,
172168
bool DelayBodyParsing) {
173-
return parseIntoSourceFileImpl(SF, BufferID, Done, SIL,
174-
PersistentState,
175-
/*FullParse=*/SF.shouldBuildSyntaxTree(),
176-
DelayBodyParsing);
169+
parseIntoSourceFileImpl(SF, BufferID, Done, SIL,
170+
PersistentState,
171+
/*FullParse=*/SF.shouldBuildSyntaxTree(),
172+
DelayBodyParsing);
177173
}
178174

179-
bool swift::parseIntoSourceFileFull(SourceFile &SF, unsigned BufferID,
175+
void swift::parseIntoSourceFileFull(SourceFile &SF, unsigned BufferID,
180176
PersistentParserState *PersistentState,
181177
bool DelayBodyParsing) {
182178
bool Done = false;
183-
return parseIntoSourceFileImpl(SF, BufferID, &Done, /*SIL=*/nullptr,
184-
PersistentState, /*FullParse=*/true,
185-
DelayBodyParsing);
179+
parseIntoSourceFileImpl(SF, BufferID, &Done, /*SIL=*/nullptr,
180+
PersistentState, /*FullParse=*/true,
181+
DelayBodyParsing);
186182
}
187183

188184

0 commit comments

Comments
 (0)