From 43887ef39171eb6f7acb5a923247d412aab0e46e Mon Sep 17 00:00:00 2001 From: Alex Hoppen Date: Thu, 10 Feb 2022 21:05:34 +0100 Subject: [PATCH] [CodeCompletion] Only filter solutions based on number of fixes rdar://73282163 --- include/swift/Sema/ConstraintSystem.h | 27 +++++++-- lib/Sema/CSBindings.cpp | 7 ++- lib/Sema/CSRanking.cpp | 15 ++++- lib/Sema/CSSolver.cpp | 10 +++- lib/Sema/TypeCheckConstraints.cpp | 4 ++ test/IDE/complete_ambiguous.swift | 6 +- test/IDE/complete_unresolved_members.swift | 66 +++++++++++++++++++++- 7 files changed, 122 insertions(+), 13 deletions(-) diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 027d5b715ac15..62bfba008d469 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -365,6 +365,10 @@ class TypeVariableType::Implementation { bool isTypeSequence() const; + /// Determine whether this type variable represents a code completion + /// expression. + bool isCodeCompletionToken() const; + /// Retrieve the representative of the equivalence class to which this /// type variable belongs. /// @@ -3032,10 +3036,25 @@ class ConstraintSystem { if (solutions.size() < 2) return; - if (auto best = findBestSolution(solutions, minimize)) { - if (*best != 0) - solutions[0] = std::move(solutions[*best]); - solutions.erase(solutions.begin() + 1, solutions.end()); + if (isForCodeCompletion()) { + // For code completion remove solutions that have more fixes than the + // minimum but don't filter based on the lower-priority scores because we + // want to show all options even if they need to go through more + // conversions. + Score minScore = std::min_element(solutions.begin(), solutions.end(), + [](const Solution &a, const Solution &b) { + return a.getFixedScore() < b.getFixedScore(); + })->getFixedScore(); + + llvm::erase_if(solutions, [&](const Solution &S) { + return S.getFixedScore().Data[SK_Fix] > minScore.Data[SK_Fix]; + }); + } else { + if (auto best = findBestSolution(solutions, minimize)) { + if (*best != 0) + solutions[0] = std::move(solutions[*best]); + solutions.erase(solutions.begin() + 1, solutions.end()); + } } } diff --git a/lib/Sema/CSBindings.cpp b/lib/Sema/CSBindings.cpp index ca318a2eb4f91..55e69e907617c 100644 --- a/lib/Sema/CSBindings.cpp +++ b/lib/Sema/CSBindings.cpp @@ -1788,7 +1788,12 @@ bool TypeVarBindingProducer::computeNext() { // Allow solving for T even for a binding kind where that's invalid // if fixes are allowed, because that gives us the opportunity to // match T? values to the T binding by adding an unwrap fix. - if (binding.Kind == BindingKind::Subtypes || CS.shouldAttemptFixes()) { + // But don't allow it when solving for code completion because otherwise + // we might also receive T as a contextual type and we can't differentiate + // that completion results should only be considered to have a convertible, + // not identical type relation. + if ((binding.Kind == BindingKind::Subtypes || CS.shouldAttemptFixes()) && + !TypeVar->getImpl().isCodeCompletionToken()) { // If we were unsuccessful solving for T?, try solving for T. if (auto objTy = type->getOptionalObjectType()) { // If T is a type variable, only attempt this if both the diff --git a/lib/Sema/CSRanking.cpp b/lib/Sema/CSRanking.cpp index 536ee9fcf3e7b..5e4024f191b01 100644 --- a/lib/Sema/CSRanking.cpp +++ b/lib/Sema/CSRanking.cpp @@ -130,10 +130,21 @@ bool ConstraintSystem::worseThanBestSolution() const { if (getASTContext().TypeCheckerOpts.DisableConstraintSolverPerformanceHacks) return false; - if (!solverState || !solverState->BestScore || - CurrentScore <= *solverState->BestScore) + if (!solverState || !solverState->BestScore) return false; + if (isForCodeCompletion()) { + // For code completion, we only filter based on SK_Fix, not the entire + // score. See ConstraintSystem::filterSolutions. + if (CurrentScore.Data[SK_Fix] <= solverState->BestScore->Data[SK_Fix]) { + return false; + } + } else { + if (CurrentScore <= *solverState->BestScore) { + return false; + } + } + if (isDebugMode()) { llvm::errs().indent(solverState->depth * 2) << "(solution is worse than the best solution)\n"; diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index 7e547d4e030a7..859d4d9d1e237 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -73,8 +73,14 @@ Solution ConstraintSystem::finalize() { // Update the best score we've seen so far. auto &ctx = getASTContext(); - assert(ctx.TypeCheckerOpts.DisableConstraintSolverPerformanceHacks || - !solverState->BestScore || CurrentScore <= *solverState->BestScore); + if (isForCodeCompletion()) { + assert(ctx.TypeCheckerOpts.DisableConstraintSolverPerformanceHacks || + !solverState->BestScore || + CurrentScore.Data[SK_Fix] <= solverState->BestScore->Data[SK_Fix]); + } else { + assert(ctx.TypeCheckerOpts.DisableConstraintSolverPerformanceHacks || + !solverState->BestScore || CurrentScore <= *solverState->BestScore); + } if (!solverState->BestScore || CurrentScore <= *solverState->BestScore) { solverState->BestScore = CurrentScore; diff --git a/lib/Sema/TypeCheckConstraints.cpp b/lib/Sema/TypeCheckConstraints.cpp index 70eea76808872..986187b66bec9 100644 --- a/lib/Sema/TypeCheckConstraints.cpp +++ b/lib/Sema/TypeCheckConstraints.cpp @@ -130,6 +130,10 @@ bool TypeVariableType::Implementation::isTypeSequence() const { && locator->getGenericParameter()->isTypeSequence(); } +bool TypeVariableType::Implementation::isCodeCompletionToken() const { + return locator && locator->directlyAt(); +} + void *operator new(size_t bytes, ConstraintSystem& cs, size_t alignment) { return cs.getAllocator().Allocate(bytes, alignment); diff --git a/test/IDE/complete_ambiguous.swift b/test/IDE/complete_ambiguous.swift index 628e333dcf1c4..76a00987fca12 100644 --- a/test/IDE/complete_ambiguous.swift +++ b/test/IDE/complete_ambiguous.swift @@ -448,8 +448,8 @@ struct Struct123: Equatable { } func testBestSolutionFilter() { let a = Struct123(); - let b = [Struct123]().first(where: { $0 == a && 1 + 90 * 5 / 8 == 45 * -10 })?.structMem != .#^BEST_SOLUTION_FILTER?xfail=rdar73282163^# - let c = min(10.3, 10 / 10.4) < 6 / 7 ? true : Optional(a)?.structMem != .#^BEST_SOLUTION_FILTER2?check=BEST_SOLUTION_FILTER;xfail=rdar73282163^# + let b = [Struct123]().first(where: { $0 == a && 1 + 90 * 5 / 8 == 45 * -10 })?.structMem != .#^BEST_SOLUTION_FILTER^# + let c = min(10.3, 10 / 10.4) < 6 / 7 ? true : Optional(a)?.structMem != .#^BEST_SOLUTION_FILTER2?check=BEST_SOLUTION_FILTER^# } // BEST_SOLUTION_FILTER: Begin completions @@ -465,7 +465,7 @@ func testBestSolutionGeneric() { func genAndInt(_ x: Int) -> Int { return 1 } func genAndInt(_ x: T) -> Test1 { return Test1() } - genAndInt(2).#^BEST_SOLUTION_FILTER_GEN?xfail=rdar73282163^# + genAndInt(2).#^BEST_SOLUTION_FILTER_GEN^# } // BEST_SOLUTION_FILTER_GEN: Begin completions diff --git a/test/IDE/complete_unresolved_members.swift b/test/IDE/complete_unresolved_members.swift index 0cc2bb602ed85..ef4498b612491 100644 --- a/test/IDE/complete_unresolved_members.swift +++ b/test/IDE/complete_unresolved_members.swift @@ -259,7 +259,18 @@ class C6 { class C6 { func f1(e: SomeEnum1) { - if let x = Optional(e) where x == .#^UNRESOLVED_25?check=UNRESOLVED_3^# + if let x = Optional(e) where x == .#^UNRESOLVED_25^# +// UNRESOLVED_25: Begin completions, 9 items +// UNRESOLVED_25-DAG: Decl[EnumElement]/CurrNominal/Flair[ExprSpecific]/TypeRelation[Identical]: North[#SomeEnum1#]; +// UNRESOLVED_25-DAG: Decl[EnumElement]/CurrNominal/Flair[ExprSpecific]/TypeRelation[Identical]: South[#SomeEnum1#]; +// UNRESOLVED_25-DAG: Decl[InstanceMethod]/CurrNominal/TypeRelation[Invalid]: hash({#(self): SomeEnum1#})[#(into: inout Hasher) -> Void#]; +// UNRESOLVED_25-DAG: Keyword[nil]/None/Erase[1]/TypeRelation[Identical]: nil[#SomeEnum1?#]; name=nil +// UNRESOLVED_25-DAG: Decl[EnumElement]/CurrNominal/IsSystem/TypeRelation[Identical]: none[#Optional#]; name=none +// UNRESOLVED_25-DAG: Decl[EnumElement]/CurrNominal/IsSystem/TypeRelation[Identical]: some({#SomeEnum1#})[#Optional#]; +// UNRESOLVED_25-DAG: Decl[InstanceMethod]/CurrNominal/IsSystem: map({#(self): Optional#})[#((SomeEnum1) throws -> U) -> U?#]; +// UNRESOLVED_25-DAG: Decl[InstanceMethod]/CurrNominal/IsSystem: flatMap({#(self): Optional#})[#((SomeEnum1) throws -> U?) -> U?#]; +// UNRESOLVED_25-DAG: Decl[InstanceMethod]/CurrNominal/IsSystem/TypeRelation[Invalid]: hash({#(self): Optional#})[#(into: inout Hasher) -> Void#]; +// UNRESOLVED_25: End completions } } class C7 {} @@ -721,3 +732,56 @@ func testSameType() { // SUGAR_TYPE: End completions } +func testOverloadWithConvertibleArg() { + // This test case used to cause problems because the second overload needs a value to optional conversion, which increases the solution’s score in one of the later compontens. + // We were prematurely filtering the solution because its score is worse than the 0-score of the first overload (which only ignores the second argument, which doesn't increase the score) + struct S { + func xxx(_ foo: Foo) { } + func xxx(_ bar: Bar, _ other: Int?) { } + } + + struct Foo { + init(x: String) {} + } + + struct Bar { + static let top: Bar = Bar() + } + + S().xxx(.#^CONVERTIBLE_ARG_OVERLOAD^#, 32) +// CONVERTIBLE_ARG_OVERLOAD: Begin completions, 2 items +// CONVERTIBLE_ARG_OVERLOAD-DAG: Decl[StaticVar]/CurrNominal/Flair[ExprSpecific]/TypeRelation[Identical]: top[#Bar#]; +// CONVERTIBLE_ARG_OVERLOAD-DAG: Decl[Constructor]/CurrNominal/TypeRelation[Identical]: init()[#Bar#]; +// CONVERTIBLE_ARG_OVERLOAD: End completions +} + +func testOverloadWithConvertibleArgWithFixesInScore() { + // same as testOverloadWithConvertibleArg but with solutions that contain a fix-score > 0 because the result builder has no body for the initialization of FStack. + + @resultBuilder struct ResBuilder { + static func buildBlock(_ content: Content) -> Content { fatalError() } + } + + struct S { + init(@ResBuilder content: () -> Content) {} + + func xxx(_ foo: Foo) -> Self { return self } + func xxx(_ bar: Bar = .top, _ other: Int?) -> Self { return self } + } + + struct Foo { + init(x: String) {} + } + + struct Bar { + static let top: Bar = Bar() + } + + S() {} + .xxx(.#^CONVERTIBLE_ARG_OVERLOAD_WITH_FIX_IN_SCORE^#top, 32) +// CONVERTIBLE_ARG_OVERLOAD_WITH_FIX_IN_SCORE: Begin completions, 2 items +// CONVERTIBLE_ARG_OVERLOAD_WITH_FIX_IN_SCORE-DAG: Decl[StaticVar]/CurrNominal/Flair[ExprSpecific]/TypeRelation[Identical]: top[#Bar#]; name=top +// CONVERTIBLE_ARG_OVERLOAD_WITH_FIX_IN_SCORE-DAG: Decl[Constructor]/CurrNominal/TypeRelation[Identical]: init()[#Bar#]; name=init() +// CONVERTIBLE_ARG_OVERLOAD_WITH_FIX_IN_SCORE: End completions + +}