Skip to content

Commit 3c90c89

Browse files
ahoppenxedin
authored andcommitted
[CodeCompletion] Drop ForCodeCompletion constraint system option for syntactic elements that don't contain completion token
1 parent c03addb commit 3c90c89

10 files changed

+121
-13
lines changed

include/swift/AST/Decl.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1969,6 +1969,10 @@ class PatternBindingDecl final : public Decl,
19691969
getMutablePatternList()[i].setInit(E);
19701970
}
19711971

1972+
void setOriginalInit(unsigned i, Expr *E) {
1973+
getMutablePatternList()[i].setOriginalInit(E);
1974+
}
1975+
19721976
Pattern *getPattern(unsigned i) const {
19731977
return getPatternList()[i].getPattern();
19741978
}

include/swift/Sema/ConstraintSystem.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6507,6 +6507,10 @@ class ConjunctionElement {
65076507
Element->print(Out, SM, indent);
65086508
}
65096509

6510+
/// Returns \c false if this conjunction element is known not to contain the
6511+
/// code compleiton token.
6512+
bool mightContainCodeCompletionToken(const ConstraintSystem &cs) const;
6513+
65106514
private:
65116515
/// Find type variables referenced by this conjunction element.
65126516
/// If this is a closure body element, it would look inside \c ASTNode.

lib/AST/Decl.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1529,6 +1529,7 @@ PatternBindingDecl *PatternBindingDecl::createImplicit(
15291529
Pat, /*EqualLoc*/ SourceLoc(), nullptr, Parent);
15301530
Result->setImplicit();
15311531
Result->setInit(0, E);
1532+
Result->setOriginalInit(0, E);
15321533
return Result;
15331534
}
15341535

lib/Sema/BuilderTransform.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -959,8 +959,8 @@ class ResultBuilderTransform
959959
VarDecl *captureExpr(Expr *expr, SmallVectorImpl<ASTNode> &container) {
960960
auto *var = builder.buildVar(expr->getStartLoc());
961961
Pattern *pattern = NamedPattern::createImplicit(ctx, var);
962-
auto *PB = PatternBindingDecl::createImplicit(ctx, StaticSpellingKind::None,
963-
pattern, expr, dc);
962+
auto *PB = PatternBindingDecl::createImplicit(
963+
ctx, StaticSpellingKind::None, pattern, expr, dc, var->getStartLoc());
964964
return recordVar(PB, container);
965965
}
966966

@@ -972,7 +972,8 @@ class ResultBuilderTransform
972972
ctx, NamedPattern::createImplicit(ctx, var),
973973
type ? type : PlaceholderType::get(ctx, var));
974974
auto *PB = PatternBindingDecl::createImplicit(
975-
ctx, StaticSpellingKind::None, placeholder, /*init=*/initExpr, dc);
975+
ctx, StaticSpellingKind::None, placeholder, /*init=*/initExpr, dc,
976+
var->getStartLoc());
976977
return recordVar(PB, container);
977978
}
978979

lib/Sema/CSStep.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -880,6 +880,17 @@ bool ConjunctionStep::attempt(const ConjunctionElement &element) {
880880
CS.Timer.emplace(element.getLocator(), CS);
881881
}
882882

883+
assert(!ModifiedOptions.hasValue() &&
884+
"Previously modified options should have been restored in resume");
885+
if (CS.isForCodeCompletion() &&
886+
!element.mightContainCodeCompletionToken(CS)) {
887+
ModifiedOptions.emplace(CS.Options);
888+
// If we know that this conjunction element doesn't contain the code
889+
// completion token, type check it in normal mode without any special
890+
// behavior that is intended for the code completion token.
891+
CS.Options -= ConstraintSystemFlags::ForCodeCompletion;
892+
}
893+
883894
auto success = element.attempt(CS);
884895

885896
// If element attempt has failed, mark whole conjunction
@@ -891,6 +902,9 @@ bool ConjunctionStep::attempt(const ConjunctionElement &element) {
891902
}
892903

