Skip to content

[5.5][CodeCompletion] Peform complete filterSolutions in code completion #37076

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 27, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions include/swift/Sema/ConstraintSystem.h
Original file line number Diff line number Diff line change
Expand Up @@ -4874,11 +4874,9 @@ class ConstraintSystem {
/// \param diff The differences among the solutions.
/// \param idx1 The index of the first solution.
/// \param idx2 The index of the second solution.
/// \param isForCodeCompletion Whether solving for code completion.
static SolutionCompareResult
compareSolutions(ConstraintSystem &cs, ArrayRef<Solution> solutions,
const SolutionDiff &diff, unsigned idx1, unsigned idx2,
bool isForCodeCompletion);
const SolutionDiff &diff, unsigned idx1, unsigned idx2);

public:
/// Increase the score of the given kind for the current (partial) solution
Expand Down
45 changes: 4 additions & 41 deletions lib/Sema/CSRanking.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -763,48 +763,14 @@ static void addKeyPathDynamicMemberOverloads(
}
}

SolutionCompareResult compareSolutionsForCodeCompletion(
ConstraintSystem &cs, ArrayRef<Solution> solutions, unsigned idx1,
unsigned idx2) {

// When solving for code completion we can't consider one solution worse than
// another according to the same rules as regular compilation. For example,
// with the code below:
//
// func foo(_ x: Int) -> Int {}
// func foo<T>(_ x: T) -> String {}
// foo(3).<complete here> // Still want solutions with for both foo
// // overloads - String and Int members are both
// // valid here.
//
// the comparison for regular compilation considers the solution with the more
// specialized `foo` overload `foo(_: Int)` to be better than the solution
// with the generic overload `foo(_: T)` even though both are otherwise
// viable. For code completion purposes offering members of 'String' based
// on the solution with the generic overload is equally as import as offering
// members of 'Int' as choosing one of those completions will then result in
// regular compilation resolving the call to the generic overload instead.

if (solutions[idx1].getFixedScore() == solutions[idx2].getFixedScore())
return SolutionCompareResult::Incomparable;
return solutions[idx1].getFixedScore() < solutions[idx2].getFixedScore()
? SolutionCompareResult::Better
: SolutionCompareResult::Worse;
}


SolutionCompareResult ConstraintSystem::compareSolutions(
ConstraintSystem &cs, ArrayRef<Solution> solutions,
const SolutionDiff &diff, unsigned idx1, unsigned idx2,
bool isForCodeCompletion) {
const SolutionDiff &diff, unsigned idx1, unsigned idx2) {
if (cs.isDebugMode()) {
llvm::errs().indent(cs.solverState->depth * 2)
<< "comparing solutions " << idx1 << " and " << idx2 <<"\n";
}

if (isForCodeCompletion)
return compareSolutionsForCodeCompletion(cs, solutions, idx1, idx2);

// Whether the solutions are identical.
bool identical = true;

Expand Down Expand Up @@ -1340,8 +1306,7 @@ ConstraintSystem::findBestSolution(SmallVectorImpl<Solution> &viable,
SmallVector<bool, 16> losers(viable.size(), false);
unsigned bestIdx = 0;
for (unsigned i = 1, n = viable.size(); i != n; ++i) {
switch (compareSolutions(*this, viable, diff, i, bestIdx,
isForCodeCompletion())) {
switch (compareSolutions(*this, viable, diff, i, bestIdx)) {
case SolutionCompareResult::Identical:
// FIXME: Might want to warn about this in debug builds, so we can
// find a way to eliminate the redundancy in the search space.
Expand All @@ -1365,8 +1330,7 @@ ConstraintSystem::findBestSolution(SmallVectorImpl<Solution> &viable,
if (i == bestIdx)
continue;

switch (compareSolutions(*this, viable, diff, bestIdx, i,
isForCodeCompletion())) {
switch (compareSolutions(*this, viable, diff, bestIdx, i)) {
case SolutionCompareResult::Identical:
// FIXME: Might want to warn about this in debug builds, so we can
// find a way to eliminate the redundancy in the search space.
Expand Down Expand Up @@ -1418,8 +1382,7 @@ ConstraintSystem::findBestSolution(SmallVectorImpl<Solution> &viable,
if (losers[j])
continue;

switch (compareSolutions(*this, viable, diff, i, j,
isForCodeCompletion())) {
switch (compareSolutions(*this, viable, diff, i, j)) {
case SolutionCompareResult::Identical:
// FIXME: Dub one of these the loser arbitrarily?
break;
Expand Down
6 changes: 3 additions & 3 deletions test/IDE/complete_ambiguous.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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^#
let c = min(10.3, 10 / 10.4) < 6 / 7 ? true : Optional(a)?.structMem != .#^BEST_SOLUTION_FILTER2?check=BEST_SOLUTION_FILTER^#
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^#
}

// BEST_SOLUTION_FILTER: Begin completions
Expand All @@ -465,7 +465,7 @@ func testBestSolutionGeneric() {
func genAndInt(_ x: Int) -> Int { return 1 }
func genAndInt<T>(_ x: T) -> Test1 { return Test1() }

genAndInt(2).#^BEST_SOLUTION_FILTER_GEN^#
genAndInt(2).#^BEST_SOLUTION_FILTER_GEN?xfail=rdar73282163^#
}

// BEST_SOLUTION_FILTER_GEN: Begin completions
Expand Down