Skip to content

[DNM] Experiment: Disable fallback completion #83593

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

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft
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: 0 additions & 4 deletions include/swift/IDE/PostfixCompletion.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,6 @@ class PostfixCompletionCallback : public TypeCheckCompletionCallback {
PostfixCompletionCallback(CodeCompletionExpr *CompletionExpr, DeclContext *DC)
: CompletionExpr(CompletionExpr), DC(DC) {}

/// Typecheck the code completion expression in isolation, calling
/// \c sawSolution for each solution formed.
void fallbackTypeCheck(DeclContext *DC) override;

/// Deliver code completion results that were discoverd by \c sawSolution to
/// \p Consumer.
/// \param DotLoc If we are completing after a dot, the location of the dot,
Expand Down
4 changes: 0 additions & 4 deletions include/swift/IDE/TypeCheckCompletionCallback.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,6 @@ class TypeCheckCompletionCallback {
/// True if at least one solution was passed via the \c sawSolution
/// callback.
bool gotCallback() const { return GotCallback; }

/// Typecheck the code completion expression in its outermost expression
/// context, calling \c sawSolution for each solution formed.
virtual void fallbackTypeCheck(DeclContext *DC);
};

// MARK: - Utility functions for subclasses of TypeCheckCompletionCallback
Expand Down
64 changes: 1 addition & 63 deletions include/swift/Sema/CompletionContextFinder.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,22 +25,6 @@ class SyntacticElementTarget;
}

class CompletionContextFinder : public ASTWalker {
enum class ContextKind {
FallbackExpression,
StringInterpolation,
SingleStmtClosure,
MultiStmtClosure,
ErrorExpression
};

struct Context {
ContextKind Kind;
Expr *E;
};

/// Stack of all "interesting" contexts up to code completion expression.
llvm::SmallVector<Context, 4> Contexts;

/// If we are completing inside an expression, the \c CodeCompletionExpr that
/// represents the code completion token.

Expand All @@ -49,44 +33,18 @@ class CompletionContextFinder : public ASTWalker {
/// component.
llvm::PointerUnion<CodeCompletionExpr *, const KeyPathExpr *> CompletionNode;

Expr *InitialExpr = nullptr;
DeclContext *InitialDC;

/// Whether we're looking for any viable fallback expression.
bool ForFallback = false;

/// Finder for fallback completion contexts within the outermost non-closure
/// context of the code completion expression's direct context.
CompletionContextFinder(DeclContext *completionDC)
: InitialDC(completionDC), ForFallback(true) {
while (auto *ACE = dyn_cast<AbstractClosureExpr>(InitialDC))
InitialDC = ACE->getParent();
InitialDC->walkContext(*this);
}

public:
MacroWalking getMacroWalkingBehavior() const override {
return MacroWalking::Arguments;
}

/// Finder for completion contexts within the provided SyntacticElementTarget.
CompletionContextFinder(constraints::SyntacticElementTarget target,
DeclContext *DC);

static CompletionContextFinder forFallback(DeclContext *DC) {
return CompletionContextFinder(DC);
}
CompletionContextFinder(constraints::SyntacticElementTarget target);

PreWalkResult<Expr *> walkToExprPre(Expr *E) override;

PostWalkResult<Expr *> walkToExprPost(Expr *E) override;

PreWalkAction walkToDeclPre(Decl *D) override;

bool locatedInStringInterpolation() const {
return hasContext(ContextKind::StringInterpolation);
}

bool hasCompletionExpr() const {
return CompletionNode.dyn_cast<CodeCompletionExpr *>() != nullptr;
}
Expand Down Expand Up @@ -114,28 +72,8 @@ class CompletionContextFinder : public ASTWalker {
/// If we are completing in a key path, returns the index at which the key
/// path has the code completion component.
size_t getKeyPathCompletionComponentIndex() const;

struct Fallback {
Expr *E; ///< The fallback expression.
DeclContext *DC; ///< The fallback expression's decl context.
bool SeparatePrecheck; ///< True if the fallback may require prechecking.
};

/// As a fallback sometimes its useful to not only type-check
/// code completion expression directly but instead add some
/// of the enclosing context e.g. when completion is an argument
/// to a call.
std::optional<Fallback> getFallbackCompletionExpr() const;

private:
bool hasContext(ContextKind kind) const {
return llvm::find_if(Contexts, [&kind](const Context &currContext) {
return currContext.Kind == kind;
}) != Contexts.end();
}
};


/// Returns \c true if \p range is valid and contains the IDE inspection
/// target. This performs the underlying check based on \c CharSourceRange
/// to make sure we correctly return \c true if the ide inspection target
Expand Down
16 changes: 0 additions & 16 deletions include/swift/Sema/IDETypeChecking.h
Original file line number Diff line number Diff line change
Expand Up @@ -140,22 +140,6 @@ namespace swift {
bool typeCheckASTNodeAtLoc(TypeCheckASTNodeAtLocContext TypeCheckCtx,
SourceLoc TargetLoc);

/// Thunk around \c TypeChecker::typeCheckForCodeCompletion to make it
/// available to \c swift::ide.
/// Type check the given expression and provide results back to code
/// completion via specified callback.
///
/// This method is designed to be used for code completion which means that
/// it doesn't mutate given expression, even if there is a single valid
/// solution, and constraint solver is allowed to produce partially correct
/// solutions. Such solutions can have any number of holes in them.
///
/// \returns `true` if target was applicable and it was possible to infer
/// types for code completion, `false` otherwise.
bool typeCheckForCodeCompletion(
constraints::SyntacticElementTarget &target, bool needsPrecheck,
llvm::function_ref<void(const constraints::Solution &)> callback);

/// Thunk around \c TypeChecker::resolveDeclRefExpr to make it available to
/// \c swift::ide
Expr *resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE, DeclContext *Context);
Expand Down
1 change: 0 additions & 1 deletion lib/IDE/AfterPoundExprCompletion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
#include "swift/IDE/AfterPoundExprCompletion.h"
#include "swift/IDE/CodeCompletion.h"
#include "swift/IDE/CompletionLookup.h"
#include "swift/Sema/CompletionContextFinder.h"
#include "swift/Sema/ConstraintSystem.h"
#include "swift/Sema/IDETypeChecking.h"

Expand Down
12 changes: 0 additions & 12 deletions lib/IDE/CodeCompletion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1494,18 +1494,6 @@ void CodeCompletionCallbacksImpl::typeCheckWithLookup(
TypeCheckASTNodeAtLocContext::declContext(CurDeclContext),
CompletionLoc);
}

// This (hopefully) only happens in cases where the expression isn't
// typechecked during normal compilation either (e.g. member completion in a
// switch case where there control expression is invalid). Having normal
// typechecking still resolve even these cases would be beneficial for
// tooling in general though.
if (!Lookup.gotCallback()) {
if (Context.TypeCheckerOpts.DebugConstraintSolver) {
llvm::errs() << "--- Fallback typecheck for code completion ---\n";
}
Lookup.fallbackTypeCheck(CurDeclContext);
}
}

