Skip to content

Commit 8953c09

Browse files
xedinxedin
authored
Merge pull request #40397 from xedin/reland-se-0326-related-changes-without-reenabling
[TypeChecker] Re-land fixes/improvements related to SE-0326
2 parents c850f56 + 277b0fc commit 8953c09

28 files changed

+393
-164
lines changed

‎include/swift/Sema/CSBindings.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -338,6 +338,14 @@ class BindingSet {
338338

339339
TypeVariableType *getTypeVariable() const { return Info.TypeVar; }
340340

341+
/// Check whether this binding set belongs to a type variable
342+
/// that represents a result type of a closure.
343+
bool forClosureResult() const;
344+
345+
/// Check whether this binding set belongs to a type variable
346+
/// that represents a generic parameter.
347+
bool forGenericParameter() const;
348+
341349
bool canBeNil() const;
342350

343351
/// If this type variable doesn't have any viable bindings, or

‎include/swift/Sema/CSFix.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2308,6 +2308,10 @@ class SpecifyClosureParameterType final : public ConstraintFix {
23082308

23092309
bool diagnose(const Solution &solution, bool asNote = false) const override;
23102310

2311+
bool diagnoseForAmbiguity(CommonFixesArray commonFixes) const override {
2312+
return diagnose(*commonFixes.front().first);
2313+
}
2314+
23112315
static SpecifyClosureParameterType *create(ConstraintSystem &cs,
23122316
ConstraintLocator *locator);
23132317

‎include/swift/Sema/Constraint.h

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -442,6 +442,8 @@ class Constraint final : public llvm::ilist_node<Constraint>,
442442
ASTNode Element;
443443
/// Contextual information associated with the element (if any).
444444
ContextualTypeInfo Context;
445+
/// Identifies whether result of this node is unused.
446+
bool IsDiscarded;
445447
} ClosureElement;
446448
};
447449

@@ -495,7 +497,7 @@ class Constraint final : public llvm::ilist_node<Constraint>,
495497
SmallPtrSetImpl<TypeVariableType *> &typeVars);
496498

497499
/// Construct a closure body element constraint.
498-
Constraint(ASTNode node, ContextualTypeInfo context,
500+
Constraint(ASTNode node, ContextualTypeInfo context, bool isDiscarded,
499501
ConstraintLocator *locator,
500502
SmallPtrSetImpl<TypeVariableType *> &typeVars);
501503

@@ -585,12 +587,14 @@ class Constraint final : public llvm::ilist_node<Constraint>,
585587

586588
static Constraint *createClosureBodyElement(ConstraintSystem &cs,
587589
ASTNode node,
588-
ConstraintLocator *locator);
590+
ConstraintLocator *locator,
591+
bool isDiscarded = false);
589592

590593
static Constraint *createClosureBodyElement(ConstraintSystem &cs,
591594
ASTNode node,
592595
ContextualTypeInfo context,
593-
ConstraintLocator *locator);
596+
ConstraintLocator *locator,
597+
bool isDiscarded = false);
594598

595599
/// Determine the kind of constraint.
596600
ConstraintKind getKind() const { return Kind; }
@@ -857,6 +861,11 @@ class Constraint final : public llvm::ilist_node<Constraint>,
857861
return ClosureElement.Context;
858862
}
859863

864+
bool isDiscardedElement() const {
865+
assert(Kind == ConstraintKind::ClosureBodyElement);
866+
return ClosureElement.IsDiscarded;
867+
}
868+
860869
/// For an applicable function constraint, retrieve the trailing closure
861870
/// matching rule.
862871
Optional<TrailingClosureMatching> getTrailingClosureMatching() const;

‎include/swift/Sema/ConstraintSystem.h

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4854,8 +4854,8 @@ class ConstraintSystem {
48544854
/// Simplify a closure body element constraint by generating required
48554855
/// constraints to represent the given element in constraint system.
48564856
SolutionKind simplifyClosureBodyElementConstraint(
4857-
ASTNode element, ContextualTypeInfo context, TypeMatchOptions flags,
4858-
ConstraintLocatorBuilder locator);
4857+
ASTNode element, ContextualTypeInfo context, bool isDiscarded,
4858+
TypeMatchOptions flags, ConstraintLocatorBuilder locator);
48594859

