Skip to content

Commit 5d01a09

Browse files
committed
[CodeCompletion] Don't distinguish convertible and idenical type relation
I think that preferring identical over convertible makes sense in e.g. C++ where we have implicit user-defined type conversions but since we don’t have them in Swift, I think the distinction doesn’t make too much sense, because if we have a `func foo(x: Int?)`, want don’t really want to prioritize variables of type `Int?` over `Int` Similarly if we have `func foo(x: View)`, we don’t want to prioritize a variable of type `View` over e.g. `Text`. rdar://91349364
1 parent f9d27b9 commit 5d01a09

File tree

56 files changed

+508
-530
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

56 files changed

+508
-530
lines changed

include/swift/IDE/CodeCompletionResultType.h

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -100,13 +100,10 @@ enum class CodeCompletionResultTypeRelation : uint8_t {
100100
/// The result's type is invalid at the expected position.
101101
Invalid,
102102

103-
/// The result's type is convertible to the type of the expected.
103+
/// The result's type is convertible or identical to the type of the expected.
104104
Convertible,
105105

106-
/// The result's type is identical to the type of the expected.
107-
Identical,
108-
109-
MAX_VALUE = Identical
106+
MAX_VALUE = Convertible
110107
};
111108

112109
class USRBasedType;
@@ -145,23 +142,14 @@ class USRBasedTypeContext {
145142
/// convertible.
146143
llvm::SmallVector<const USRBasedType *, 1> Types;
147144

148-
/// Whether a match against this type should be considered convertible
149-
/// instead of identical. This is used to model optional conversions.
150-
/// If we have
151-
/// let x: Int? = #^COMPLETE^#
152-
/// we add both 'Int?' and 'Int' as contextual types to the
153-
/// USRBasedTypeContext, but an identical match against 'Int' should only
154-
/// be considered convertible.
155-
bool IsConvertible;
156-
157145
public:
158146
/// Compute the type relation of \p ResultType to this conextual type.
159147
CodeCompletionResultTypeRelation
160148
typeRelation(const USRBasedType *ResultType,
161149
const USRBasedType *VoidType) const;
162150

163-
ContextualType(ArrayRef<const USRBasedType *> Types, bool IsConvertible)
164-
: Types(Types.begin(), Types.end()), IsConvertible(IsConvertible) {
151+
ContextualType(ArrayRef<const USRBasedType *> Types)
152+
: Types(Types.begin(), Types.end()) {
165153
assert(!Types.empty() && "USRBasedTypeContext::ContextualType should "
166154
"contain at least one type");
167155
}

lib/IDE/CodeCompletionResult.cpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -490,9 +490,6 @@ void CodeCompletionResult::printPrefix(raw_ostream &OS) const {
490490
case CodeCompletionResultTypeRelation::Invalid:
491491
Prefix.append("/TypeRelation[Invalid]");
492492
break;
493-
case CodeCompletionResultTypeRelation::Identical:
494-
Prefix.append("/TypeRelation[Identical]");
495-
break;
496493
case CodeCompletionResultTypeRelation::Convertible:
497494
Prefix.append("/TypeRelation[Convertible]");
498495
break;

lib/IDE/CodeCompletionResultType.cpp

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -29,15 +29,13 @@ USRBasedTypeContext::USRBasedTypeContext(const ExpectedTypeContext *TypeContext,
2929
: Arena(Arena) {
3030

3131
for (auto possibleTy : TypeContext->getPossibleTypes()) {
32-
ContextualTypes.emplace_back(USRBasedType::fromType(possibleTy, Arena),
33-
/*IsConvertible=*/false);
32+
ContextualTypes.emplace_back(USRBasedType::fromType(possibleTy, Arena));
3433

3534
// Add the unwrapped optional types as 'convertible' contextual types.
3635
auto UnwrappedOptionalType = possibleTy->getOptionalObjectType();
3736
while (UnwrappedOptionalType) {
3837
ContextualTypes.emplace_back(
39-
USRBasedType::fromType(UnwrappedOptionalType, Arena),
40-
/*IsConvertible=*/true);
38+
USRBasedType::fromType(UnwrappedOptionalType, Arena));
4139
UnwrappedOptionalType = UnwrappedOptionalType->getOptionalObjectType();
4240
}
4341

@@ -57,7 +55,7 @@ USRBasedTypeContext::USRBasedTypeContext(const ExpectedTypeContext *TypeContext,
5755
// Archetypes are also be used to model generic return types, in which
5856
// case they don't have any conformsTo entries. We simply ignore those.
5957
if (!USRTypes.empty()) {
60-
ContextualTypes.emplace_back(USRTypes, /*IsConvertible=*/false);
58+
ContextualTypes.emplace_back(USRTypes);
6159
}
6260
}
6361
}
@@ -76,6 +74,9 @@ USRBasedTypeContext::typeRelation(const USRBasedType *ResultType) const {
7674
TypeRelation Res = TypeRelation::Unknown;
7775
for (auto &ContextualType : ContextualTypes) {
7876
Res = std::max(Res, ContextualType.typeRelation(ResultType, VoidType));
77+
if (Res == TypeRelation::MAX_VALUE) {
78+
return Res; // We can't improve further
79+
}
7980
}
8081
return Res;
8182
}
@@ -105,7 +106,7 @@ TypeRelation USRBasedType::typeRelationImpl(
105106
return TypeRelation::Unknown;
106107
}
107108
if (ResultType == this) {
108-
return TypeRelation::Identical;
109+
return TypeRelation::Convertible;
109110
}
110111
for (const USRBasedType *Supertype : ResultType->getSupertypes()) {
111112
if (!VisitedTypes.insert(Supertype).second) {
@@ -239,14 +240,10 @@ TypeRelation USRBasedTypeContext::ContextualType::typeRelation(
239240

240241
/// Types is a conjunction, not a disjunction (see documentation on Types),
241242
/// so we need to compute the minimum type relation here.
242-
TypeRelation Result = TypeRelation::Identical;
243+
TypeRelation Result = TypeRelation::Convertible;
243244
for (auto ContextType : Types) {
244245
Result = std::min(Result, ContextType->typeRelation(ResultType, VoidType));
245246
}
246-
247-
if (IsConvertible && Result == TypeRelation::Identical) {
248-
Result = TypeRelation::Convertible;
249-
}
250247
return Result;
251248
}
252249

@@ -262,7 +259,7 @@ static TypeRelation calculateTypeRelation(Type Ty, Type ExpectedTy,
262259
// requirements – ignore them
263260
if (!Ty->hasTypeParameter() && !ExpectedTy->hasTypeParameter()) {
264261
if (Ty->isEqual(ExpectedTy))
265-
return TypeRelation::Identical;
262+
return TypeRelation::Convertible;
266263
bool isAny = false;
267264
isAny |= ExpectedTy->isAny();
268265
isAny |= ExpectedTy->is<ArchetypeType>() &&

test/IDE/complete_accessor.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
// NO_OBSERVER-NOT: willSet
1212
// NO_OBSERVER-NOT: didSet
1313

14-
// WITH_GLOBAL: Decl[GlobalVar]/CurrModule{{(/TypeRelation\[Identical\])?}}: globalValue[#String#];
14+
// WITH_GLOBAL: Decl[GlobalVar]/CurrModule{{(/TypeRelation\[Convertible\])?}}: globalValue[#String#];
1515
// NO_GLOBAL-NOT: globalValue;
1616

1717
// WITH_SELF: Decl[LocalVar]/Local: self[#{{.+}}#]; name=self

test/IDE/complete_after_super.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -107,11 +107,11 @@ extension SuperBaseA {
107107
// COMMON_BASE_A_DOT: End completions
108108

109109
// COMMON_BASE_A_DOT_CONTEXT: Begin completions
110-
// COMMON_BASE_A_DOT_CONTEXT-DAG: Decl[InstanceVar]/CurrNominal/TypeRelation[Identical]: baseInstanceVar[#Int#]{{; name=.+$}}
111-
// COMMON_BASE_A_DOT_CONTEXT-DAG: Decl[InstanceVar]/CurrNominal/TypeRelation[Identical]: baseProp[#Int#]{{; name=.+$}}
110+
// COMMON_BASE_A_DOT_CONTEXT-DAG: Decl[InstanceVar]/CurrNominal/TypeRelation[Convertible]: baseInstanceVar[#Int#]{{; name=.+$}}
111+
// COMMON_BASE_A_DOT_CONTEXT-DAG: Decl[InstanceVar]/CurrNominal/TypeRelation[Convertible]: baseProp[#Int#]{{; name=.+$}}
112112
// COMMON_BASE_A_DOT_CONTEXT-DAG: Decl[InstanceMethod]/CurrNominal: baseFunc0()[#Void#]{{; name=.+$}}
113113
// COMMON_BASE_A_DOT_CONTEXT-DAG: Decl[InstanceMethod]/CurrNominal: baseFunc1({#(a): Int#})[#Void#]{{; name=.+$}}
114-
// COMMON_BASE_A_DOT_CONTEXT-DAG: Decl[InstanceVar]/CurrNominal/TypeRelation[Identical]: baseExtProp[#Int#]{{; name=.+$}}
114+
// COMMON_BASE_A_DOT_CONTEXT-DAG: Decl[InstanceVar]/CurrNominal/TypeRelation[Convertible]: baseExtProp[#Int#]{{; name=.+$}}
115115
// COMMON_BASE_A_DOT_CONTEXT-DAG: Decl[InstanceMethod]/CurrNominal: baseExtFunc0()[#Void#]{{; name=.+$}}
116116
// COMMON_BASE_A_DOT_CONTEXT: End completions
117117

0 commit comments

Comments
 (0)