void CodeCompletionCallbacksImpl::postfixCompletion(SourceLoc CompletionLoc,
Expand Down
6 changes: 6 additions & 0 deletions lib/IDE/ExprCompletion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,12 @@ void ExprTypeCheckCompletionCallback::collectResults(
UnifiedCanHandleAsync |= Result.IsInAsyncContext;
}

// If we didn't find any results, at least try to collect unqualified results.
if (Results.empty()) {
Lookup.getValueCompletionsInDeclContext(CCLoc);
Lookup.getSelfTypeCompletionInDeclContext(CCLoc, /*isForDeclResult=*/false);
}

collectCompletionResults(CompletionCtx, Lookup, DC, UnifiedTypeContext,
UnifiedCanHandleAsync);
}
34 changes: 1 addition & 33 deletions lib/IDE/PostfixCompletion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,10 @@
//
//===----------------------------------------------------------------------===//

#include "swift/Basic/Assertions.h"
#include "swift/IDE/PostfixCompletion.h"
#include "swift/Basic/Assertions.h"
#include "swift/IDE/CodeCompletion.h"
#include "swift/IDE/CompletionLookup.h"
#include "swift/Sema/CompletionContextFinder.h"
#include "swift/Sema/ConstraintSystem.h"
#include "swift/Sema/IDETypeChecking.h"

Expand Down Expand Up @@ -72,37 +71,6 @@ void PostfixCompletionCallback::addResult(const Result &Res) {
Results.push_back(Res);
}

void PostfixCompletionCallback::fallbackTypeCheck(DeclContext *DC) {
assert(!gotCallback());

// Default to checking the completion expression in isolation.
Expr *fallbackExpr = CompletionExpr;
DeclContext *fallbackDC = DC;

auto finder = CompletionContextFinder::forFallback(DC);
if (finder.hasCompletionExpr()) {
if (auto fallback = finder.getFallbackCompletionExpr()) {
fallbackExpr = fallback->E;
fallbackDC = fallback->DC;
}
}

if (isa<AbstractClosureExpr>(fallbackDC)) {
// If the expression is embedded in a closure, the constraint system tries
// to retrieve that closure's type, which will fail since we won't have
// generated any type variables for it. Thus, fallback type checking isn't
// available in this case.
return;
}

SyntacticElementTarget completionTarget(fallbackExpr, fallbackDC, CTP_Unused,
Type(),
/*isDiscared=*/true);

typeCheckForCodeCompletion(completionTarget, /*needsPrecheck*/ true,
[&](const Solution &S) { sawSolution(S); });
}