48604860
public: // FIXME: Public for use by static functions.
48614861
/// Simplify a conversion constraint with a fix applied to it.
@@ -5014,7 +5014,8 @@ class ConstraintSystem {
50145014
/// \param replaceInvalidRefsWithErrors Indicates whether it's allowed
50155015
/// to replace any discovered invalid member references with `ErrorExpr`.
50165016
static bool preCheckExpression(Expr *&expr, DeclContext *dc,
5017-
bool replaceInvalidRefsWithErrors);
5017+
bool replaceInvalidRefsWithErrors,
5018+
bool leaveClosureBodiesUnchecked);
50185019

50195020
/// Solve the system of constraints generated from provided target.
50205021
///
@@ -5245,6 +5246,16 @@ class ConstraintSystem {
52455246
/// imported from C/ObjectiveC.
52465247
bool isArgumentOfImportedDecl(ConstraintLocatorBuilder locator);
52475248

5249+
/// Check whether given closure should participate in inference e.g.
5250+
/// if it's a single-expression closure - it always does, but
5251+
/// multi-statement closures require special flags.
5252+
bool participatesInInference(ClosureExpr *closure) const;
5253+
5254+
/// Visit each subexpression that will be part of the constraint system
5255+
/// of the given expression, including those in closure bodies that will be
5256+
/// part of the constraint system.
5257+
void forEachExpr(Expr *expr, llvm::function_ref<Expr *(Expr *)> callback);
5258+
52485259
SWIFT_DEBUG_DUMP;
52495260
SWIFT_DEBUG_DUMPER(dump(Expr *));
52505261

@@ -6069,18 +6080,6 @@ BraceStmt *applyResultBuilderTransform(
60696080
constraints::SolutionApplicationTarget)>
60706081
rewriteTarget);
60716082