893904
StepResult ConjunctionStep::resume(bool prevFailed) {
905+
// Restore the old ConstraintSystemOptions if 'attempt' modified them.
906+
ModifiedOptions.reset();
907+
894908
// Return from the follow-up splitter step that
895909
// attempted to apply information gained from the
896910
// isolated constraint to the outer context.

lib/Sema/CSStep.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -966,6 +966,11 @@ class ConjunctionStep : public BindingStep<ConjunctionElementProducer> {
966966
/// in isolated mode.
967967
SmallVector<Solution, 4> IsolatedSolutions;
968968

969+
/// If \c ConjunctionStep::attempt modified the constraint system options,
970+
/// it will store the original options in this \c llvm::SaveAndRestore.
971+
/// Upon \c resume, these values will be restored.
972+
Optional<llvm::SaveAndRestore<ConstraintSystemOptions>> ModifiedOptions;
973+
969974
public:
970975
ConjunctionStep(ConstraintSystem &cs, Constraint *conjunction,
971976
SmallVectorImpl<Solution> &solutions)

lib/Sema/CSSyntacticElement.cpp

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2206,6 +2206,21 @@ bool ConstraintSystem::applySolutionToBody(Solution &solution,
22062206
return false;
22072207
}
22082208

2209+
bool ConjunctionElement::mightContainCodeCompletionToken(
2210+
const ConstraintSystem &cs) const {
2211+
if (Element->getKind() == ConstraintKind::SyntacticElement) {
2212+
if (Element->getSyntacticElement().getSourceRange().isInvalid()) {
2213+
return true;
2214+
} else {
2215+
return cs.containsIDEInspectionTarget(Element->getSyntacticElement());
2216+
}
2217+
} else {
2218+
// All other constraint kinds are not handled yet. Assume that they might
2219+
// contain the code completion token.
2220+
return true;
2221+
}
2222+
}
2223+
22092224
void ConjunctionElement::findReferencedVariables(
22102225
ConstraintSystem &cs, SmallPtrSetImpl<TypeVariableType *> &typeVars) const {
22112226
auto referencedVars = Element->getTypeVariables();

test/IDE/complete_in_result_builder.swift

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,3 +250,27 @@ func testTypeRelationInResultBuilder() {
250250
}
251251
}
252252
}
253+
254+
func testAmbiguousInResultBuilder() {
255+
@resultBuilder
256+
struct MyViewBuilder {
257+
static func buildBlock(_ x: Int) -> Int { return x }
258+
}
259+
260+
struct QStack {
261+
init(@MyViewBuilder content: () -> Int) {}
262+
}
263+
264+
struct Foo {
265+
func qtroke(_ content: Int, style: Int) -> Int { return 1 }
266+
func qtroke(_ content: Int, lineWidth: Int = 1) -> Int { return 2 }
267+
}
268+
269+
QStack {
270+
Foo().qtroke(0, #^AMBIGUOUS_IN_RESULT_BUILDER^#)
271+
// AMBIGUOUS_IN_RESULT_BUILDER: Begin completions, 2 items
272+
// AMBIGUOUS_IN_RESULT_BUILDER-DAG: Pattern/Local/Flair[ArgLabels]: {#style: Int#}[#Int#];
273+
// AMBIGUOUS_IN_RESULT_BUILDER-DAG: Pattern/Local/Flair[ArgLabels]: {#lineWidth: Int#}[#Int#];
274+
// AMBIGUOUS_IN_RESULT_BUILDER: End completions
275+
}
276+
}
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token COMPLETE | %FileCheck %s
2+
3+
protocol View {}
4+
5+
@resultBuilder
6+
struct ViewBuilder {
7+
static func buildBlock(_ component: MyView) -> MyView {
8+
MyView()
9+
}
10+
}
11+
12+
struct QStack<Content> {
13+
init(@ViewBuilder content: () -> Content) {}
14+
15+
func qheet<Content>(@ViewBuilder content: @escaping () -> Content) -> some View {
16+
MyView()
17+
}
18+
}
19+
20+
struct MyView : View {
21+
func qag<V>(_ tag: V) -> MyView {
22+
MyView()
23+
}
24+
}
25+
26+
27+
func foo() {
28+
QStack {
29+
// When solving for code completion, the constraint system doesn't increase the score for non-default literals.
30+
// This causes ambiguous results for this qag call if it's type checked for code completion, preventing results from showing up at the code completion token.
31+
// Solution is to do normal type checking for syntactic elements that don't contain the code completion token.
32+
MyView().qag(0)
33+
}
34+
.qheet() {
35+
MyView().#^COMPLETE^#
36+
}
37+
}
38+
// CHECK: Begin completions, 2 items
39+
// CHECK-DAG: Keyword[self]/CurrNominal: self[#MyView#];
40+
// CHECK-DAG: Decl[InstanceMethod]/CurrNominal/TypeRelation[Convertible]: qag({#(tag): V#})[#MyView#];
41+
// CHECK: End completions

test/IDE/complete_type_relation_global_results.swift

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -137,15 +137,14 @@ func testGenericReturn<T: MyProto>() -> T {
137137
return #^GENERIC_RETURN^#
138138
}
139139

140-
// FIXME: We don't support USR-based type comparison in generic contexts. MyProto, returnMyProto, makeFoo and FooBar should be 'Convertible'
141140
// GENERIC_RETURN: Begin completions
142-
// GENERIC_RETURN-DAG: Decl[Struct]/OtherModule[Lib]: Foo[#Foo#];
143-
// GENERIC_RETURN-DAG: Decl[GlobalVar]/OtherModule[Lib]: GLOBAL_FOO[#Foo#];
141+
// GENERIC_RETURN-DAG: Decl[Struct]/OtherModule[Lib]/TypeRelation[Convertible]: Foo[#Foo#];
142+
// GENERIC_RETURN-DAG: Decl[GlobalVar]/OtherModule[Lib]/TypeRelation[Convertible]: GLOBAL_FOO[#Foo#];
144143
// GENERIC_RETURN-DAG: Decl[Struct]/OtherModule[Lib]: Bar[#Bar#];
145-
// GENERIC_RETURN-DAG: Decl[Protocol]/OtherModule[Lib]/Flair[RareType]: MyProto[#MyProto#];
146-
// GENERIC_RETURN-DAG: Decl[FreeFunction]/OtherModule[Lib]: makeFoo()[#Foo#];
147-
// GENERIC_RETURN-DAG: Decl[Struct]/OtherModule[Lib]: FooBar[#FooBar#];
148-
// GENERIC_RETURN-DAG: Decl[FreeFunction]/OtherModule[Lib]: returnSomeMyProto()[#MyProto#];
144+
// GENERIC_RETURN-DAG: Decl[Protocol]/OtherModule[Lib]/Flair[RareType]/TypeRelation[Convertible]: MyProto[#MyProto#];
145+
// GENERIC_RETURN-DAG: Decl[FreeFunction]/OtherModule[Lib]/TypeRelation[Convertible]: makeFoo()[#Foo#];
146+
// GENERIC_RETURN-DAG: Decl[Struct]/OtherModule[Lib]/TypeRelation[Convertible]: FooBar[#FooBar#];
147+
// GENERIC_RETURN-DAG: Decl[FreeFunction]/OtherModule[Lib]/TypeRelation[Convertible]: returnSomeMyProto()[#MyProto#];
149148
// GENERIC_RETURN: End completions
150149

151150
// RUN: %empty-directory(%t/completion-cache)
@@ -220,9 +219,9 @@ func protoWithAssocTypeInGenericContext<T: ProtoWithAssocType>() -> T {
220219
}
221220

222221
// PROTO_WITH_ASSOC_TYPE_GENERIC_RETURN_CONTEXT: Begin completions
223-
// PROTO_WITH_ASSOC_TYPE_GENERIC_RETURN_CONTEXT-DAG: Decl[Struct]/OtherModule[Lib]: StructWithAssocType[#StructWithAssocType#];
224-
// PROTO_WITH_ASSOC_TYPE_GENERIC_RETURN_CONTEXT-DAG: Decl[FreeFunction]/OtherModule[Lib]: makeProtoWithAssocType()[#ProtoWithAssocType#];
225-
// PROTO_WITH_ASSOC_TYPE_GENERIC_RETURN_CONTEXT-DAG: Decl[Protocol]/OtherModule[Lib]/Flair[RareType]: ProtoWithAssocType[#ProtoWithAssocType#];
222+
// PROTO_WITH_ASSOC_TYPE_GENERIC_RETURN_CONTEXT-DAG: Decl[Struct]/OtherModule[Lib]/TypeRelation[Convertible]: StructWithAssocType[#StructWithAssocType#];
223+
// PROTO_WITH_ASSOC_TYPE_GENERIC_RETURN_CONTEXT-DAG: Decl[FreeFunction]/OtherModule[Lib]/TypeRelation[Convertible]: makeProtoWithAssocType()[#ProtoWithAssocType#];
224+
// PROTO_WITH_ASSOC_TYPE_GENERIC_RETURN_CONTEXT-DAG: Decl[Protocol]/OtherModule[Lib]/Flair[RareType]/TypeRelation[Convertible]: ProtoWithAssocType[#ProtoWithAssocType#];
226225
// PROTO_WITH_ASSOC_TYPE_GENERIC_RETURN_CONTEXT: End completions
227226

228227

0 commit comments

Comments
 (0)