static ActorIsolation
getClosureActorIsolation(const Solution &S, AbstractClosureExpr *ACE) {
auto getType = [&S](Expr *E) -> Type {
Expand Down
26 changes: 1 addition & 25 deletions lib/IDE/TypeCheckCompletionCallback.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,40 +10,16 @@
//
//===----------------------------------------------------------------------===//

#include "swift/Basic/Assertions.h"
#include "swift/IDE/TypeCheckCompletionCallback.h"
#include "swift/Basic/Assertions.h"
#include "swift/IDE/CompletionLookup.h"
#include "swift/Sema/CompletionContextFinder.h"
#include "swift/Sema/ConstraintSystem.h"
#include "swift/Sema/IDETypeChecking.h"

using namespace swift;
using namespace swift::ide;
using namespace swift::constraints;

void TypeCheckCompletionCallback::fallbackTypeCheck(DeclContext *DC) {
assert(!GotCallback);

auto finder = CompletionContextFinder::forFallback(DC);
if (!finder.hasCompletionExpr())
return;

auto fallback = finder.getFallbackCompletionExpr();
if (!fallback || isa<AbstractClosureExpr>(fallback->DC)) {
// If the expression is embedded in a closure, the constraint system tries
// to retrieve that closure's type, which will fail since we won't have
// generated any type variables for it. Thus, fallback type checking isn't
// available in this case.
return;
}

SyntacticElementTarget completionTarget(fallback->E, fallback->DC, CTP_Unused,
Type(),
/*isDiscared=*/true);
typeCheckForCodeCompletion(completionTarget, /*needsPrecheck=*/true,
[&](const Solution &S) { sawSolution(S); });
}

// MARK: - Utility functions for subclasses of TypeCheckCompletionCallback

Type swift::ide::getTypeForCompletion(const constraints::Solution &S,
Expand Down
3 changes: 1 addition & 2 deletions lib/IDE/UnresolvedMemberCompletion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,10 @@
//
//===----------------------------------------------------------------------===//

#include "swift/Basic/Assertions.h"
#include "swift/IDE/UnresolvedMemberCompletion.h"
#include "swift/Basic/Assertions.h"
#include "swift/IDE/CodeCompletion.h"
#include "swift/IDE/CompletionLookup.h"
#include "swift/Sema/CompletionContextFinder.h"
#include "swift/Sema/ConstraintSystem.h"
#include "swift/Sema/IDETypeChecking.h"

Expand Down
3 changes: 3 additions & 0 deletions lib/Parse/ParseExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1555,6 +1555,9 @@ Parser::parseExprPostfixSuffix(ParserResult<Expr> Result, bool isExprBasic,
if (!Tok.isAtStartOfLine() && Tok.is(tok::unknown)) {
SourceLoc UnknownLoc = consumeToken();
SourceRange ErrorRange = Result.get()->getSourceRange();
if (SourceMgr.rangeContainsIDEInspectionTarget(Lexer::getCharSourceRangeFromSourceRange(SourceMgr, ErrorRange))) {
continue;
}
ErrorRange.widen(UnknownLoc);
Result = makeParserResult(Result, new (Context) ErrorExpr(ErrorRange,
Type(),
Expand Down
2 changes: 1 addition & 1 deletion lib/Sema/BuilderTransform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1007,7 +1007,7 @@ TypeChecker::applyResultBuilderBodyTransform(FuncDecl *func, Type builderType) {
cs.Options |= ConstraintSystemFlags::ForCodeCompletion;
cs.solveForCodeCompletion(solutions);

CompletionContextFinder analyzer(target, func->getDeclContext());
CompletionContextFinder analyzer(target);
if (analyzer.hasCompletion()) {
filterSolutionsForCodeCompletion(solutions, analyzer);
for (const auto &solution : solutions) {
Expand Down
9 changes: 7 additions & 2 deletions lib/Sema/CSGen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4907,9 +4907,14 @@ bool ConstraintSystem::generateConstraints(
return convertTypeLocator;
};

// Substitute type variables in for placeholder types.
if (convertType->hasError()) {
recordFix(
IgnoreInvalidASTNode::create(*this, convertTypeLocator));
}

// Substitute type variables in for placeholder and error types.
convertType = convertType.transformRec([&](Type type) -> std::optional<Type> {
if (type->is<PlaceholderType>()) {
if (isa<PlaceholderType, ErrorType>(type.getPointer())) {
return Type(createTypeVariable(getLocator(type),
TVO_CanBindToNoEscape |
TVO_PrefersSubtypeBinding |
Expand Down
Loading