6072-
/// Determine whether the given closure expression should be type-checked
6073-
/// within the context of its enclosing expression. Otherwise, it will be
6074-
/// separately type-checked once its enclosing expression has determined all
6075-
/// of the parameter and result types without looking at the body.
6076-
bool shouldTypeCheckInEnclosingExpression(ClosureExpr *expr);
6077-
6078-
/// Visit each subexpression that will be part of the constraint system
6079-
/// of the given expression, including those in closure bodies that will be
6080-
/// part of the constraint system.
6081-
void forEachExprInConstraintSystem(
6082-
Expr *expr, llvm::function_ref<Expr *(Expr *)> callback);
6083-
60846083
/// Whether the given parameter requires an argument.
60856084
bool parameterRequiresArgument(
60866085
ArrayRef<AnyFunctionType::Param> params,

‎include/swift/Sema/IDETypeChecking.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,8 @@ namespace swift {
4747
struct PrintOptions;
4848

4949
/// Typecheck binding initializer at \p bindingIndex.
50-
void typeCheckPatternBinding(PatternBindingDecl *PBD, unsigned bindingIndex);
50+
void typeCheckPatternBinding(PatternBindingDecl *PBD, unsigned bindingIndex,
51+
bool leaveClosureBodiesUnchecked);
5152

5253
/// Check if T1 is convertible to T2.
5354
///

‎lib/IDE/ExprContextAnalysis.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,8 @@ void swift::ide::typeCheckContextAt(DeclContext *DC, SourceLoc Loc) {
9898
[](VarDecl *VD) { (void)VD->getInterfaceType(); });
9999
if (PBD->getInit(i)) {
100100
if (!PBD->isInitializerChecked(i))
101-
typeCheckPatternBinding(PBD, i);
101+
typeCheckPatternBinding(PBD, i,
102+
/*LeaveClosureBodyUnchecked=*/true);
102103
}
103104
}
104105
} else if (auto *defaultArg = dyn_cast<DefaultArgumentInitializer>(DC)) {

‎lib/Sema/BuilderTransform.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1932,7 +1932,9 @@ class PreCheckResultBuilderApplication : public ASTWalker {
19321932
DiagnosticTransaction transaction(diagEngine);
19331933

19341934
HasError |= ConstraintSystem::preCheckExpression(
1935-
E, DC, /*replaceInvalidRefsWithErrors=*/true);
1935+
E, DC, /*replaceInvalidRefsWithErrors=*/true,
1936+
/*leaveClosureBodiesUnchecked=*/false);
1937+
19361938
HasError |= transaction.hasErrors();
19371939

19381940
if (!HasError)

‎lib/Sema/CSApply.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4056,8 +4056,7 @@ namespace {
40564056
if (expr->isLiteralInit()) {
40574057
auto *literalInit = expr->getSubExpr();
40584058
if (auto *call = dyn_cast<CallExpr>(literalInit)) {
4059-
forEachExprInConstraintSystem(call->getFn(),
4060-
[&](Expr *subExpr) -> Expr * {
4059+
cs.forEachExpr(call->getFn(), [&](Expr *subExpr) -> Expr * {
40614060
auto *TE = dyn_cast<TypeExpr>(subExpr);
40624061
if (!TE)
40634062
return subExpr;

‎lib/Sema/CSBindings.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,14 @@ using namespace swift;
2525
using namespace constraints;
2626
using namespace inference;
2727

28+
bool BindingSet::forClosureResult() const {
29+
return Info.TypeVar->getImpl().isClosureResultType();
30+
}
31+
32+
bool BindingSet::forGenericParameter() const {
33+
return bool(Info.TypeVar->getImpl().getGenericParameter());
34+
}
35+
2836
bool BindingSet::canBeNil() const {
2937
auto &ctx = CS.getASTContext();
3038
return Literals.count(

‎lib/Sema/CSClosure.cpp

Lines changed: 55 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
//
1717
//===----------------------------------------------------------------------===//
1818

19+
#include "MiscDiagnostics.h"
1920
#include "TypeChecker.h"
2021
#include "swift/Sema/ConstraintSystem.h"
2122

@@ -162,8 +163,8 @@ static bool isViableElement(ASTNode element) {
162163
return true;
163164
}
164165

165-
using ElementInfo =
166-
std::tuple<ASTNode, ContextualTypeInfo, ConstraintLocator *>;
166+
using ElementInfo = std::tuple<ASTNode, ContextualTypeInfo,
167+
/*isDiscarded=*/bool, ConstraintLocator *>;
167168

168169
static void createConjunction(ConstraintSystem &cs,
169170
ArrayRef<ElementInfo> elements,
@@ -190,7 +191,8 @@ static void createConjunction(ConstraintSystem &cs,
190191
for (const auto &entry : elements) {
191192
ASTNode element = std::get<0>(entry);
192193
ContextualTypeInfo context = std::get<1>(entry);
193-
ConstraintLocator *elementLoc = std::get<2>(entry);
194+
bool isDiscarded = std::get<2>(entry);
195+
ConstraintLocator *elementLoc = std::get<3>(entry);
194196

195197
if (!isViableElement(element))
196198
continue;
@@ -201,8 +203,8 @@ static void createConjunction(ConstraintSystem &cs,
201203
if (isIsolated)
202204
element.walk(paramCollector);
203205

204-
constraints.push_back(
205-
Constraint::createClosureBodyElement(cs, element, context, elementLoc));
206+
constraints.push_back(Constraint::createClosureBodyElement(
207+
cs, element, context, elementLoc, isDiscarded));
206208
}
207209

208210
// It's possible that there are no viable elements in the body,
@@ -220,8 +222,9 @@ static void createConjunction(ConstraintSystem &cs,
220222
}
221223

222224
ElementInfo makeElement(ASTNode node, ConstraintLocator *locator,
223-
ContextualTypeInfo context = ContextualTypeInfo()) {
224-
return std::make_tuple(node, context, locator);
225+
ContextualTypeInfo context = ContextualTypeInfo(),
226+
bool isDiscarded = false) {
227+
return std::make_tuple(node, context, isDiscarded, locator);
225228
}
226229

227230
static ProtocolDecl *getSequenceProtocol(ASTContext &ctx, SourceLoc loc,
@@ -751,6 +754,8 @@ class ClosureConstraintGenerator
751754

752755
void visitBraceStmt(BraceStmt *braceStmt) {
753756
if (isSupportedMultiStatementClosure()) {
757+
auto &ctx = cs.getASTContext();
758+
754759
if (isChildOf(StmtKind::Case)) {
755760
auto *caseStmt = cast<CaseStmt>(
756761
locator->castLastElementTo<LocatorPathElt::ClosureBodyElement>()
@@ -765,10 +770,15 @@ class ClosureConstraintGenerator
765770

766771
SmallVector<ElementInfo, 4> elements;
767772
for (auto element : braceStmt->getElements()) {
773+
bool isDiscarded =
774+
element.is<Expr *>() &&
775+
(!ctx.LangOpts.Playground && !ctx.LangOpts.DebuggerSupport);
776+
768777
elements.push_back(makeElement(
769778
element,
770779
cs.getConstraintLocator(
771-
locator, LocatorPathElt::ClosureBodyElement(element))));
780+
locator, LocatorPathElt::ClosureBodyElement(element)),
781+
/*contextualInfo=*/{}, isDiscarded));
772782
}
773783

774784
createConjunction(cs, elements, locator);
@@ -845,7 +855,7 @@ class ClosureConstraintGenerator
845855

846856
bool isSupportedMultiStatementClosure() const {
847857
return !closure->hasSingleExpressionBody() &&
848-
shouldTypeCheckInEnclosingExpression(closure);
858+
cs.participatesInInference(closure);
849859
}
850860

851861
#define UNSUPPORTED_STMT(STMT) void visit##STMT##Stmt(STMT##Stmt *) { \
@@ -876,7 +886,7 @@ class ClosureConstraintGenerator
876886
bool ConstraintSystem::generateConstraints(ClosureExpr *closure) {
877887
auto &ctx = closure->getASTContext();
878888

879-
if (shouldTypeCheckInEnclosingExpression(closure)) {
889+
if (participatesInInference(closure)) {
880890
ClosureConstraintGenerator generator(*this, closure,
881891
getConstraintLocator(closure));
882892
generator.visit(closure->getBody());
@@ -925,28 +935,22 @@ bool isConditionOfStmt(ConstraintLocatorBuilder locator) {
925935

926936
ConstraintSystem::SolutionKind
927937
ConstraintSystem::simplifyClosureBodyElementConstraint(
928-
ASTNode element, ContextualTypeInfo context, TypeMatchOptions flags,
929-
ConstraintLocatorBuilder locator) {
938+
ASTNode element, ContextualTypeInfo context, bool isDiscarded,
939+
TypeMatchOptions flags, ConstraintLocatorBuilder locator) {
930940
auto *closure = castToExpr<ClosureExpr>(locator.getAnchor());
931941

932942
ClosureConstraintGenerator generator(*this, closure,
933943
getConstraintLocator(locator));
934944

935945
if (auto *expr = element.dyn_cast<Expr *>()) {
936-
if (context.purpose != CTP_Unused) {
937-
SolutionApplicationTarget target(expr, closure, context.purpose,
938-
context.getType(),
939-
/*isDiscarded=*/false);
940-
941-
if (generateConstraints(target, FreeTypeVariableBinding::Disallow))
942-
return SolutionKind::Error;
943-
944-
setSolutionApplicationTarget(expr, target);
945-
return SolutionKind::Solved;
946-
}
946+
SolutionApplicationTarget target(expr, closure, context.purpose,
947+
context.getType(), isDiscarded);
947948

948-
if (!generateConstraints(expr, closure, /*isInputExpression=*/false))
949+
if (generateConstraints(target, FreeTypeVariableBinding::Disallow))
949950
return SolutionKind::Error;
951+
952+
setSolutionApplicationTarget(expr, target);
953+
return SolutionKind::Solved;
950954
} else if (auto *stmt = element.dyn_cast<Stmt *>()) {
951955
generator.visit(stmt);
952956
} else if (auto *cond = element.dyn_cast<StmtCondition *>()) {
@@ -1160,12 +1164,17 @@ class ClosureConstraintApplication
11601164

11611165
auto forEachTarget =
11621166
rewriteTarget(*cs.getSolutionApplicationTarget(forEachStmt));
1167+
11631168
if (!forEachTarget)
11641169
hadError = true;
11651170

11661171
auto body = visit(forEachStmt->getBody()).get<Stmt *>();
11671172
forEachStmt->setBody(cast<BraceStmt>(body));
11681173

1174+
// Check to see if the sequence expr is throwing (in async context),
1175+
// if so require the stmt to have a `try`.
1176+
hadError |= diagnoseUnhandledThrowsInAsyncContext(closure, forEachStmt);
1177+
11691178
return forEachStmt;
11701179
}
11711180

@@ -1241,13 +1250,32 @@ class ClosureConstraintApplication
12411250
}
12421251

12431252
ASTNode visitBraceStmt(BraceStmt *braceStmt) {
1253+
auto &cs = solution.getConstraintSystem();
1254+
1255+
// Diagnose defer statement being last one in block.
1256+
if (!braceStmt->empty()) {
1257+
if (auto stmt = braceStmt->getLastElement().dyn_cast<Stmt *>()) {
1258+
if (auto deferStmt = dyn_cast<DeferStmt>(stmt)) {
1259+
auto &diags = closure->getASTContext().Diags;
1260+
diags
1261+
.diagnose(deferStmt->getStartLoc(), diag::defer_stmt_at_block_end)
1262+
.fixItReplace(deferStmt->getStartLoc(), "do");
1263+
}
1264+
}
1265+
}
1266+
12441267
for (auto &node : braceStmt->getElements()) {
12451268
if (auto expr = node.dyn_cast<Expr *>()) {
12461269
// Rewrite the expression.
1247-
if (auto rewrittenExpr = rewriteExpr(expr))
1248-
node = rewrittenExpr;
1249-
else
1270+
auto target = *cs.getSolutionApplicationTarget(expr);
1271+
if (auto rewrittenTarget = rewriteTarget(target)) {
1272+
node = rewrittenTarget->getAsExpr();
1273+
1274+
if (target.isDiscardedExpr())
1275+
TypeChecker::checkIgnoredExpr(castToExpr(node));
1276+
} else {
12501277
hadError = true;
1278+
}
12511279
} else if (auto stmt = node.dyn_cast<Stmt *>()) {
12521280
node = visit(stmt);
12531281
} else {

‎lib/Sema/CSDiagnostics.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -466,7 +466,8 @@ bool MissingConformanceFailure::diagnoseAsError() {
466466
}
467467

468468
bool hasFix = false;
469-
forEachExprInConstraintSystem(caseExpr, [&](Expr *expr) -> Expr * {
469+
auto &cs = getConstraintSystem();
470+
cs.forEachExpr(caseExpr, [&](Expr *expr) -> Expr * {
470471
hasFix |= anchors.count(expr);
471472
return hasFix ? nullptr : expr;
472473
});
@@ -3594,7 +3595,8 @@ bool MissingMemberFailure::diagnoseAsError() {
35943595

35953596
bool hasUnresolvedPattern = false;
35963597
if (auto *E = getAsExpr(anchor)) {
3597-
forEachExprInConstraintSystem(const_cast<Expr *>(E), [&](Expr *expr) {
3598+
auto &cs = getConstraintSystem();
3599+
cs.forEachExpr(const_cast<Expr *>(E), [&](Expr *expr) {
35983600
hasUnresolvedPattern |= isa<UnresolvedPatternExpr>(expr);
35993601
return hasUnresolvedPattern ? nullptr : expr;
36003602
});

0 commit comments

Comments
 (0)