Skip to content

Commit 7fc5023

Browse files
committed
[CS] Preserve fatal scoring information across conjunction elements
We currently solve conjunction elements starting from a zero score, but that means we can end up dropping `SK_Fix` and `SK_Hole` scoring information, resulting in attempting to run CSApply on invalid solutions. Make sure we preserve these score kinds both across conjunction elements as well as propagating into non-isolated conjunctions.
1 parent 1315953 commit 7fc5023

File tree

4 files changed

+23
-9
lines changed

4 files changed

+23
-9
lines changed

lib/Sema/CSStep.cpp

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -803,21 +803,32 @@ bool ConjunctionStep::attempt(const ConjunctionElement &element) {
803803
// subsequent elements.
804804
CS.solverState->BestScore.reset();
805805

806+
// Make sure that element is solved in isolation by dropping scoring
807+
// information. We do still need to preserve fatal score kinds below though.
808+
CS.clearScore();
809+
806810
// Apply solution inferred for all the previous elements
807811
// because this element could reference declarations
808812
// established in previous element(s).
813+
Score PreviousScore;
809814
if (!Solutions.empty()) {
810815
assert(Solutions.size() == 1);
811816
// Note that solution is removed here. This is done
812817
// because we want build a single complete solution
813818
// incrementally.
814-
CS.replaySolution(Solutions.pop_back_val(),
815-
/*shouldIncrementScore=*/false);
819+
auto S = Solutions.pop_back_val();
820+
CS.replaySolution(S, /*shouldIncrementScore=*/false);
821+
PreviousScore = S.getFixedScore();
822+
} else if (!Conjunction->isIsolated()) {
823+
// If this isn't an isolated conjunction, we need to preserve outer fatal
824+
// scoring information.
825+
PreviousScore = OuterScore;
816826
}
817827

818-
// Make sure that element is solved in isolation
819-
// by dropping all scoring information.
820-
CS.clearScore();
828+
// Preserve fatal scoring information to ensure they don't get dropped from
829+
// the resulting solution.
830+
for (auto kind : {SK_Fix, SK_Hole})
831+
CS.increaseScore(kind, PreviousScore.Data[kind]);
821832

822833
// Reset the scope and trail counters to avoid "too complex"
823834
// failures when closure has a lot of elements in the body.

lib/Sema/CSStep.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -858,6 +858,9 @@ class ConjunctionStep : public BindingStep<ConjunctionElementProducer> {
858858
void replaySolution(const Solution &solution);
859859
};
860860

861+
/// The score of the outer scope.
862+
Score OuterScore;
863+
861864
/// Best solution solver reached so far.
862865
std::optional<Score> BestScore;
863866

@@ -901,7 +904,7 @@ class ConjunctionStep : public BindingStep<ConjunctionElementProducer> {
901904
SmallVectorImpl<Solution> &solutions)
902905
: BindingStep(cs, {cs, conjunction},
903906
conjunction->isIsolated() ? IsolatedSolutions : solutions),
904-
BestScore(getBestScore()),
907+
OuterScore(cs.CurrentScore), BestScore(getBestScore()),
905908
OuterNumSolverScopes(cs.NumSolverScopes, 0),
906909
OuterNumTrailSteps(cs.NumTrailSteps, 0),
907910
Conjunction(conjunction),

test/IDE/complete_in_closures.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -543,8 +543,8 @@ func testCompleteAfterClosureWithIfExprThatContainErrors() {
543543
}#^AFTER_CLOSURE_WITH_IF_EXPR_THAT_CONTAIN_ERRORS^#
544544

545545
// AFTER_CLOSURE_WITH_IF_EXPR_THAT_CONTAIN_ERRORS: Begin completions, 2 items
546-
// AFTER_CLOSURE_WITH_IF_EXPR_THAT_CONTAIN_ERRORS-DAG: Keyword[self]/CurrNominal: .self[#() -> _#]; name=self
547-
// AFTER_CLOSURE_WITH_IF_EXPR_THAT_CONTAIN_ERRORS-DAG: Pattern/CurrModule/Flair[ArgLabels]: ()[#_#]; name=()
546+
// AFTER_CLOSURE_WITH_IF_EXPR_THAT_CONTAIN_ERRORS-DAG: Keyword[self]/CurrNominal: .self[#() -> Void#]; name=self
547+
// AFTER_CLOSURE_WITH_IF_EXPR_THAT_CONTAIN_ERRORS-DAG: Pattern/CurrModule/Flair[ArgLabels]: ()[#Void#]; name=()
548548
// AFTER_CLOSURE_WITH_IF_EXPR_THAT_CONTAIN_ERRORS: End completions
549549
}
550550

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
// {"kind":"typecheck","signature":"(anonymous namespace)::ExprWalker::walkToExprPost(swift::Expr*)","signatureAssert":"Assertion failed: (Ptr && \"Cannot dereference a null Type!\"), function operator->"}
2-
// RUN: not --crash %target-swift-frontend -typecheck %s
2+
// RUN: not %target-swift-frontend -typecheck %s
33
func a {
44
{
55
\ b() a

0 commit comments

Comments
 (0)