From 9b0ece29feffd1154513c75596b95e7b385842f1 Mon Sep 17 00:00:00 2001 From: Nathan Hawes Date: Thu, 25 Mar 2021 21:54:44 +1000 Subject: [PATCH 1/7] [CodeCompletion][Sema] Migrate CallArgurment position completion to the solver-based implementation. This hooks up call argument position completion to the typeCheckForCodeCompletion API to generate completions from all the solutions the constraint solver produces (even those requiring fixes), rather than relying on a single solution being applied to the AST (if any). --- .../swift/Sema/CodeCompletionTypeChecking.h | 51 ++++ include/swift/Sema/ConstraintSystem.h | 35 +++ lib/IDE/CodeCompletion.cpp | 257 ++++++++++++------ lib/IDE/ExprContextAnalysis.h | 7 + lib/Sema/CSBindings.cpp | 7 + lib/Sema/CSSimplify.cpp | 247 ++++++++++++++--- lib/Sema/ConstraintSystem.cpp | 8 +- lib/Sema/TypeCheckCodeCompletion.cpp | 142 ++++++++++ lib/Sema/TypeCheckConstraints.cpp | 4 + test/IDE/complete_ambiguous.swift | 85 ++++++ test/IDE/complete_call_arg.swift | 108 +++++++- test/IDE/complete_subscript.swift | 18 +- 12 files changed, 832 insertions(+), 137 deletions(-) diff --git a/include/swift/Sema/CodeCompletionTypeChecking.h b/include/swift/Sema/CodeCompletionTypeChecking.h index 0ea51047f3f07..949acc18ec225 100644 --- a/include/swift/Sema/CodeCompletionTypeChecking.h +++ b/include/swift/Sema/CodeCompletionTypeChecking.h @@ -139,6 +139,57 @@ namespace swift { void sawSolution(const constraints::Solution &solution) override; }; + +class ArgumentTypeCheckCompletionCallback: public TypeCheckCompletionCallback { +public: + struct Result { + /// The type associated with the code completion expression itself. + Type ExpectedType; + /// The innermost call expression containing the completion location. + Expr *CallE; + /// The FuncDecl or SubscriptDecl associated with the call. + Decl *FuncD; + /// The type of the function being called. + Type FuncTy; + /// The index of the argument containing the completion location + unsigned ArgIdx; + /// The index of the parameter corresponding to the completion argument. + Optional ParamIdx; + /// The indices of all params that were bound to non-synthesized arguments. + SmallVector ClaimedParamIndices; + /// True if the completion is a noninitial term in a variadic argument. + bool IsNoninitialVariadic; + /// The base type of the call. + Type BaseType; + /// True if an argument label precedes the completion location. + bool HasLabel; + + /// \returns true if the code completion is within a subscript. + bool isSubscript() const; + }; + +private: + CodeCompletionExpr *CompletionExpr; + SmallVector Results; + bool GotCallback = false; + +public: + ArgumentTypeCheckCompletionCallback(CodeCompletionExpr *CompletionExpr) + : CompletionExpr(CompletionExpr) {} + + ArrayRef getResults() const { return Results; } + + /// 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. + void fallbackTypeCheck(DeclContext *DC); + + void sawSolution(const constraints::Solution &solution) override; +}; + } #endif diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 1e7a31777e3c5..3dd927966658e 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -360,6 +360,10 @@ class TypeVariableType::Implementation { /// a type of a key path expression. bool isKeyPathType() 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. /// @@ -5321,8 +5325,38 @@ class MatchCallArgumentListener { /// \returns true to indicate that this should cause a failure, false /// otherwise. virtual bool relabelArguments(ArrayRef newNames); + + /// Indicates that arguments after the code completion token were not valid + /// and ignored. + /// + /// \returns true to indicate that this should cause a failure, false + /// otherwise. + virtual bool invalidArgumentsAfterCompletion(); +}; + +/// For a callsite containing a code completion expression, stores the index of +/// the arg containing it along with the index of the first trailing closure. +struct CompletionArgInfo { + unsigned completionIdx; + Optional firstTrailingIdx; + unsigned argCount; + + /// \returns true if the given argument index is possibly about to be written + /// by the user (given the completion index) so shouldn't be penalised as + /// missing when ranking solutions. + bool allowsMissingArgAt(unsigned argInsertIdx, AnyFunctionType::Param param); + + /// \returns true if the argument containing the completion location is before + /// the argument with the given index. + bool isBefore(unsigned argIdx) { return completionIdx < argIdx; } }; +/// Extracts the index of the argument containing the code completion location +/// from the provided anchor if it's a \c CallExpr, \c SubscriptExpr, or +/// \c ObjectLiteralExpr). +Optional +getCompletionArgInfo(ASTNode anchor, constraints::ConstraintSystem &cs); + /// Match the call arguments (as described by the given argument type) to /// the parameters (as described by the given parameter type). /// @@ -5348,6 +5382,7 @@ matchCallArguments( ArrayRef params, const ParameterListInfo ¶mInfo, Optional unlabeledTrailingClosureIndex, + Optional completionInfo, bool allowFixes, MatchCallArgumentListener &listener, Optional trailingClosureMatching); diff --git a/lib/IDE/CodeCompletion.cpp b/lib/IDE/CodeCompletion.cpp index 8f10b297b9359..4968fed494fe3 100644 --- a/lib/IDE/CodeCompletion.cpp +++ b/lib/IDE/CodeCompletion.cpp @@ -1523,32 +1523,6 @@ class CodeCompletionCallbacksImpl : public CodeCompletionCallbacks { std::vector> SubModuleNameVisibilityPairs; - void addSuperKeyword(CodeCompletionResultSink &Sink) { - auto *DC = CurDeclContext->getInnermostTypeContext(); - if (!DC) - return; - auto *CD = DC->getSelfClassDecl(); - if (!CD) - return; - Type ST = CD->getSuperclass(); - if (ST.isNull() || ST->is()) - return; - - CodeCompletionResultBuilder Builder(Sink, - CodeCompletionResult::ResultKind::Keyword, - SemanticContextKind::CurrentNominal, - {}); - if (auto *AFD = dyn_cast(CurDeclContext)) { - if (AFD->getOverriddenDecl() != nullptr) { - Builder.addFlair(CodeCompletionFlairBit::CommonKeywordAtCurrentPosition); - } - } - - Builder.setKeywordKind(CodeCompletionKeywordKind::kw_super); - Builder.addKeyword("super"); - Builder.addTypeAnnotation(ST, PrintOptions()); - } - Optional> typeCheckParsedExpr() { assert(ParsedExpr && "should have an expression"); @@ -6265,6 +6239,34 @@ static void addExprKeywords(CodeCompletionResultSink &Sink, DeclContext *DC) { CodeCompletionResult::ExpectedTypeRelation::NotApplicable, flair); } +static void addSuperKeyword(CodeCompletionResultSink &Sink, DeclContext *DC) { + if (!DC) + return; + auto *TC = DC->getInnermostTypeContext(); + if (!TC) + return; + auto *CD = TC->getSelfClassDecl(); + if (!CD) + return; + Type ST = CD->getSuperclass(); + if (ST.isNull() || ST->is()) + return; + + CodeCompletionResultBuilder Builder(Sink, + CodeCompletionResult::ResultKind::Keyword, + SemanticContextKind::CurrentNominal, + {}); + if (auto *AFD = dyn_cast(DC)) { + if (AFD->getOverriddenDecl() != nullptr) { + Builder.addFlair(CodeCompletionFlairBit::CommonKeywordAtCurrentPosition); + } + } + + Builder.setKeywordKind(CodeCompletionKeywordKind::kw_super); + Builder.addKeyword("super"); + Builder.addTypeAnnotation(ST, PrintOptions()); +} + static void addOpaqueTypeKeyword(CodeCompletionResultSink &Sink) { addKeyword(Sink, "some", CodeCompletionKeywordKind::None, "some"); } @@ -6337,7 +6339,7 @@ void CodeCompletionCallbacksImpl::addKeywords(CodeCompletionResultSink &Sink, case CompletionKind::YieldStmtExpr: case CompletionKind::PostfixExprBeginning: case CompletionKind::ForEachSequence: - addSuperKeyword(Sink); + addSuperKeyword(Sink, CurDeclContext); addLetVarKeywords(Sink); addExprKeywords(Sink, CurDeclContext); addAnyTypeKeyword(Sink, CurDeclContext->getASTContext().TheAnyType); @@ -6744,7 +6746,130 @@ static void deliverCompletionResults(CodeCompletionContext &CompletionContext, Consumer.handleResultsAndModules(CompletionContext, RequestedModules, DC); } -void deliverUnresolvedMemberResults( +/// Populates a vector of parameters to suggest along with a vector of types to +/// match the lookup results against. +/// +/// \Returns true if global lookup should be performed. +static bool addPossibleParams( + const ArgumentTypeCheckCompletionCallback::Result &Ret, + SmallVectorImpl &Params, SmallVectorImpl &Types) { + ArrayRef ParamsToPass = + Ret.FuncTy->getAs()->getParams(); + ParameterList *PL = nullptr; + + if (auto *FD = dyn_cast(Ret.FuncD)) { + PL = FD->getParameters(); + } else if (auto *SD = dyn_cast(Ret.FuncD)) { + PL = SD->getIndices(); + } else { + assert(false && "Unhandled decl type?"); + return true; + } + assert(PL->size() == ParamsToPass.size()); + + if (!Ret.ParamIdx) + return true; + + if (Ret.HasLabel) { + Types.push_back(Ret.ExpectedType); + return true; + } + + bool showGlobalCompletions = false; + for (auto Idx: range(*Ret.ParamIdx, PL->size())) { + bool IsCompletion = Idx == Ret.ParamIdx; + + // Stop at the first param claimed by other arguments. + if (!IsCompletion && llvm::is_contained(Ret.ClaimedParamIndices, Idx)) + break; + + const AnyFunctionType::Param *P = &ParamsToPass[Idx]; + bool Required = !PL->get(Idx)->isDefaultArgument() && !PL->get(Idx)->isVariadic(); + + if (P->hasLabel() && !(IsCompletion && Ret.IsNoninitialVariadic)) { + PossibleParamInfo PP(P, Required); + if (!llvm::any_of(Params, [&](PossibleParamInfo &Existing){ return PP == Existing; })) + Params.push_back(std::move(PP)); + } else { + showGlobalCompletions = true; + Types.push_back(P->getPlainType()); + } + if (Required) + break; + } + return showGlobalCompletions; +} + +static void deliverArgumentResults( + ArrayRef Results, + bool IncludeSignature, SourceLoc Loc, DeclContext *DC, + ide::CodeCompletionContext &CompletionCtx, + CodeCompletionConsumer &Consumer) { + ASTContext &Ctx = DC->getASTContext(); + CompletionLookup Lookup(CompletionCtx.getResultSink(), Ctx, DC, + &CompletionCtx); + + bool shouldPerformGlobalCompletion = Results.empty(); + SmallVector ExpectedTypes; + + if (IncludeSignature && !Results.empty()) { + Lookup.setHaveLParen(true); + for (auto &Result: Results) { + auto SemanticContext = SemanticContextKind::None; + NominalTypeDecl *BaseNominal = nullptr; + Type BaseTy = Result.BaseType; + if (BaseTy) { + if (auto InstanceTy = BaseTy->getMetatypeInstanceType()) + BaseTy = InstanceTy; + if ((BaseNominal = BaseTy->getAnyNominal())) { + SemanticContext = SemanticContextKind::CurrentNominal; + if (Result.FuncD && Result.FuncD->getDeclContext()->getSelfNominalTypeDecl() != BaseNominal) + SemanticContext = SemanticContextKind::Super; + } else if (BaseTy->is()) { + SemanticContext = SemanticContextKind::CurrentNominal; + } + } + if (Result.isSubscript()) { + assert(SemanticContext != SemanticContextKind::None && BaseTy); + auto *SD = dyn_cast_or_null(Result.FuncD); + Lookup.addSubscriptCallPattern(Result.FuncTy->getAs(), + SD, SemanticContext); + } else { + auto *FD = dyn_cast_or_null(Result.FuncD); + Lookup.addFunctionCallPattern(Result.FuncTy->getAs(), + FD, SemanticContext); + } + } + Lookup.setHaveLParen(false); + + shouldPerformGlobalCompletion |= + !Lookup.FoundFunctionCalls || Lookup.FoundFunctionsWithoutFirstKeyword; + } else if (!Results.empty()) { + SmallVector Params; + for (auto &Ret: Results) { + shouldPerformGlobalCompletion |= + addPossibleParams(Ret, Params, ExpectedTypes); + } + Lookup.addCallArgumentCompletionResults(Params); + } + + if (shouldPerformGlobalCompletion) { + for (auto &Result: Results) { + ExpectedTypes.push_back(Result.ExpectedType); + } + Lookup.setExpectedTypes(ExpectedTypes, false); + Lookup.getValueCompletionsInDeclContext(Loc); + Lookup.getSelfTypeCompletionInDeclContext(Loc, /*isForDeclResult=*/false); + + // Add any keywords that can be used in an argument expr position. + addSuperKeyword(CompletionCtx.getResultSink(), DC); + addExprKeywords(CompletionCtx.getResultSink(), DC); + } + + deliverCompletionResults(CompletionCtx, Lookup, DC, Consumer); +} + +static void deliverUnresolvedMemberResults( ArrayRef Results, DeclContext *DC, SourceLoc DotLoc, ide::CodeCompletionContext &CompletionCtx, @@ -6783,7 +6908,7 @@ void deliverUnresolvedMemberResults( deliverCompletionResults(CompletionCtx, Lookup, DC, Consumer); } -void deliverKeyPathResults( +static void deliverKeyPathResults( ArrayRef Results, DeclContext *DC, SourceLoc DotLoc, ide::CodeCompletionContext &CompletionCtx, @@ -6805,7 +6930,7 @@ void deliverKeyPathResults( deliverCompletionResults(CompletionCtx, Lookup, DC, Consumer); } -void deliverDotExprResults( +static void deliverDotExprResults( ArrayRef Results, Expr *BaseExpr, DeclContext *DC, SourceLoc DotLoc, bool IsInSelector, ide::CodeCompletionContext &CompletionCtx, @@ -6911,6 +7036,21 @@ bool CodeCompletionCallbacksImpl::trySolverCompletion(bool MaybeFuncBody) { CompletionContext, Consumer); return true; } + case CompletionKind::CallArg: { + assert(CodeCompleteTokenExpr); + assert(CurDeclContext); + ArgumentTypeCheckCompletionCallback Lookup(CodeCompleteTokenExpr); + llvm::SaveAndRestore + CompletionCollector(Context.CompletionCallback, &Lookup); + typeCheckContextAt(CurDeclContext, CompletionLoc); + + if (!Lookup.gotCallback()) + Lookup.fallbackTypeCheck(CurDeclContext); + + deliverArgumentResults(Lookup.getResults(), ShouldCompleteCallPatternAfterParen, + CompletionLoc, CurDeclContext, CompletionContext, Consumer); + return true; + } default: return false; } @@ -7032,6 +7172,7 @@ void CodeCompletionCallbacksImpl::doneParsing() { case CompletionKind::DotExpr: case CompletionKind::UnresolvedMember: case CompletionKind::KeyPathExprSwift: + case CompletionKind::CallArg: llvm_unreachable("should be already handled"); return; @@ -7093,7 +7234,7 @@ void CodeCompletionCallbacksImpl::doneParsing() { Lookup.setHaveLParen(false); // Add any keywords that can be used in an argument expr position. - addSuperKeyword(CompletionContext.getResultSink()); + addSuperKeyword(CompletionContext.getResultSink(), CurDeclContext); addExprKeywords(CompletionContext.getResultSink(), CurDeclContext); DoPostfixExprBeginning(); @@ -7195,58 +7336,6 @@ void CodeCompletionCallbacksImpl::doneParsing() { Lookup.addImportModuleNames(); break; } - case CompletionKind::CallArg: { - ExprContextInfo ContextInfo(CurDeclContext, CodeCompleteTokenExpr); - - bool shouldPerformGlobalCompletion = true; - - if (ShouldCompleteCallPatternAfterParen && - !ContextInfo.getPossibleCallees().empty()) { - Lookup.setHaveLParen(true); - for (auto &typeAndDecl : ContextInfo.getPossibleCallees()) { - auto apply = ContextInfo.getAnalyzedExpr(); - if (apply && isa(apply)) { - Lookup.addSubscriptCallPattern( - typeAndDecl.Type, - dyn_cast_or_null(typeAndDecl.Decl), - typeAndDecl.SemanticContext); - } else { - Lookup.addFunctionCallPattern( - typeAndDecl.Type, - dyn_cast_or_null(typeAndDecl.Decl), - typeAndDecl.SemanticContext); - } - } - Lookup.setHaveLParen(false); - - shouldPerformGlobalCompletion = - !Lookup.FoundFunctionCalls || - (Lookup.FoundFunctionCalls && - Lookup.FoundFunctionsWithoutFirstKeyword); - } else if (!ContextInfo.getPossibleParams().empty()) { - auto params = ContextInfo.getPossibleParams(); - Lookup.addCallArgumentCompletionResults(params); - - shouldPerformGlobalCompletion = !ContextInfo.getPossibleTypes().empty(); - // Fallback to global completion if the position is out of number. It's - // better than suggest nothing. - shouldPerformGlobalCompletion |= llvm::all_of( - params, [](const PossibleParamInfo &P) { return !P.Param; }); - } - - if (shouldPerformGlobalCompletion) { - Lookup.setExpectedTypes(ContextInfo.getPossibleTypes(), - ContextInfo.isImplicitSingleExpressionReturn()); - - // Add any keywords that can be used in an argument expr position. - addSuperKeyword(CompletionContext.getResultSink()); - addExprKeywords(CompletionContext.getResultSink(), CurDeclContext); - - DoPostfixExprBeginning(); - } - break; - } - case CompletionKind::LabeledTrailingClosure: { ExprContextInfo ContextInfo(CurDeclContext, CodeCompleteTokenExpr); @@ -7297,7 +7386,7 @@ void CodeCompletionCallbacksImpl::doneParsing() { Context.LangOpts.EnableExperimentalConcurrency, Context.LangOpts.EnableExperimentalDistributed); addStmtKeywords(Sink, CurDeclContext, MaybeFuncBody); - addSuperKeyword(Sink); + addSuperKeyword(Sink, CurDeclContext); addLetVarKeywords(Sink); addExprKeywords(Sink, CurDeclContext); addAnyTypeKeyword(Sink, Context.TheAnyType); diff --git a/lib/IDE/ExprContextAnalysis.h b/lib/IDE/ExprContextAnalysis.h index d672b1a90b74b..cb15b6164f065 100644 --- a/lib/IDE/ExprContextAnalysis.h +++ b/lib/IDE/ExprContextAnalysis.h @@ -73,6 +73,13 @@ struct PossibleParamInfo { assert((Param || !IsRequired) && "nullptr with required flag is not allowed"); }; + + friend bool operator==(const PossibleParamInfo &lhs, + const PossibleParamInfo &rhs) { + if (*lhs.Param != *rhs.Param) + return false; + return lhs.IsRequired == rhs.IsRequired; + } }; /// Given an expression and its decl context, the analyzer tries to figure out diff --git a/lib/Sema/CSBindings.cpp b/lib/Sema/CSBindings.cpp index bcfbe17bb0712..5941415aa97c5 100644 --- a/lib/Sema/CSBindings.cpp +++ b/lib/Sema/CSBindings.cpp @@ -1950,6 +1950,13 @@ bool TypeVariableBinding::attempt(ConstraintSystem &cs) const { auto argLoc = srcLocator->findLast(); if (argLoc && argLoc->isAfterCodeCompletionLoc()) return false; + // Don't penalize solutions with holes corresponding to the completion + // expression itself. This is needed to avoid filtering out keypath + // application subscripts currently. + if (srcLocator->directlyAt() && + dstLocator->directlyAt()) { + return false; + } } // Reflect in the score that this type variable couldn't be // resolved and had to be bound to a placeholder "hole" type. diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index 7fa5c2a3976dd..960e5a48681b2 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -42,6 +42,8 @@ MatchCallArgumentListener::~MatchCallArgumentListener() { } bool MatchCallArgumentListener::extraArgument(unsigned argIdx) { return true; } +bool MatchCallArgumentListener::invalidArgumentsAfterCompletion() { return true; } + Optional MatchCallArgumentListener::missingArgument(unsigned paramIdx, unsigned argInsertIdx) { @@ -167,8 +169,8 @@ static bool areConservativelyCompatibleArgumentLabels( return matchCallArguments(args, params, paramInfo, unlabeledTrailingClosureArgIndex, - /*allow fixes*/ false, listener, - None).hasValue(); + /*completionInfo*/None, /*allow fixes*/ false, + listener, None).hasValue(); } Expr *constraints::getArgumentLabelTargetExpr(Expr *fn) { @@ -253,11 +255,21 @@ static bool anyParameterRequiresArgument( return false; } +static bool isCodeCompletionTypeVar(Type type) { + if (auto *TVT = type->getAs()) { + if (TVT->getImpl().isCodeCompletionToken()) + return true; + } + return false; +} + + static bool matchCallArgumentsImpl( SmallVectorImpl &args, ArrayRef params, const ParameterListInfo ¶mInfo, Optional unlabeledTrailingClosureArgIndex, + Optional completionInfo, bool allowFixes, TrailingClosureMatching trailingClosureMatching, MatchCallArgumentListener &listener, @@ -340,6 +352,16 @@ static bool matchCallArgumentsImpl( if (numClaimedArgs == numArgs) return None; + // If the next arg is a code completion token with no label, and we're using + // the new solver-based code completion (to avoid affecting the AST used by + // unmigrated completion kinds) claim it without complaining about the + // missing label. Completion will suggest the label in such cases. + if (completionInfo && nextArgIdx < numArgs && + args[nextArgIdx].getLabel().empty()) { + if (isCodeCompletionTypeVar(args[nextArgIdx].getPlainType())) + return claim(paramLabel, nextArgIdx, /*ignoreNameMismatch*/true); + } + // Go hunting for an unclaimed argument whose name does match. Optional claimedWithSameName; for (unsigned i = nextArgIdx; i != numArgs; ++i) { @@ -382,6 +404,14 @@ static bool matchCallArgumentsImpl( if (argLabel.empty()) continue; + // We don't allow out-of-order matching beyond the completion location. + // Arguments after the completion position must all be valid for any to + // be bound. If any are invalid we ignore all of them, proceeding as if + // the completion was the last argument, as the user may be re-writing + // the callsite rather than updating an argument in an existing one. + if (completionInfo && completionInfo->isBefore(i)) + continue; + potentiallyOutOfOrder = true; } @@ -393,6 +423,13 @@ static bool matchCallArgumentsImpl( if (!allowFixes) return None; + // Also don't attempt any fixes if this argument is after the completion + // position. Arguments after the completion position must all be valid to + // be considered. If any are invalid we ignore them, as the user may be + // re-writing the callsite rather than updating an argument. + if (completionInfo && completionInfo->isBefore(nextArgIdx)) + return None; + // Several things could have gone wrong here, and we'll check for each // of them at some point: // - The keyword argument might be redundant, in which case we can point @@ -590,6 +627,12 @@ static bool matchCallArgumentsImpl( llvm::SmallVector unclaimedNamedArgs; for (auto argIdx : indices(args)) { if (claimedArgs[argIdx]) continue; + + // We don't allow arguments after the completion position to be claimed + // during recovery, so skip them. + if (completionInfo && completionInfo->isBefore(argIdx)) + continue; + if (!args[argIdx].getLabel().empty()) unclaimedNamedArgs.push_back(argIdx); } @@ -666,6 +709,12 @@ static bool matchCallArgumentsImpl( continue; bindNextParameter(paramIdx, nextArgIdx, true); + + // Don't allow arguments after the completion position to be claimed + // during recovery. We ignore them if they are invalid, as the user may + // may be re-writing the callsite rather than updating an argument. + if (completionInfo && completionInfo->isBefore(nextArgIdx)) + break; } } @@ -679,12 +728,18 @@ static bool matchCallArgumentsImpl( continue; // If parameter has a default value, we don't really - // now if label doesn't match because it's incorrect + // know if label doesn't match because it's incorrect // or argument belongs to some other parameter, so // we just leave this parameter unfulfilled. if (paramInfo.hasDefaultArgument(i)) continue; + // Don't allow arguments after the completion position to be claimed + // during recovery. We ignore them if they are invalid, as the user may + // may be re-writing the callsite rather than updating an argument. + if (completionInfo && completionInfo->isBefore(i)) + continue; + // Looks like there was no parameter claimed at the same // position, it could only mean that label is completely // different, because typo correction has been attempted already. @@ -695,13 +750,71 @@ static bool matchCallArgumentsImpl( // If we still haven't claimed all of the arguments, // fail if there is no recovery. if (numClaimedArgs != numArgs) { + bool unclaimedArgAfterCompletion = false; for (auto index : indices(claimedArgs)) { if (claimedArgs[index]) continue; + // Avoid reporting individual extra arguments after the completion + // position. They are reported via a single callback below. + if (completionInfo && completionInfo->isBefore(index)) { + unclaimedArgAfterCompletion = true; + continue; + } + if (listener.extraArgument(index)) return true; } + + if (unclaimedArgAfterCompletion) { + // There were invalid arguments after the completion position. The + // user may be intending to rewrite the callsite (rather than update an + // argument) so proceed as if all arguments after the completion + // position (even those already bound) weren't present. This prevents + // this solution being penalized based on the exact number and kind + // of issues those arguments contain. + unsigned afterIdx = completionInfo->completionIdx + 1; + + auto argsBegin = args.begin() + afterIdx; + if (argsBegin != args.end()) + args.erase(argsBegin, args.end()); + numArgs = args.size(); + + if (!actualArgNames.empty()) { + auto actualNamesBegin = actualArgNames.begin() + afterIdx; + if (actualNamesBegin != actualArgNames.end()) + actualArgNames.erase(actualNamesBegin, actualArgNames.end()); + } + + auto claimedBegin = claimedArgs.begin() + afterIdx; + if (claimedBegin != claimedArgs.end()) + claimedArgs.erase(claimedBegin, claimedArgs.end()); + numClaimedArgs = llvm::count_if(claimedArgs, [](bool claimed){ + return claimed; + }); + + for (auto ¶ms: llvm::reverse(parameterBindings)) { + if (params.empty()) + continue; + if (!completionInfo->isBefore(params.back())) + break; + if (completionInfo->isBefore(params.front())) { + params.clear(); + haveUnfulfilledParams = true; + } else { + llvm::erase_if(params, [&](unsigned argIdx) { + return completionInfo->isBefore(argIdx); + }); + } + } + + // Report that post-completion args where ignored. This allows us to + // rank overloads that didn't require dropping the args higher, while + // still treating all overloads that did equally, regardless of how + // many args were dropped or what issues they would otherwise trigger. + if (listener.invalidArgumentsAfterCompletion()) + return true; + } } // FIXME: If we had the actual parameters and knew the body names, those @@ -837,6 +950,12 @@ static bool matchCallArgumentsImpl( // - The argument is unnamed, in which case we try to fix the // problem by adding the name. } else if (argumentLabel.empty()) { + // If the argument is the code completion token, the label will be + // offered in the completion results, so it shouldn't be considered + // missing. + Type argTy = args[fromArgIdx].getPlainType(); + if (completionInfo && isCodeCompletionTypeVar(argTy)) + continue; hadLabelMismatch = true; if (listener.missingLabel(paramIdx)) return true; @@ -935,6 +1054,7 @@ constraints::matchCallArguments( ArrayRef params, const ParameterListInfo ¶mInfo, Optional unlabeledTrailingClosureArgIndex, + Optional completionInfo, bool allowFixes, MatchCallArgumentListener &listener, Optional trailingClosureMatching) { @@ -946,7 +1066,7 @@ constraints::matchCallArguments( SmallVector paramBindings; if (matchCallArgumentsImpl( args, params, paramInfo, unlabeledTrailingClosureArgIndex, - allowFixes, scanDirection, listener, paramBindings)) + completionInfo, allowFixes, scanDirection, listener, paramBindings)) return None; return MatchCallArgumentResult{ @@ -968,14 +1088,16 @@ constraints::matchCallArguments( // Try the forward direction first. SmallVector forwardParamBindings; bool forwardFailed = matchCallArgumentsImpl( - args, params, paramInfo, unlabeledTrailingClosureArgIndex, allowFixes, - TrailingClosureMatching::Forward, noOpListener, forwardParamBindings); + args, params, paramInfo, unlabeledTrailingClosureArgIndex, completionInfo, + allowFixes, TrailingClosureMatching::Forward, noOpListener, + forwardParamBindings); // Try the backward direction. SmallVector backwardParamBindings; bool backwardFailed = matchCallArgumentsImpl( - args, params, paramInfo, unlabeledTrailingClosureArgIndex, allowFixes, - TrailingClosureMatching::Backward, noOpListener, backwardParamBindings); + args, params, paramInfo, unlabeledTrailingClosureArgIndex, completionInfo, + allowFixes, TrailingClosureMatching::Backward, noOpListener, + backwardParamBindings); // If at least one of them failed, or they produced the same results, run // call argument matching again with the real visitor. @@ -999,35 +1121,30 @@ constraints::matchCallArguments( }; } -struct CompletionArgInfo { - unsigned completionIdx; - Optional firstTrailingIdx; +bool CompletionArgInfo::allowsMissingArgAt(unsigned argInsertIdx, + AnyFunctionType::Param param) { + // If the argument is before or at the index of the argument containing the + // completion, the user would likely have already written it if they + // intended this overload. + if (completionIdx >= argInsertIdx) + return false; - bool isAllowableMissingArg(unsigned argInsertIdx, - AnyFunctionType::Param param) { - // If the argument is before or at the index of the argument containing the - // completion, the user would likely have already written it if they - // intended this overload. - if (completionIdx >= argInsertIdx) + // If the argument is after the first trailing closure, the user can only + // continue on to write more trailing arguments, so only allow this overload + // if the missing argument is of function type. + if (firstTrailingIdx && argInsertIdx > *firstTrailingIdx) { + if (param.isInOut()) return false; - // If the argument is after the first trailing closure, the user can only - // continue on to write more trailing arguments, so only allow this overload - // if the missing argument is of function type. - if (firstTrailingIdx && argInsertIdx > *firstTrailingIdx) { - if (param.isInOut()) - return false; - - Type expectedTy = param.getPlainType()->lookThroughAllOptionalTypes(); - return expectedTy->is() || expectedTy->isAny() || - expectedTy->isTypeVariableOrMember(); - } - return true; + Type expectedTy = param.getPlainType()->lookThroughAllOptionalTypes(); + return expectedTy->is() || expectedTy->isAny() || + expectedTy->isTypeVariableOrMember(); } -}; + return true; +} -static Optional -getCompletionArgInfo(ASTNode anchor, ConstraintSystem &CS) { +Optional +constraints::getCompletionArgInfo(ASTNode anchor, ConstraintSystem &CS) { Expr *arg = nullptr; if (auto *CE = getAsExpr(anchor)) arg = CE->getArg(); @@ -1042,11 +1159,11 @@ getCompletionArgInfo(ASTNode anchor, ConstraintSystem &CS) { auto trailingIdx = arg->getUnlabeledTrailingClosureIndexOfPackedArgument(); if (auto *PE = dyn_cast(arg)) { if (CS.containsCodeCompletionLoc(PE->getSubExpr())) - return CompletionArgInfo{ 0, trailingIdx }; + return CompletionArgInfo{ 0, trailingIdx, 1}; } else if (auto *TE = dyn_cast(arg)) { for (unsigned i: indices(TE->getElements())) { if (CS.containsCodeCompletionLoc(TE->getElement(i))) - return CompletionArgInfo{ i, trailingIdx }; + return CompletionArgInfo{ i, trailingIdx, TE->getNumElements() }; } } return None; @@ -1093,7 +1210,7 @@ class ArgumentFailureTracker : public MatchCallArgumentListener { if (!CompletionArgInfo) CompletionArgInfo = getCompletionArgInfo(Locator.getAnchor(), CS); isAfterCodeCompletionLoc = CompletionArgInfo && - CompletionArgInfo->isAllowableMissingArg(argInsertIdx, param); + CompletionArgInfo->allowsMissingArgAt(argInsertIdx, param); } auto *argLoc = CS.getConstraintLocator( @@ -1128,6 +1245,15 @@ class ArgumentFailureTracker : public MatchCallArgumentListener { return false; } + bool invalidArgumentsAfterCompletion() override { + assert(CS.isForCodeCompletion()); + // This is only triggered when solving for code completion (which doesn't + // produce diagnostics) so no need to record a fix for this. Still increase + // the score to penalize this solution though. + CS.increaseScore(SK_Fix); + return false; + } + bool missingLabel(unsigned paramIndex) override { return !CS.shouldAttemptFixes(); } @@ -1328,9 +1454,12 @@ ConstraintSystem::TypeMatchResult constraints::matchCallArguments( { ArgumentFailureTracker listener(cs, argsWithLabels, params, locator); + Optional completionInfo = None; + if (cs.isForCodeCompletion()) + completionInfo = getCompletionArgInfo(locator.getAnchor(), cs); auto callArgumentMatch = constraints::matchCallArguments( argsWithLabels, params, paramInfo, - argInfo->UnlabeledTrailingClosureIndex, + argInfo->UnlabeledTrailingClosureIndex, completionInfo, cs.shouldAttemptFixes(), listener, trailingClosureMatching); if (!callArgumentMatch) return cs.getTypeMatchFailure(locator); @@ -7026,7 +7155,8 @@ static bool isForKeyPathSubscriptWithoutLabel(ConstraintSystem &cs, if (auto *SE = getAsExpr(locator->getAnchor())) { auto *indexExpr = SE->getIndex(); return isa(indexExpr) && - isa(indexExpr->getSemanticsProvidingExpr()); + (isa(indexExpr->getSemanticsProvidingExpr()) || + isa(indexExpr->getSemanticsProvidingExpr())); } return false; } @@ -9733,6 +9863,14 @@ ConstraintSystem::simplifyKeyPathApplicationConstraint( return matchTypes(resultTy, valueTy, ConstraintKind::Bind, subflags, locator); } + + if (keyPathTy->isPlaceholder()) { + if (rootTy->hasTypeVariable()) + recordAnyTypeVarAsPotentialHole(rootTy); + if (valueTy->hasTypeVariable()) + recordAnyTypeVarAsPotentialHole(valueTy); + return SolutionKind::Solved; + } if (auto bgt = keyPathTy->getAs()) { // We have the key path type. Match it to the other ends of the constraint. @@ -11270,6 +11408,41 @@ bool ConstraintSystem::recordFix(ConstraintFix *fix, unsigned impact) { log << ")\n"; } + // If this fix is in a call argument after the argument containing the code + // completion location, give a fixed penalty with the same impact as that for + // the fix to ignore all arguments after the completion position when matching + // arguments based on labels. + if (isForCodeCompletion()) { + + auto getArgAndAnchor = [&](ConstraintLocator *loc) -> Optional> { + SmallVector path; + auto anchor = loc->getAnchor(); + if (auto applyArg = loc->findLast()) + return std::make_pair(applyArg->getArgIdx(), anchor); + return None; + }; + auto *loc = fix->getLocator(); + loc->dump(this); + auto completionInfo = getCompletionArgInfo(loc->getAnchor(), *this); + auto argAndAnchor = getArgAndAnchor(loc); + if (completionInfo && argAndAnchor && + completionInfo->isBefore(argAndAnchor->first)) { + // Make sure we only penalize issues in arguments after the argument + // containing the completion position once. + bool isFirst = llvm::all_of(getFixes(), [&](const ConstraintFix *fix) { + if (auto fixArgAndAnchor = getArgAndAnchor(fix->getLocator())) + return !completionInfo->isBefore(fixArgAndAnchor->first) || + fixArgAndAnchor->second != argAndAnchor->second || + fixArgAndAnchor->first > argAndAnchor->first; + return true; + }); + + if (!isFirst) + return false; + impact = 1; + } + } + // Record the fix. // If this is just a warning, it shouldn't affect the solver. Otherwise, diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index db5437fd9403d..a5742d8563f94 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -2888,7 +2888,13 @@ void ConstraintSystem::resolveOverload(ConstraintLocator *locator, refType = subscriptTy; // Increase the score so that actual subscripts get preference. - increaseScore(SK_KeyPathSubscript); + // ...except if we're solving for code completion and the index expression + // contains the completion location + auto SE = getAsExpr(locator->getAnchor()); + if (!isForCodeCompletion() || + (SE && !containsCodeCompletionLoc(SE->getIndex()))) { + increaseScore(SK_KeyPathSubscript); + } break; } } diff --git a/lib/Sema/TypeCheckCodeCompletion.cpp b/lib/Sema/TypeCheckCodeCompletion.cpp index 4706719235849..e761e918661d1 100644 --- a/lib/Sema/TypeCheckCodeCompletion.cpp +++ b/lib/Sema/TypeCheckCodeCompletion.cpp @@ -1165,6 +1165,26 @@ fallbackTypeCheck(DeclContext *DC) { [&](const Solution &S) { sawSolution(S); }); } +void ArgumentTypeCheckCompletionCallback:: +fallbackTypeCheck(DeclContext *DC) { + assert(!gotCallback()); + + CompletionContextFinder finder(DC); + if (!finder.hasCompletionExpr()) + return; + + auto fallback = finder.getFallbackCompletionExpr(); + if (!fallback) + return; + + SolutionApplicationTarget completionTarget(fallback->E, fallback->DC, + CTP_Unused, Type(), + /*isDiscarded=*/true); + TypeChecker::typeCheckForCodeCompletion( + completionTarget, /*needsPrecheck*/true, + [&](const Solution &S) { sawSolution(S); }); +} + static Type getTypeForCompletion(const constraints::Solution &S, Expr *E) { auto &CS = S.getConstraintSystem(); @@ -1359,3 +1379,125 @@ void KeyPathTypeCheckCompletionCallback::sawSolution( Results.push_back({BaseType, /*OnRoot=*/(ComponentIndex == 0)}); } } + +bool ArgumentTypeCheckCompletionCallback::Result::isSubscript() const { + return CallE && isa(CallE); +} + +void ArgumentTypeCheckCompletionCallback:: +sawSolution(const constraints::Solution &S) { + GotCallback = true; + + Type ExpectedTy = getTypeForCompletion(S, CompletionExpr); + if (!ExpectedTy) + return; + + auto &CS = S.getConstraintSystem(); + + Expr *ParentCall = CompletionExpr; + while (ParentCall && + !(isa(ParentCall) || isa(ParentCall))) + ParentCall = CS.getParentExpr(ParentCall); + + if (!ParentCall || ParentCall == CompletionExpr) { + assert(false && "no containing call?"); + return; + } + + auto *Locator = CS.getConstraintLocator(ParentCall); + auto *CalleeLocator = S.getCalleeLocator(Locator); + auto SelectedOverload = S.getOverloadChoiceIfAvailable(CalleeLocator); + if (!SelectedOverload) + return; + + Type BaseTy = SelectedOverload->choice.getBaseType(); + if (BaseTy) + BaseTy = S.simplifyType(BaseTy); + + ValueDecl *FuncD = SelectedOverload->choice.getDeclOrNull(); + Type FuncTy = S.simplifyType(SelectedOverload->openedType); + + // For completion as the arg in a call to the implicit (keypath: _) method the + // solver can't know what kind of keypath is expected (e.g. KeyPath vs + // WritableKeyPath) so it ends up as a hole. Just assume KeyPath so we show + // the root type to users. + if (SelectedOverload->choice.getKind() == OverloadChoiceKind::KeyPathApplication) { + auto Params = FuncTy->getAs()->getParams(); + if (Params.size() == 1 && Params[0].getPlainType()->is()) { + auto *KPDecl = CS.getASTContext().getKeyPathDecl(); + Type KPTy = KPDecl->mapTypeIntoContext(KPDecl->getDeclaredInterfaceType()); + Type KPValueTy = KPTy->castTo()->getGenericArgs()[1]; + KPTy = BoundGenericType::get(KPDecl, Type(), {BaseTy, KPValueTy}); + FuncTy = FunctionType::get({Params[0].withType(KPTy)}, KPValueTy); + } + } + + auto ArgInfo = getCompletionArgInfo(ParentCall, CS); + auto ArgIdx = ArgInfo->completionIdx; + auto Bindings = S.argumentMatchingChoices.find(CS.getConstraintLocator( + Locator, ConstraintLocator::ApplyArgument))->second.parameterBindings; + + // Find the parameter the completion was bound to (if any), as well as which + // parameters are already bound (so we don't suggest them even when the args + // are out of order). + Optional ParamIdx; + SmallVector ClaimedParams; + bool IsNoninitialVariadic = false; + + for (auto i: indices(Bindings)) { + bool Claimed = false; + for (auto j: Bindings[i]) { + if (j == ArgIdx) { + assert(!ParamIdx); + ParamIdx = i; + IsNoninitialVariadic = llvm::any_of(Bindings[i], [j](unsigned other) { + return other < j; + }); + } + // Synthesized args don't count. + if (j < ArgInfo->argCount) + Claimed = true; + } + if (Claimed) + ClaimedParams.push_back(i); + } + + if (!ParamIdx && Bindings.size()) { + if (ClaimedParams.empty()) { + // If no parameters were claimed, assume the completion arg corresponds to + // the first paramter. + assert(!ParamIdx); + ParamIdx = 0; + } else if (ClaimedParams.back() < Bindings.size() - 1) { + // Otherwise assume it's the param after the last claimed parameter. + ParamIdx = ClaimedParams.back() + 1; + IsNoninitialVariadic = Bindings[*ParamIdx].size() > 1; + } + } + + bool HasLabel = false; + Expr *Arg = CS.getParentExpr(CompletionExpr); + if (TupleExpr *TE = dyn_cast(Arg)) + HasLabel = !TE->getElementName(ArgIdx).empty(); + + // If this is a duplicate of any other result, ignore this solution. + if (llvm::any_of(Results, [&](const Result &R) { + if (R.FuncD != FuncD) + return false; + if (!R.FuncTy->isEqual(FuncTy)) + return false; + if (R.BaseType) { + if (!BaseTy || !BaseTy->isEqual(R.BaseType)) + return false; + } else if (BaseTy) { + return false; + } + return R.ParamIdx == ParamIdx && + R.IsNoninitialVariadic == IsNoninitialVariadic; + })) { + return; + } + + Results.push_back({ExpectedTy, ParentCall, FuncD, FuncTy, ArgIdx, ParamIdx, + std::move(ClaimedParams), IsNoninitialVariadic, BaseTy, HasLabel}); +} diff --git a/lib/Sema/TypeCheckConstraints.cpp b/lib/Sema/TypeCheckConstraints.cpp index 01a5a07e596f1..7a9d4c5ca7cc4 100644 --- a/lib/Sema/TypeCheckConstraints.cpp +++ b/lib/Sema/TypeCheckConstraints.cpp @@ -115,6 +115,10 @@ bool TypeVariableType::Implementation::isKeyPathType() const { return locator && locator->isKeyPathType(); } +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..b5f5bf6ca9ca4 100644 --- a/test/IDE/complete_ambiguous.swift +++ b/test/IDE/complete_ambiguous.swift @@ -473,3 +473,88 @@ func testBestSolutionGeneric() { // BEST_SOLUTION_FILTER_GEN-DAG: Keyword[self]/CurrNominal: self[#Test1#]; name=self // BEST_SOLUTION_FILTER_GEN: End completions +func testAmbiguousArgs() { + struct A { + func someFunc(a: Int, b: Int) -> A { return self } + func variadic(y: Int, x: Int...) + } + + struct B { + func someFunc(a: Int, c: String = "", d: String = "") {} + } + + func overloaded() -> A { return A() } + func overloaded() -> B { return B() } + + let localInt = 10 + let localString = "Hello" + + overloaded().someFunc(a: 2, #^ARG_NO_LABEL^#) + + // ARG_NO_LABEL: Begin completions, 3 items + // ARG_NO_LABEL-DAG: Pattern/Local/Flair[ArgLabels]: {#b: Int#}[#Int#]; name=b: Int + // ARG_NO_LABEL-DAG: Pattern/Local/Flair[ArgLabels]: {#c: String#}[#String#]; name=c: String + // ARG_NO_LABEL-DAG: Pattern/Local/Flair[ArgLabels]: {#d: String#}[#String#]; name=d: String + // ARG_NO_LABEL: End completions + + overloaded().someFunc(a: 2, b: #^ARG_LABEL^#) + + // ARG_LABEL: Begin completions + // ARG_LABEL-DAG: Decl[LocalVar]/Local: localString[#String#]; name=localString + // ARG_LABEL-DAG: Decl[LocalVar]/Local/TypeRelation[Identical]: localInt[#Int#]; name=localInt + // ARG_LABEL: End completions + + overloaded().someFunc(a: 2, c: "Foo", #^ARG_NO_LABEL2^#) + + // ARG_NO_LABEL2: Begin completions, 1 item + // ARG_NO_LABEL2: Pattern/Local/Flair[ArgLabels]: {#d: String#}[#String#]; name=d: String + // ARG_NO_LABEL2: End completions + + overloaded().someFunc(a: 2, wrongLabel: "Foo", #^ARG_NO_LABEL_PREV_WRONG^#) + + // ARG_NO_LABEL_PREV_WRONG: Begin completions, 2 items + // ARG_NO_LABEL_PREV_WRONG-DAG: Pattern/Local/Flair[ArgLabels]: {#c: String#}[#String#]; name=c: String + // ARG_NO_LABEL_PREV_WRONG-DAG: Pattern/Local/Flair[ArgLabels]: {#d: String#}[#String#]; name=d: String + // ARG_NO_LABEL_PREV_WRONG: End completions + + overloaded().someFunc(a: 2, d: "Foo", #^ARG_NO_LABEL_OUT_OF_ORDER^#) + // ARG_NO_LABEL_OUT_OF_ORDER: Begin completions + // ARG_NO_LABEL_OUT_OF_ORDER-NOT: name=d: String + // ARG_NO_LABEL_OUT_OF_ORDER: Pattern/Local/Flair[ArgLabels]: {#c: String#}[#String#]; name=c: String + // ARG_NO_LABEL_OUT_OF_ORDER-NOT: name=d: String + // ARG_NO_LABEL_OUT_OF_ORDER: End completions + + func noArgs() {} + noArgs(12, #^ARG_EXTRANEOUS^#) + // ARG_EXTRANEOUS: Begin completions + // ARG_EXTRANEOUS-DAG: localInt + // ARG_EXTRANEOUS-DAG: localString + // ARG_EXTRANEOUS: End completions + + overloaded().someFunc(a: 2, #^LATER_ARGS^#, d: "foo") + // LATER_ARGS: Begin completions, 1 item + // LATER_ARGS: Pattern/Local/Flair[ArgLabels]: {#c: String#}[#String#]; name=c: String + // LATER_ARGS: End completions + + overloaded().someFunc(a: 2, #^LATER_ARGS_WRONG^#, k: 4.5) + // LATER_ARGS_WRONG: Begin completions, 3 items + // LATER_ARGS_WRONG-DAG: Pattern/Local/Flair[ArgLabels]: {#b: Int#}[#Int#]; name=b: Int + // LATER_ARGS_WRONG-DAG: Pattern/Local/Flair[ArgLabels]: {#c: String#}[#String#]; name=c: String + // LATER_ARGS_WRONG-DAG: Pattern/Local/Flair[ArgLabels]: {#d: String#}[#String#]; name=d: String + // LATER_ARGS_WRONG-DAG: End completions + + + overloaded().variadic(y: 2, #^INITIAL_VARARG^#, 4) + // INITIAL_VARARG: Begin completions, 1 item + // INITIAL_VARARG: Pattern/Local/Flair[ArgLabels]: {#x: Int...#}[#Int#]; name=x: Int... + // INITIAL VARARG: End completions + + overloaded().variadic(y: 2, x: 2, #^NONINITIAL_VARARG^#) + // NONINITIAL_VARARG: Begin completions + // NONINITIAL_VARARG-NOT: name=x: + // NONINITIAL_VARARG: Decl[LocalVar]/Local/TypeRelation[Identical]: localInt[#Int#]; name=localInt + // NONINITIAL_VARARG-NOT: name=x: + // NONINITIAL_VARARG: End completions + +} + diff --git a/test/IDE/complete_call_arg.swift b/test/IDE/complete_call_arg.swift index e89d07500ae36..8b7661238831d 100644 --- a/test/IDE/complete_call_arg.swift +++ b/test/IDE/complete_call_arg.swift @@ -180,10 +180,10 @@ class C3 { var C1I = C1() var C2I = C2() func f1() { - foo2(C1I, #^OVERLOAD1?xfail=FIXME^#) + foo2(C1I, #^OVERLOAD1^#) } func f2() { - foo2(C2I, #^OVERLOAD2?xfail=FIXME^#) + foo2(C2I, #^OVERLOAD2^#) } func f3() { foo2(C1I, b1: #^OVERLOAD3^#) @@ -207,11 +207,11 @@ class C3 { } // OVERLOAD1: Begin completions, 1 items -// OVERLOAD1-NEXT: Keyword/ExprSpecific: b1: [#Argument name#]; name=b1: +// OVERLOAD1-NEXT: Pattern/Local/Flair[ArgLabels]: {#b1: C2#}[#C2#]; name=b1: C2 // OVERLOAD1-NEXT: End completions // OVERLOAD2: Begin completions, 1 items -// OVERLOAD2-NEXT: Keyword/ExprSpecific: b2: [#Argument name#]; name=b2: +// OVERLOAD2-NEXT: Pattern/Local/Flair[ArgLabels]: {#b2: C1#}[#C1#]; name=b2: C1 // OVERLOAD2-NEXT: End completions // OVERLOAD3: Begin completions @@ -253,8 +253,8 @@ extension C3 { func f7(obj: C3) { let _ = obj.hasError(#^HASERROR1^# - let _ = obj.hasError(a1: #^HASERROR2^# - let _ = obj.hasError(a1: IC1, #^HASERROR3^# + let _ = obj.hasError(a1: #^HASERROR2?xfail=FIXME^# + let _ = obj.hasError(a1: IC1, #^HASERROR3?xfail=FIXME^# let _ = obj.hasError(a1: IC1, b1: #^HASERROR4^# } } @@ -992,8 +992,10 @@ struct Rdar77867723 { } func test2 { self.fn(eee: .up, #^OVERLOAD_LABEL2^#) -// OVERLOAD_LABEL2: Begin completions, 2 items +// OVERLOAD_LABEL2: Begin completions, 4 items +// OVERLOAD_LABEL2-DAG: Pattern/Local/Flair[ArgLabels]: {#aaa: Horizontal#}[#Horizontal#]; // OVERLOAD_LABEL2-DAG: Pattern/Local/Flair[ArgLabels]: {#bbb: Vertical#}[#Vertical#]; +// OVERLOAD_LABEL2-DAG: Pattern/Local/Flair[ArgLabels]: {#ccc: Vertical#}[#Vertical#]; // OVERLOAD_LABEL2-DAG: Pattern/Local/Flair[ArgLabels]: {#ddd: Horizontal#}[#Horizontal#]; // OVERLOAD_LABEL2: End completions } @@ -1015,3 +1017,95 @@ func test_SR14737() { // GENERIC_INIT_IN_INVALID: End completions } } + +func testArgsAfterCompletion() { + enum A { case a } + enum B { case b } + + let localA = A.a + let localB = B.b + + func overloaded(x: Int, _ first: A, _ second: B) {} + func overloaded(x: Int, _ first: B, _ second: A) {} + + overloaded(x: 1, .#^VALID_UNRESOLVED^#, localB) + + // VALID_UNRESOLVED: Begin completions, 2 items + // VALID_UNRESOLVED-DAG: Decl[EnumElement]/CurrNominal/Flair[ExprSpecific]/TypeRelation[Identical]: a[#A#]; name=a + // VALID_UNRESOLVED-DAG: Decl[InstanceMethod]/CurrNominal/TypeRelation[Invalid]: hash({#(self): A#})[#(into: inout Hasher) -> Void#]; name=hash(self: A) + // VALID_UNRESOLVED: End completions + + overloaded(x: 1, #^VALID_GLOBAL^#, localB) + // VALID_GLOBAL: Begin completions + // VALID_GLOBAL-DAG: Decl[LocalVar]/Local/TypeRelation[Identical]: localA[#A#]; name=localA + // VALID_GLOBAL-DAG: Decl[LocalVar]/Local: localB[#B#]; name=localB + // VALID_GLOBAL: End completions + + func overloadedLabel(x: Int, firstA: A, second: B) {} + func overloadedLabel(x: Int, firstB: B, second: A) {} + + overloadedLabel(x: 1, #^VALID_LABEL^#, second: localB) + // VALID_LABEL: Begin completions, 1 items + // VALID_LABEL: Pattern/Local/Flair[ArgLabels]: {#firstA: A#}[#A#]; name=firstA: A + // VALID_LABEL: End completions + + overloadedLabel(x: 1, #^INVALID_LABEL^#, wrongLabelRightType: localB) + // INVALID_LABEL: Begin completions, 2 items + // INVALID_LABEL-DAG: Pattern/Local/Flair[ArgLabels]: {#firstA: A#}[#A#]; name=firstA: A + // INVALID_LABEL-DAG: Pattern/Local/Flair[ArgLabels]: {#firstB: B#}[#B#]; name=firstB: B + // INVALID_LABEL: End completions + + overloadedLabel(x: 1, #^INVALID_LABEL_TYPE?check=INVALID_LABEL^#, wrongLabelWrongType: 2) // invalid + + func overloadedArity(x: Int, firstA: A, second: B, third: Double) {} + func overloadedArity(x: Int, firstB: B, second: A) {} + + overloadedArity(x: 1, #^VALID_ARITY^#, second: localB, third: 4.5) + // VALID_ARITY: Begin completions, 1 items + // VALID_ARITY: Pattern/Local/Flair[ArgLabels]: {#firstA: A#}[#A#]; name=firstA: A + // VALID_ARITY: End completions + + overloadedArity(x: 1, #^INVALID_ARITY^#, wrong: localB) + // INVALID_ARITY: Begin completions, 2 items + // INVALID_ARITY-DAG: Pattern/Local/Flair[ArgLabels]: {#firstA: A#}[#A#]; name=firstA: A + // INVALID_ARITY-DAG: Pattern/Local/Flair[ArgLabels]: {#firstB: B#}[#B#]; name=firstB: B + // INVALID_ARITY: End completions + + // type mismatch in 'second' vs extra arg 'third'. + overloadedArity(x: 1, #^INVALID_ARITY_TYPE?check=INVALID_ARITY^#, second: localA, third: 2.5) + overloadedArity(x: 1, #^INVALID_ARITY_TYPE_2?check=INVALID_ARITY^#, second: localA, third: "wrong") + + func overloadedDefaulted(x: Int, p: A) {} + func overloadedDefaulted(x: Int, y: A = A.a, z: A = A.a) {} + + overloadedDefaulted(x: 1, #^VALID_DEFAULTED^#) + // VALID_DEFAULTED: Begin completions, 3 items + // VALID_DEFAULTED-DAG: Pattern/Local/Flair[ArgLabels]: {#p: A#}[#A#]; name=p: A + // VALID_DEFAULTED-DAG: Pattern/Local/Flair[ArgLabels]: {#y: A#}[#A#]; name=y: A + // VALID_DEFAULTED-DAG: Pattern/Local/Flair[ArgLabels]: {#z: A#}[#A#]; name=z: A + // VALID_DEFAULTED: End completions + + overloadedDefaulted(x: 1, #^VALID_DEFAULTED_AFTER^#, z: localA) + // VALID_DEFAULTED_AFTER: Begin completions, 1 items + // VALID_DEFAULTED_AFTER-DAG: Pattern/Local/Flair[ArgLabels]: {#y: A#}[#A#]; name=y: A + // VALID_DEFAULTED_AFTER: End completions + + overloadedDefaulted(x: 1, #^INVALID_DEFAULTED?check=VALID_DEFAULTED^#, w: "hello") + overloadedDefaulted(x: 1, #^INVALID_DEFAULTED_TYPO?check=VALID_DEFAULTED^#, zz: localA) + overloadedDefaulted(x: 1, #^INVALID_DEFAULTED_TYPO_TYPE?check=VALID_DEFAULTED^#, zz: "hello") + + overloadedDefaulted(x: 1, #^INVALID_DEFAULTED_TYPE^#, z: localB) + // INVALID_DEFAULTED_TYPE: Begin completions, 2 items + // INVALID_DEFAULTED_TYPE-DAG: Pattern/Local/Flair[ArgLabels]: {#p: A#}[#A#]; name=p: A + // INVALID_DEFAULTED_TYPE-DAG: Pattern/Local/Flair[ArgLabels]: {#y: A#}[#A#]; name=y: A + // INVALID_DEFAULTED_TYPE: End completions + + func overloadedGeneric(x: Int, y: String, z: T, y: T) {} + func overloadedGeneric(x: Int, p: String, q: Int) {} + struct MissingConformance {} + overloadedGeneric(x: 1, #^INVALID_MISSINGCONFORMANCE^#, z: MissingConformance(), y: MissingConformance()) + // FIXME: This should also suggest y. + // INVALID_MISSINGCONFORMANCE: Begin completions, 1 item + // INVALID_MISSINGCONFORMANCE: Pattern/Local/Flair[ArgLabels]: {#p: String#}[#String#]; name=p: String + // INVALID_MISSINGCONFORMANCE: End completions +} diff --git a/test/IDE/complete_subscript.swift b/test/IDE/complete_subscript.swift index 0e4c527fad4c5..1539a3119ef09 100644 --- a/test/IDE/complete_subscript.swift +++ b/test/IDE/complete_subscript.swift @@ -63,7 +63,7 @@ func test1() { let _ = MyStruct()[#^INSTANCE_INT_BRACKET^# // INSTANCE_INT_BRACKET: Begin completions // INSTANCE_INT_BRACKET-DAG: Decl[Subscript]/CurrNominal/Flair[ArgLabels]: ['[']{#(x): Int#}, {#instance: Int#}[']'][#Int#]; -// INSTANCE_INT_BRACKET-DAG: Pattern/CurrModule/Flair[ArgLabels]: ['[']{#keyPath: KeyPath, Value>#}[']'][#Value#]; +// INSTANCE_INT_BRACKET-DAG: Pattern/CurrNominal/Flair[ArgLabels]: ['[']{#keyPath: KeyPath, Value>#}[']'][#Value#]; // INSTANCE_INT_BRACKET: End completions } func test2(value: MyStruct) { @@ -89,7 +89,7 @@ func test2(value: MyStruct) { let _ = value[#^INSTANCE_ARCHETYPE_BRACKET^# // INSTANCE_ARCHETYPE_BRACKET: Begin completions // INSTANCE_ARCHETYPE_BRACKET-DAG: Decl[Subscript]/CurrNominal/Flair[ArgLabels]: ['[']{#(x): Int#}, {#instance: U#}[']'][#Int#]; -// INSTANCE_ARCHETYPE_BRACKET-DAG: Pattern/CurrModule/Flair[ArgLabels]: ['[']{#keyPath: KeyPath, Value>#}[']'][#Value#]; +// INSTANCE_ARCHETYPE_BRACKET-DAG: Pattern/CurrNominal/Flair[ArgLabels]: ['[']{#keyPath: KeyPath, Value>#}[']'][#Value#]; // INSTANCE_ARCHETYPE_BRACKET: End completions let _ = MyStruct[42, #^METATYPE_LABEL^# @@ -116,26 +116,28 @@ class Derived: Base { // SELF_IN_INSTANCEMETHOD: Begin completions, 3 items // SELF_IN_INSTANCEMETHOD-DAG: Decl[Subscript]/CurrNominal/Flair[ArgLabels]: ['[']{#derivedInstance: Int#}[']'][#Int#]; // SELF_IN_INSTANCEMETHOD-DAG: Decl[Subscript]/Super/Flair[ArgLabels]: ['[']{#instance: Int#}[']'][#Int#]; -// SELF_IN_INSTANCEMETHOD-DAG: Pattern/CurrModule/Flair[ArgLabels]: ['[']{#keyPath: KeyPath#}[']'][#Value#]; +// SELF_IN_INSTANCEMETHOD-DAG: Pattern/CurrNominal/Flair[ArgLabels]: ['[']{#keyPath: KeyPath#}[']'][#Value#]; // SELF_IN_INSTANCEMETHOD: End completions let _ = super[#^SUPER_IN_INSTANCEMETHOD^#] // SUPER_IN_INSTANCEMETHOD: Begin completions, 2 items // SUPER_IN_INSTANCEMETHOD-DAG: Decl[Subscript]/CurrNominal/Flair[ArgLabels]: ['[']{#instance: Int#}[']'][#Int#]; -// SUPER_IN_INSTANCEMETHOD-DAG: Pattern/CurrModule/Flair[ArgLabels]: ['[']{#keyPath: KeyPath#}[']'][#Value#]; +// SUPER_IN_INSTANCEMETHOD-DAG: Pattern/CurrNominal/Flair[ArgLabels]: ['[']{#keyPath: KeyPath#}[']'][#Value#]; // SUPER_IN_INSTANCEMETHOD: End completions } static func testStatic() { let _ = self[#^SELF_IN_STATICMETHOD^#] -// SELF_IN_STATICMETHOD: Begin completions, 2 items +// SELF_IN_STATICMETHOD: Begin completions, 3 items // SELF_IN_STATICMETHOD-DAG: Decl[Subscript]/CurrNominal/Flair[ArgLabels]: ['[']{#derivedStatic: Int#}[']'][#Int#]; // SELF_IN_STATICMETHOD-DAG: Decl[Subscript]/Super/Flair[ArgLabels]: ['[']{#static: Int#}[']'][#Int#]; +// SELF_IN_STATICMETHOD-DAG: Pattern/CurrNominal/Flair[ArgLabels]: ['[']{#keyPath: KeyPath#}[']'][#Value#]; // SELF_IN_STATICMETHOD: End completions let _ = super[#^SUPER_IN_STATICMETHOD^#] -// SUPER_IN_STATICMETHOD: Begin completions, 1 items +// SUPER_IN_STATICMETHOD: Begin completions, 2 items // SUPER_IN_STATICMETHOD-DAG: Decl[Subscript]/CurrNominal/Flair[ArgLabels]: ['[']{#static: Int#}[']'][#Int#]; +// SUPER_IN_STATICMETHOD-DAG: Pattern/CurrNominal/Flair[ArgLabels]: ['[']{#keyPath: KeyPath#}[']'][#Value#]; // SUPER_IN_STATICMETHOD: End completions } } @@ -147,13 +149,13 @@ func testSubscriptCallSig(val: MyStruct1) { val[#^LABELED_SUBSCRIPT^# // LABELED_SUBSCRIPT: Begin completions, 2 items // LABELED_SUBSCRIPT-DAG: Decl[Subscript]/CurrNominal/Flair[ArgLabels]: ['[']{#idx1: Int#}, {#idx2: Comparable#}[']'][#Int!#]; -// LABELED_SUBSCRIPT-DAG: Pattern/CurrModule/Flair[ArgLabels]: ['[']{#keyPath: KeyPath, Value>#}[']'][#Value#]; +// LABELED_SUBSCRIPT-DAG: Pattern/CurrNominal/Flair[ArgLabels]: ['[']{#keyPath: KeyPath, Value>#}[']'][#Value#]; // LABELED_SUBSCRIPT: End completions } func testSubcscriptTuple(val: (x: Int, String)) { val[#^TUPLE^#] // TUPLE: Begin completions, 1 items -// TUPLE-DAG: Pattern/CurrModule/Flair[ArgLabels]: ['[']{#keyPath: KeyPath<(x: Int, String), Value>#}[']'][#Value#]; +// TUPLE-DAG: Pattern/CurrNominal/Flair[ArgLabels]: ['[']{#keyPath: KeyPath<(x: Int, String), Value>#}[']'][#Value#]; // TUPLE: End completions } From 24b22dcae9040168d84ca5875db717e340c6aa84 Mon Sep 17 00:00:00 2001 From: Nathan Hawes Date: Tue, 27 Jul 2021 13:53:26 +1000 Subject: [PATCH 2/7] test fixes after rebase --- test/IDE/complete_call_arg.swift | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/test/IDE/complete_call_arg.swift b/test/IDE/complete_call_arg.swift index 8b7661238831d..be80403915fc8 100644 --- a/test/IDE/complete_call_arg.swift +++ b/test/IDE/complete_call_arg.swift @@ -207,11 +207,11 @@ class C3 { } // OVERLOAD1: Begin completions, 1 items -// OVERLOAD1-NEXT: Pattern/Local/Flair[ArgLabels]: {#b1: C2#}[#C2#]; name=b1: C2 +// OVERLOAD1-NEXT: Pattern/Local/Flair[ArgLabels]: {#b1: C2#}[#C2#]; name=b1: // OVERLOAD1-NEXT: End completions // OVERLOAD2: Begin completions, 1 items -// OVERLOAD2-NEXT: Pattern/Local/Flair[ArgLabels]: {#b2: C1#}[#C1#]; name=b2: C1 +// OVERLOAD2-NEXT: Pattern/Local/Flair[ArgLabels]: {#b2: C1#}[#C1#]; name=b2: // OVERLOAD2-NEXT: End completions // OVERLOAD3: Begin completions @@ -1032,7 +1032,7 @@ func testArgsAfterCompletion() { // VALID_UNRESOLVED: Begin completions, 2 items // VALID_UNRESOLVED-DAG: Decl[EnumElement]/CurrNominal/Flair[ExprSpecific]/TypeRelation[Identical]: a[#A#]; name=a - // VALID_UNRESOLVED-DAG: Decl[InstanceMethod]/CurrNominal/TypeRelation[Invalid]: hash({#(self): A#})[#(into: inout Hasher) -> Void#]; name=hash(self: A) + // VALID_UNRESOLVED-DAG: Decl[InstanceMethod]/CurrNominal/TypeRelation[Invalid]: hash({#(self): A#})[#(into: inout Hasher) -> Void#]; name=hash(:) // VALID_UNRESOLVED: End completions overloaded(x: 1, #^VALID_GLOBAL^#, localB) @@ -1046,13 +1046,13 @@ func testArgsAfterCompletion() { overloadedLabel(x: 1, #^VALID_LABEL^#, second: localB) // VALID_LABEL: Begin completions, 1 items - // VALID_LABEL: Pattern/Local/Flair[ArgLabels]: {#firstA: A#}[#A#]; name=firstA: A + // VALID_LABEL: Pattern/Local/Flair[ArgLabels]: {#firstA: A#}[#A#]; name=firstA: // VALID_LABEL: End completions overloadedLabel(x: 1, #^INVALID_LABEL^#, wrongLabelRightType: localB) // INVALID_LABEL: Begin completions, 2 items - // INVALID_LABEL-DAG: Pattern/Local/Flair[ArgLabels]: {#firstA: A#}[#A#]; name=firstA: A - // INVALID_LABEL-DAG: Pattern/Local/Flair[ArgLabels]: {#firstB: B#}[#B#]; name=firstB: B + // INVALID_LABEL-DAG: Pattern/Local/Flair[ArgLabels]: {#firstA: A#}[#A#]; name=firstA: + // INVALID_LABEL-DAG: Pattern/Local/Flair[ArgLabels]: {#firstB: B#}[#B#]; name=firstB: // INVALID_LABEL: End completions overloadedLabel(x: 1, #^INVALID_LABEL_TYPE?check=INVALID_LABEL^#, wrongLabelWrongType: 2) // invalid @@ -1062,13 +1062,13 @@ func testArgsAfterCompletion() { overloadedArity(x: 1, #^VALID_ARITY^#, second: localB, third: 4.5) // VALID_ARITY: Begin completions, 1 items - // VALID_ARITY: Pattern/Local/Flair[ArgLabels]: {#firstA: A#}[#A#]; name=firstA: A + // VALID_ARITY: Pattern/Local/Flair[ArgLabels]: {#firstA: A#}[#A#]; name=firstA: // VALID_ARITY: End completions overloadedArity(x: 1, #^INVALID_ARITY^#, wrong: localB) // INVALID_ARITY: Begin completions, 2 items - // INVALID_ARITY-DAG: Pattern/Local/Flair[ArgLabels]: {#firstA: A#}[#A#]; name=firstA: A - // INVALID_ARITY-DAG: Pattern/Local/Flair[ArgLabels]: {#firstB: B#}[#B#]; name=firstB: B + // INVALID_ARITY-DAG: Pattern/Local/Flair[ArgLabels]: {#firstA: A#}[#A#]; name=firstA: + // INVALID_ARITY-DAG: Pattern/Local/Flair[ArgLabels]: {#firstB: B#}[#B#]; name=firstB: // INVALID_ARITY: End completions // type mismatch in 'second' vs extra arg 'third'. @@ -1080,14 +1080,14 @@ func testArgsAfterCompletion() { overloadedDefaulted(x: 1, #^VALID_DEFAULTED^#) // VALID_DEFAULTED: Begin completions, 3 items - // VALID_DEFAULTED-DAG: Pattern/Local/Flair[ArgLabels]: {#p: A#}[#A#]; name=p: A - // VALID_DEFAULTED-DAG: Pattern/Local/Flair[ArgLabels]: {#y: A#}[#A#]; name=y: A - // VALID_DEFAULTED-DAG: Pattern/Local/Flair[ArgLabels]: {#z: A#}[#A#]; name=z: A + // VALID_DEFAULTED-DAG: Pattern/Local/Flair[ArgLabels]: {#p: A#}[#A#]; name=p: + // VALID_DEFAULTED-DAG: Pattern/Local/Flair[ArgLabels]: {#y: A#}[#A#]; name=y: + // VALID_DEFAULTED-DAG: Pattern/Local/Flair[ArgLabels]: {#z: A#}[#A#]; name=z: // VALID_DEFAULTED: End completions overloadedDefaulted(x: 1, #^VALID_DEFAULTED_AFTER^#, z: localA) // VALID_DEFAULTED_AFTER: Begin completions, 1 items - // VALID_DEFAULTED_AFTER-DAG: Pattern/Local/Flair[ArgLabels]: {#y: A#}[#A#]; name=y: A + // VALID_DEFAULTED_AFTER-DAG: Pattern/Local/Flair[ArgLabels]: {#y: A#}[#A#]; name=y: // VALID_DEFAULTED_AFTER: End completions overloadedDefaulted(x: 1, #^INVALID_DEFAULTED?check=VALID_DEFAULTED^#, w: "hello") @@ -1096,8 +1096,8 @@ func testArgsAfterCompletion() { overloadedDefaulted(x: 1, #^INVALID_DEFAULTED_TYPE^#, z: localB) // INVALID_DEFAULTED_TYPE: Begin completions, 2 items - // INVALID_DEFAULTED_TYPE-DAG: Pattern/Local/Flair[ArgLabels]: {#p: A#}[#A#]; name=p: A - // INVALID_DEFAULTED_TYPE-DAG: Pattern/Local/Flair[ArgLabels]: {#y: A#}[#A#]; name=y: A + // INVALID_DEFAULTED_TYPE-DAG: Pattern/Local/Flair[ArgLabels]: {#p: A#}[#A#]; name=p: + // INVALID_DEFAULTED_TYPE-DAG: Pattern/Local/Flair[ArgLabels]: {#y: A#}[#A#]; name=y: // INVALID_DEFAULTED_TYPE: End completions func overloadedGeneric(x: Int, y: String, z: T, y: T) {} @@ -1106,6 +1106,6 @@ func testArgsAfterCompletion() { overloadedGeneric(x: 1, #^INVALID_MISSINGCONFORMANCE^#, z: MissingConformance(), y: MissingConformance()) // FIXME: This should also suggest y. // INVALID_MISSINGCONFORMANCE: Begin completions, 1 item - // INVALID_MISSINGCONFORMANCE: Pattern/Local/Flair[ArgLabels]: {#p: String#}[#String#]; name=p: String + // INVALID_MISSINGCONFORMANCE: Pattern/Local/Flair[ArgLabels]: {#p: String#}[#String#]; name=p: // INVALID_MISSINGCONFORMANCE: End completions } From af73580f081dcdee2a3ae73187b21457a948b5a4 Mon Sep 17 00:00:00 2001 From: Nathan Hawes Date: Thu, 29 Jul 2021 17:31:43 +1000 Subject: [PATCH 3/7] fixups --- include/swift/Sema/CSFix.h | 26 ++- .../swift/Sema/CodeCompletionTypeChecking.h | 58 ++--- include/swift/Sema/ConstraintSystem.h | 16 +- lib/IDE/CodeCompletion.cpp | 30 +-- lib/Sema/CSFix.cpp | 6 + lib/Sema/CSSimplify.cpp | 214 ++++++++---------- lib/Sema/CSSolver.cpp | 4 +- lib/Sema/ConstraintSystem.cpp | 4 +- lib/Sema/TypeCheckCodeCompletion.cpp | 89 +++----- lib/Sema/TypeCheckConstraints.cpp | 4 +- test/IDE/complete_ambiguous.swift | 27 +-- test/IDE/complete_call_arg.swift | 27 ++- 12 files changed, 254 insertions(+), 251 deletions(-) diff --git a/include/swift/Sema/CSFix.h b/include/swift/Sema/CSFix.h index b92854294a09f..5ff848037ba9c 100644 --- a/include/swift/Sema/CSFix.h +++ b/include/swift/Sema/CSFix.h @@ -331,7 +331,12 @@ enum class FixKind : uint8_t { /// Specify a type for an explicitly written placeholder that could not be /// resolved. - SpecifyTypeForPlaceholder + SpecifyTypeForPlaceholder, + + /// Ignore all failures in call or subscript arguments after the argument + /// containing the code completion location. This is only produced when + /// solving for code completion. + IgnoreArgumentsAfterCompletion, }; class ConstraintFix { @@ -1731,6 +1736,25 @@ class SkipUnhandledConstructInResultBuilder final : public ConstraintFix { NominalTypeDecl *builder, ConstraintLocator *locator); }; +class IgnoreArgumentsAfterCompletion final : public ConstraintFix { + IgnoreArgumentsAfterCompletion(ConstraintSystem &cs, + ConstraintLocator *locator) + : ConstraintFix(cs, FixKind::IgnoreArgumentsAfterCompletion, locator) {} + +public: + std::string getName() const override { + return "ignore arguments after the code completion position in a call or subscript"; + } + + bool diagnose(const Solution &solution, bool asNote = false) const override { + // Diagnostics are ignored when solving for code completion. + return false; + } + + static IgnoreArgumentsAfterCompletion * + create(ConstraintSystem &cs, ConstraintLocator *locator); +}; + class AllowTupleSplatForSingleParameter final : public ConstraintFix { using Param = AnyFunctionType::Param; diff --git a/include/swift/Sema/CodeCompletionTypeChecking.h b/include/swift/Sema/CodeCompletionTypeChecking.h index 949acc18ec225..6a9550f29db8e 100644 --- a/include/swift/Sema/CodeCompletionTypeChecking.h +++ b/include/swift/Sema/CodeCompletionTypeChecking.h @@ -35,11 +35,23 @@ namespace swift { } class TypeCheckCompletionCallback { + bool GotCallback = false; public: + virtual ~TypeCheckCompletionCallback() {} + /// Called for each solution produced while type-checking an expression /// that the code completion expression participates in. - virtual void sawSolution(const constraints::Solution &solution) = 0; - virtual ~TypeCheckCompletionCallback() {} + virtual void sawSolution(const constraints::Solution &solution) { + GotCallback = true; + }; + + /// 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); }; @@ -58,28 +70,21 @@ namespace swift { }; private: - DeclContext *DC; CodeCompletionExpr *CompletionExpr; SmallVector Results; llvm::DenseMap, size_t> BaseToSolutionIdx; - bool GotCallback = false; public: - DotExprTypeCheckCompletionCallback(DeclContext *DC, - CodeCompletionExpr *CompletionExpr) - : DC(DC), CompletionExpr(CompletionExpr) {} + DotExprTypeCheckCompletionCallback(CodeCompletionExpr *CompletionExpr) + : CompletionExpr(CompletionExpr) {} /// Get the results collected from any sawSolutions() callbacks recevied so /// far. ArrayRef getResults() const { return Results; } - /// True if at least one solution was passed via the \c sawSolution - /// callback. - bool gotCallback() const { return GotCallback; } - /// Typecheck the code completion expression in isolation, calling /// \c sawSolution for each solution formed. - void fallbackTypeCheck(); + void fallbackTypeCheck(DeclContext *DC) override; void sawSolution(const constraints::Solution &solution) override; }; @@ -97,7 +102,6 @@ namespace swift { private: CodeCompletionExpr *CompletionExpr; SmallVector Results; - bool GotCallback = false; public: UnresolvedMemberTypeCheckCompletionCallback(CodeCompletionExpr *CompletionExpr) @@ -105,14 +109,6 @@ namespace swift { ArrayRef getResults() const { return Results; } - /// 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. - void fallbackTypeCheck(DeclContext *DC); - void sawSolution(const constraints::Solution &solution) override; }; @@ -145,10 +141,10 @@ class ArgumentTypeCheckCompletionCallback: public TypeCheckCompletionCallback { struct Result { /// The type associated with the code completion expression itself. Type ExpectedType; - /// The innermost call expression containing the completion location. - Expr *CallE; + /// True if this is a subscript rather than a function call. + bool IsSubscript; /// The FuncDecl or SubscriptDecl associated with the call. - Decl *FuncD; + ValueDecl *FuncD; /// The type of the function being called. Type FuncTy; /// The index of the argument containing the completion location @@ -159,19 +155,15 @@ class ArgumentTypeCheckCompletionCallback: public TypeCheckCompletionCallback { SmallVector ClaimedParamIndices; /// True if the completion is a noninitial term in a variadic argument. bool IsNoninitialVariadic; - /// The base type of the call. + /// The base type of the call/subscript (null for free functions). Type BaseType; /// True if an argument label precedes the completion location. bool HasLabel; - - /// \returns true if the code completion is within a subscript. - bool isSubscript() const; }; private: CodeCompletionExpr *CompletionExpr; SmallVector Results; - bool GotCallback = false; public: ArgumentTypeCheckCompletionCallback(CodeCompletionExpr *CompletionExpr) @@ -179,14 +171,6 @@ class ArgumentTypeCheckCompletionCallback: public TypeCheckCompletionCallback { ArrayRef getResults() const { return Results; } - /// 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. - void fallbackTypeCheck(DeclContext *DC); - void sawSolution(const constraints::Solution &solution) override; }; diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 3dd927966658e..415f5b4159ed9 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -124,6 +124,9 @@ class SavedTypeVariableBinding { /// The parent or fixed type. llvm::PointerUnion ParentOrFixed; + /// The locator associated with bound type + ConstraintLocator *BoundLocator; + public: explicit SavedTypeVariableBinding(TypeVariableType *typeVar); @@ -239,6 +242,8 @@ class TypeVariableType::Implementation { /// type is bound. llvm::PointerUnion ParentOrFixed; + constraints::ConstraintLocator *BoundLocator = nullptr; + /// The corresponding node in the constraint graph. constraints::ConstraintGraphNode *GraphNode = nullptr; @@ -338,6 +343,12 @@ class TypeVariableType::Implementation { return locator; } + /// Retrieve the locator describing where the type bound to this type variable + /// originated. + constraints::ConstraintLocator *getBoundLocator() const { + return BoundLocator; + } + /// Retrieve the generic parameter opened by this type variable. GenericTypeParamType *getGenericParameter() const; @@ -465,6 +476,7 @@ class TypeVariableType::Implementation { /// Assign a fixed type to this equivalence class. void assignFixedType(Type type, + constraints::ConstraintLocator *loc, constraints::SavedTypeVariableBindings *record) { assert((!getFixedType(0) || getFixedType(0)->isEqual(type)) && "Already has a fixed type!"); @@ -472,6 +484,7 @@ class TypeVariableType::Implementation { if (record) rep->getImpl().recordBinding(*record); rep->getImpl().ParentOrFixed = type.getPointer(); + rep->getImpl().BoundLocator = loc; } void setCanBindToLValue(constraints::SavedTypeVariableBindings *record, @@ -3908,6 +3921,7 @@ class ConstraintSystem { /// \param notifyBindingInference Whether to notify binding inference about /// the change to this type variable. void assignFixedType(TypeVariableType *typeVar, Type type, + ConstraintLocator *loc, bool updateState = true, bool notifyBindingInference = true); @@ -5341,7 +5355,7 @@ struct CompletionArgInfo { Optional firstTrailingIdx; unsigned argCount; - /// \returns true if the given argument index is possibly about to be written + /// \returns true if the given argument index is possibly about to be written /// by the user (given the completion index) so shouldn't be penalised as /// missing when ranking solutions. bool allowsMissingArgAt(unsigned argInsertIdx, AnyFunctionType::Param param); diff --git a/lib/IDE/CodeCompletion.cpp b/lib/IDE/CodeCompletion.cpp index 4968fed494fe3..e20473acd4ccb 100644 --- a/lib/IDE/CodeCompletion.cpp +++ b/lib/IDE/CodeCompletion.cpp @@ -6753,6 +6753,15 @@ static void deliverCompletionResults(CodeCompletionContext &CompletionContext, static bool addPossibleParams( const ArgumentTypeCheckCompletionCallback::Result &Ret, SmallVectorImpl &Params, SmallVectorImpl &Types) { + + if (!Ret.ParamIdx) + return true; + + if (Ret.HasLabel) { + Types.push_back(Ret.ExpectedType); + return true; + } + ArrayRef ParamsToPass = Ret.FuncTy->getAs()->getParams(); ParameterList *PL = nullptr; @@ -6767,15 +6776,7 @@ static bool addPossibleParams( } assert(PL->size() == ParamsToPass.size()); - if (!Ret.ParamIdx) - return true; - - if (Ret.HasLabel) { - Types.push_back(Ret.ExpectedType); - return true; - } - - bool showGlobalCompletions = false; + bool ShowGlobalCompletions = false; for (auto Idx: range(*Ret.ParamIdx, PL->size())) { bool IsCompletion = Idx == Ret.ParamIdx; @@ -6791,13 +6792,13 @@ static bool addPossibleParams( if (!llvm::any_of(Params, [&](PossibleParamInfo &Existing){ return PP == Existing; })) Params.push_back(std::move(PP)); } else { - showGlobalCompletions = true; + ShowGlobalCompletions = true; Types.push_back(P->getPlainType()); } if (Required) break; } - return showGlobalCompletions; + return ShowGlobalCompletions; } static void deliverArgumentResults( @@ -6829,7 +6830,7 @@ static void deliverArgumentResults( SemanticContext = SemanticContextKind::CurrentNominal; } } - if (Result.isSubscript()) { + if (Result.IsSubscript) { assert(SemanticContext != SemanticContextKind::None && BaseTy); auto *SD = dyn_cast_or_null(Result.FuncD); Lookup.addSubscriptCallPattern(Result.FuncTy->getAs(), @@ -6982,8 +6983,7 @@ bool CodeCompletionCallbacksImpl::trySolverCompletion(bool MaybeFuncBody) { assert(CodeCompleteTokenExpr); assert(CurDeclContext); - DotExprTypeCheckCompletionCallback Lookup(CurDeclContext, - CodeCompleteTokenExpr); + DotExprTypeCheckCompletionCallback Lookup(CodeCompleteTokenExpr); llvm::SaveAndRestore CompletionCollector(Context.CompletionCallback, &Lookup); typeCheckContextAt(CurDeclContext, CompletionLoc); @@ -6994,7 +6994,7 @@ bool CodeCompletionCallbacksImpl::trySolverCompletion(bool MaybeFuncBody) { // typechecking still resolve even these cases would be beneficial for // tooling in general though. if (!Lookup.gotCallback()) - Lookup.fallbackTypeCheck(); + Lookup.fallbackTypeCheck(CurDeclContext); addKeywords(CompletionContext.getResultSink(), MaybeFuncBody); diff --git a/lib/Sema/CSFix.cpp b/lib/Sema/CSFix.cpp index 9cb05be1f70c3..01b43f9cd2b2a 100644 --- a/lib/Sema/CSFix.cpp +++ b/lib/Sema/CSFix.cpp @@ -1182,6 +1182,12 @@ bool SkipUnhandledConstructInResultBuilder::diagnose(const Solution &solution, return failure.diagnose(asNote); } +IgnoreArgumentsAfterCompletion * +IgnoreArgumentsAfterCompletion:: create(ConstraintSystem &cs, + ConstraintLocator *locator) { + return new (cs.getAllocator()) IgnoreArgumentsAfterCompletion(cs, locator); +} + bool AllowMutatingMemberOnRValueBase::diagnose(const Solution &solution, bool asNote) const { MutatingMemberRefOnImmutableBase failure(solution, getMember(), getLocator()); diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index 960e5a48681b2..c6b176ea27744 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -352,20 +352,12 @@ static bool matchCallArgumentsImpl( if (numClaimedArgs == numArgs) return None; - // If the next arg is a code completion token with no label, and we're using - // the new solver-based code completion (to avoid affecting the AST used by - // unmigrated completion kinds) claim it without complaining about the - // missing label. Completion will suggest the label in such cases. - if (completionInfo && nextArgIdx < numArgs && - args[nextArgIdx].getLabel().empty()) { - if (isCodeCompletionTypeVar(args[nextArgIdx].getPlainType())) - return claim(paramLabel, nextArgIdx, /*ignoreNameMismatch*/true); - } - // Go hunting for an unclaimed argument whose name does match. Optional claimedWithSameName; + unsigned firstArgIdx = nextArgIdx; for (unsigned i = nextArgIdx; i != numArgs; ++i) { auto argLabel = args[i].getLabel(); + bool claimIgnoringNameMismatch = false; if (argLabel != paramLabel && !(argLabel.str().startswith("$") && @@ -375,9 +367,22 @@ static bool matchCallArgumentsImpl( if (forVariadic) return None; - // Otherwise we can continue trying to find argument which - // matches parameter with or without label. - continue; + // If the arg is a code completion token with no label, and we're using + // the new solver-based code completion (to avoid affecting the AST used + // by unmigrated completion kinds) pretend the labels matched if it's a + // positional match (i.e. the first attempted arg match) or we're + // ignoring name mismatches. Completion will suggest the label in such + // cases. + if (completionInfo && argLabel.empty() && + isCodeCompletionTypeVar(args[i].getPlainType()) && + (i == firstArgIdx || ignoreNameMismatch)) { + // Avoid triggering relabelling fixes about the completion arg. + claimIgnoringNameMismatch = true; + } else { + // Otherwise we can continue trying to find argument which + // matches parameter with or without label. + continue; + } } // Skip claimed arguments. @@ -401,7 +406,7 @@ static bool matchCallArgumentsImpl( // func foo(_ a: Int, _ b: Int = 0, c: Int = 0, _ d: Int) {} // foo(1, c: 2, 3) // -> `3` will be claimed as '_ b:'. // ``` - if (argLabel.empty()) + if (argLabel.empty() && !claimIgnoringNameMismatch) continue; // We don't allow out-of-order matching beyond the completion location. @@ -410,13 +415,13 @@ static bool matchCallArgumentsImpl( // the completion was the last argument, as the user may be re-writing // the callsite rather than updating an argument in an existing one. if (completionInfo && completionInfo->isBefore(i)) - continue; + break; potentiallyOutOfOrder = true; } // Claim it. - return claim(paramLabel, i); + return claim(paramLabel, i, claimIgnoringNameMismatch); } // If we're not supposed to attempt any fixes, we're done. @@ -750,71 +755,25 @@ static bool matchCallArgumentsImpl( // If we still haven't claimed all of the arguments, // fail if there is no recovery. if (numClaimedArgs != numArgs) { - bool unclaimedArgAfterCompletion = false; for (auto index : indices(claimedArgs)) { if (claimedArgs[index]) continue; - // Avoid reporting individual extra arguments after the completion - // position. They are reported via a single callback below. + // Avoid reporting extra arguments after the completion position as + // individual failures. Instead give a single report that arguments + // after the completion were invalid. This allows us to rank overloads + // that didn't have invalid args after the completion higher, while + // still treating all overloads that did equally (regardless of the + // exact number of extraneous args). if (completionInfo && completionInfo->isBefore(index)) { - unclaimedArgAfterCompletion = true; - continue; + if (listener.invalidArgumentsAfterCompletion()) + return true; + break; } if (listener.extraArgument(index)) return true; } - - if (unclaimedArgAfterCompletion) { - // There were invalid arguments after the completion position. The - // user may be intending to rewrite the callsite (rather than update an - // argument) so proceed as if all arguments after the completion - // position (even those already bound) weren't present. This prevents - // this solution being penalized based on the exact number and kind - // of issues those arguments contain. - unsigned afterIdx = completionInfo->completionIdx + 1; - - auto argsBegin = args.begin() + afterIdx; - if (argsBegin != args.end()) - args.erase(argsBegin, args.end()); - numArgs = args.size(); - - if (!actualArgNames.empty()) { - auto actualNamesBegin = actualArgNames.begin() + afterIdx; - if (actualNamesBegin != actualArgNames.end()) - actualArgNames.erase(actualNamesBegin, actualArgNames.end()); - } - - auto claimedBegin = claimedArgs.begin() + afterIdx; - if (claimedBegin != claimedArgs.end()) - claimedArgs.erase(claimedBegin, claimedArgs.end()); - numClaimedArgs = llvm::count_if(claimedArgs, [](bool claimed){ - return claimed; - }); - - for (auto ¶ms: llvm::reverse(parameterBindings)) { - if (params.empty()) - continue; - if (!completionInfo->isBefore(params.back())) - break; - if (completionInfo->isBefore(params.front())) { - params.clear(); - haveUnfulfilledParams = true; - } else { - llvm::erase_if(params, [&](unsigned argIdx) { - return completionInfo->isBefore(argIdx); - }); - } - } - - // Report that post-completion args where ignored. This allows us to - // rank overloads that didn't require dropping the args higher, while - // still treating all overloads that did equally, regardless of how - // many args were dropped or what issues they would otherwise trigger. - if (listener.invalidArgumentsAfterCompletion()) - return true; - } } // FIXME: If we had the actual parameters and knew the body names, those @@ -901,8 +860,8 @@ static bool matchCallArgumentsImpl( for (const auto &binding : parameterBindings) { paramToArgMap.push_back(argIdx); // Ignore argument bindings that were synthesized due to missing args. - argIdx += llvm::count_if( - binding, [numArgs](unsigned argIdx) { return argIdx < numArgs; }); + argIdx += llvm::count_if( + binding, [numArgs](unsigned argIdx) { return argIdx < numArgs; }); } } @@ -1250,8 +1209,9 @@ class ArgumentFailureTracker : public MatchCallArgumentListener { // This is only triggered when solving for code completion (which doesn't // produce diagnostics) so no need to record a fix for this. Still increase // the score to penalize this solution though. - CS.increaseScore(SK_Fix); - return false; + auto *locator = CS.getConstraintLocator(Locator); + auto *fix = IgnoreArgumentsAfterCompletion::create(CS, locator); + return CS.recordFix(fix, /*impact=*/1); } bool missingLabel(unsigned paramIndex) override { @@ -2375,7 +2335,7 @@ ConstraintSystem::matchFunctionTypes(FunctionType *func1, FunctionType *func2, auto *typeVar = createTypeVariable(getConstraintLocator(argLoc), TVO_CanBindToNoEscape); params.emplace_back(typeVar); - assignFixedType(typeVar, input); + assignFixedType(typeVar, input, loc); }; { @@ -3297,7 +3257,9 @@ ConstraintSystem::matchTypesBindTypeVar( : getTypeMatchFailure(locator); } - assignFixedType(typeVar, type, /*updateState=*/true, + + auto loc = getConstraintLocator(locator); + assignFixedType(typeVar, type, loc, /*updateState=*/true, /*notifyInference=*/!flags.contains(TMF_BindingTypeVariable)); return getTypeMatchSuccess(); @@ -5222,9 +5184,10 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind, if (auto *iot = type1->getAs()) { if (!rep2->getImpl().canBindToLValue()) return getTypeMatchFailure(locator); - assignFixedType(rep2, LValueType::get(iot->getObjectType())); + assignFixedType(rep2, LValueType::get(iot->getObjectType()), + getConstraintLocator(locator)); } else { - assignFixedType(rep2, type1); + assignFixedType(rep2, type1, getConstraintLocator(locator)); } return getTypeMatchSuccess(); } else if (typeVar1 && !typeVar2) { @@ -5237,9 +5200,10 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind, if (auto *lvt = type2->getAs()) { if (!rep1->getImpl().canBindToInOut()) return getTypeMatchFailure(locator); - assignFixedType(rep1, InOutType::get(lvt->getObjectType())); + assignFixedType(rep1, InOutType::get(lvt->getObjectType()), + getConstraintLocator(locator)); } else { - assignFixedType(rep1, type2); + assignFixedType(rep1, type2, getConstraintLocator(locator)); } return getTypeMatchSuccess(); } if (typeVar1 && typeVar2) { @@ -6375,6 +6339,27 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyConformsToConstraint( if (!shouldAttemptFixes()) return SolutionKind::Error; + // when the conformance check fails and is for code completion + // get the current saved binding and check the source locator + // if from argument conversion do check for which args it relates to here. + if (isForCodeCompletion() && typeVar) { + auto getArgIdx = [&](ConstraintLocator *loc) -> Optional { + SmallVector path; + if (auto applyArg = loc->findLast()) + return applyArg->getArgIdx(); + return None; + }; + + if (auto loc = typeVar->getImpl().getBoundLocator()) { + auto completionInfo = getCompletionArgInfo(loc->getAnchor(), *this); + auto argIdx = getArgIdx(loc); + if (completionInfo && argIdx && completionInfo->isBefore(*argIdx)) { + auto *fix = IgnoreArgumentsAfterCompletion::create(*this, loc); + return recordFix(fix) ? SolutionKind::Error : SolutionKind::Solved; + } + } + } + auto protocolTy = protocol->getDeclaredInterfaceType(); // If this conformance has been fixed already, let's just consider this done. @@ -7147,8 +7132,8 @@ static bool isForKeyPathSubscript(ConstraintSystem &cs, return false; } -static bool isForKeyPathSubscriptWithoutLabel(ConstraintSystem &cs, - ConstraintLocator *locator) { +static bool mayBeForKeyPathSubscriptWithoutLabel(ConstraintSystem &cs, + ConstraintLocator *locator) { if (!locator || !locator->getAnchor()) return false; @@ -7292,7 +7277,7 @@ performMemberLookup(ConstraintKind constraintKind, DeclNameRef memberName, // subscript without a `keyPath:` label. Add it to the result so that it // can be caught by the missing argument label checking later. if (isForKeyPathSubscript(*this, memberLocator) || - (isForKeyPathSubscriptWithoutLabel(*this, memberLocator) + (mayBeForKeyPathSubscriptWithoutLabel(*this, memberLocator) && includeInaccessibleMembers)) { if (baseTy->isAnyObject()) { result.addUnviable( @@ -8403,7 +8388,8 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyMemberConstraint( // them to bind member could mean missing valid hole positions in // the pattern. assignFixedType(memberTypeVar, - PlaceholderType::get(getASTContext(), memberTypeVar)); + PlaceholderType::get(getASTContext(), memberTypeVar), + getConstraintLocator(locator)); } else { recordPotentialHole(memberTypeVar); } @@ -9102,7 +9088,7 @@ bool ConstraintSystem::resolveClosure(TypeVariableType *typeVar, auto closureType = FunctionType::get(parameters, inferredClosureType->getResult(), closureExtInfo); - assignFixedType(typeVar, closureType, closureLocator); + assignFixedType(typeVar, closureType, closureLocator, closureLocator); // If there is a result builder to apply, do so now. if (resultBuilderType) { @@ -11408,38 +11394,37 @@ bool ConstraintSystem::recordFix(ConstraintFix *fix, unsigned impact) { log << ")\n"; } - // If this fix is in a call argument after the argument containing the code - // completion location, give a fixed penalty with the same impact as that for - // the fix to ignore all arguments after the completion position when matching - // arguments based on labels. + // All fixes within call arguments after the argument containing the code + // completion location are coalesced into a single fix with a fixed impact. + // This is so that the exact number and kind of such fixes won't impact the + // ranking of different overload choices. Fixes being required in arguments + // after the completion point are taken as an indication that the user is + // re-writing the call and intends to remove those arguments afterwards. if (isForCodeCompletion()) { - - auto getArgAndAnchor = [&](ConstraintLocator *loc) -> Optional> { - SmallVector path; - auto anchor = loc->getAnchor(); + auto loc = fix->getLocator(); + auto isIgnoreArgsFix = [&](ConstraintFix *otherFix) { + return otherFix->getKind() == FixKind::IgnoreArgumentsAfterCompletion && + otherFix->getLocator()->getAnchor() == loc->getAnchor(); + }; + auto getArgIdx = [&](ConstraintLocator *loc) -> Optional { if (auto applyArg = loc->findLast()) - return std::make_pair(applyArg->getArgIdx(), anchor); + return applyArg->getArgIdx(); return None; }; - auto *loc = fix->getLocator(); - loc->dump(this); - auto completionInfo = getCompletionArgInfo(loc->getAnchor(), *this); - auto argAndAnchor = getArgAndAnchor(loc); - if (completionInfo && argAndAnchor && - completionInfo->isBefore(argAndAnchor->first)) { - // Make sure we only penalize issues in arguments after the argument - // containing the completion position once. - bool isFirst = llvm::all_of(getFixes(), [&](const ConstraintFix *fix) { - if (auto fixArgAndAnchor = getArgAndAnchor(fix->getLocator())) - return !completionInfo->isBefore(fixArgAndAnchor->first) || - fixArgAndAnchor->second != argAndAnchor->second || - fixArgAndAnchor->first > argAndAnchor->first; - return true; - }); - if (!isFirst) - return false; - impact = 1; + + if (fix->getKind() != FixKind::IgnoreArgumentsAfterCompletion) { + // If this fix is within a call arg after the arg containing the code + // completion point, remap it to an IgnoreArgumentsAfterCompletion fix. + auto *loc = fix->getLocator(); + auto completionInfo = getCompletionArgInfo(loc->getAnchor(), *this); + auto argIdx = getArgIdx(loc); + if (completionInfo && argIdx && completionInfo->isBefore(*argIdx)) + return recordFix(IgnoreArgumentsAfterCompletion::create(*this, loc)); + } else if (llvm::any_of(getFixes(), isIgnoreArgsFix)) { + // If we've already recorded an IgnoreArgumentsAfterCompletion fix for + // this anchor, ignore this fix. + return false; } } @@ -11783,7 +11768,7 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint( TVO_CanBindToLValue | TVO_CanBindToNoEscape | TVO_CanBindToHole); - assignFixedType(valueBaseTy, dictTy); + assignFixedType(valueBaseTy, dictTy, fix->getLocator()); auto dictionaryKeyTy = DependentMemberType::get(valueBaseTy, keyAssocTy); // Extract the array element type. @@ -11865,6 +11850,7 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint( case FixKind::SpecifyClosureParameterType: case FixKind::SpecifyClosureReturnType: case FixKind::AddQualifierToAccessTopLevelName: + case FixKind::IgnoreArgumentsAfterCompletion: llvm_unreachable("handled elsewhere"); } diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index 490bc1cf0ac6f..420de710a53bd 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -92,7 +92,7 @@ Solution ConstraintSystem::finalize() { break; case FreeTypeVariableBinding::UnresolvedType: - assignFixedType(tv, ctx.TheUnresolvedType); + assignFixedType(tv, ctx.TheUnresolvedType, /*loc=*/nullptr); break; } } @@ -214,7 +214,7 @@ void ConstraintSystem::applySolution(const Solution &solution) { // If we don't already have a fixed type for this type variable, // assign the fixed type from the solution. if (!getFixedType(binding.first) && !binding.second->hasTypeVariable()) - assignFixedType(binding.first, binding.second, /*updateState=*/false); + assignFixedType(binding.first, binding.second, /*loc=*/nullptr, /*updateState=*/false); } // Register overload choices. diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index a5742d8563f94..313d568bfd7d1 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -155,12 +155,12 @@ bool ConstraintSystem::typeVarOccursInType(TypeVariableType *typeVar, } void ConstraintSystem::assignFixedType(TypeVariableType *typeVar, Type type, - bool updateState, + ConstraintLocator *loc, bool updateState, bool notifyBindingInference) { assert(!type->hasError() && "Should not be assigning a type involving ErrorType!"); - typeVar->getImpl().assignFixedType(type, getSavedBindings()); + typeVar->getImpl().assignFixedType(type, loc, getSavedBindings()); if (!updateState) return; diff --git a/lib/Sema/TypeCheckCodeCompletion.cpp b/lib/Sema/TypeCheckCodeCompletion.cpp index e761e918661d1..45ef4499eb188 100644 --- a/lib/Sema/TypeCheckCodeCompletion.cpp +++ b/lib/Sema/TypeCheckCodeCompletion.cpp @@ -1120,33 +1120,9 @@ swift::lookupSemanticMember(DeclContext *DC, Type ty, DeclName name) { return TypeChecker::lookupMember(DC, ty, DeclNameRef(name), None); } -void DotExprTypeCheckCompletionCallback::fallbackTypeCheck() { - assert(!gotCallback()); - - // Default to checking the completion expression in isolation. - Expr *fallbackExpr = CompletionExpr; - DeclContext *fallbackDC = DC; - - CompletionContextFinder finder(DC); - if (finder.hasCompletionExpr()) { - if (auto fallback = finder.getFallbackCompletionExpr()) { - fallbackExpr = fallback->E; - fallbackDC = fallback->DC; - } - } - - SolutionApplicationTarget completionTarget(fallbackExpr, fallbackDC, - CTP_Unused, Type(), - /*isDiscared=*/true); - - TypeChecker::typeCheckForCodeCompletion( - completionTarget, /*needsPrecheck*/true, - [&](const Solution &S) { sawSolution(S); }); -} - -void UnresolvedMemberTypeCheckCompletionCallback:: +void TypeCheckCompletionCallback:: fallbackTypeCheck(DeclContext *DC) { - assert(!gotCallback()); + assert(!GotCallback); CompletionContextFinder finder(DC); if (!finder.hasCompletionExpr()) @@ -1156,7 +1132,6 @@ fallbackTypeCheck(DeclContext *DC) { if (!fallback) return; - SolutionApplicationTarget completionTarget(fallback->E, fallback->DC, CTP_Unused, Type(), /*isDiscared=*/true); @@ -1165,21 +1140,25 @@ fallbackTypeCheck(DeclContext *DC) { [&](const Solution &S) { sawSolution(S); }); } -void ArgumentTypeCheckCompletionCallback:: -fallbackTypeCheck(DeclContext *DC) { +void DotExprTypeCheckCompletionCallback::fallbackTypeCheck(DeclContext *DC) { assert(!gotCallback()); - CompletionContextFinder finder(DC); - if (!finder.hasCompletionExpr()) - return; + // Default to checking the completion expression in isolation. + Expr *fallbackExpr = CompletionExpr; + DeclContext *fallbackDC = DC; - auto fallback = finder.getFallbackCompletionExpr(); - if (!fallback) - return; + CompletionContextFinder finder(DC); + if (finder.hasCompletionExpr()) { + if (auto fallback = finder.getFallbackCompletionExpr()) { + fallbackExpr = fallback->E; + fallbackDC = fallback->DC; + } + } - SolutionApplicationTarget completionTarget(fallback->E, fallback->DC, + SolutionApplicationTarget completionTarget(fallbackExpr, fallbackDC, CTP_Unused, Type(), - /*isDiscarded=*/true); + /*isDiscared=*/true); + TypeChecker::typeCheckForCodeCompletion( completionTarget, /*needsPrecheck*/true, [&](const Solution &S) { sawSolution(S); }); @@ -1261,7 +1240,7 @@ static bool isImplicitSingleExpressionReturn(ConstraintSystem &CS, void DotExprTypeCheckCompletionCallback:: sawSolution(const constraints::Solution &S) { - GotCallback = true; + TypeCheckCompletionCallback::sawSolution(S); auto &CS = S.getConstraintSystem(); auto *ParsedExpr = CompletionExpr->getBase(); auto *SemanticExpr = ParsedExpr->getSemanticsProvidingExpr(); @@ -1308,7 +1287,7 @@ sawSolution(const constraints::Solution &S) { void UnresolvedMemberTypeCheckCompletionCallback:: sawSolution(const constraints::Solution &S) { - GotCallback = true; + TypeCheckCompletionCallback::sawSolution(S); auto &CS = S.getConstraintSystem(); Type ExpectedTy = getTypeForCompletion(S, CompletionExpr); @@ -1331,6 +1310,8 @@ sawSolution(const constraints::Solution &S) { void KeyPathTypeCheckCompletionCallback::sawSolution( const constraints::Solution &S) { + TypeCheckCompletionCallback::sawSolution(S); + // Determine the code completion. size_t ComponentIndex = 0; for (auto &Component : KeyPath->getComponents()) { @@ -1380,13 +1361,9 @@ void KeyPathTypeCheckCompletionCallback::sawSolution( } } -bool ArgumentTypeCheckCompletionCallback::Result::isSubscript() const { - return CallE && isa(CallE); -} - void ArgumentTypeCheckCompletionCallback:: sawSolution(const constraints::Solution &S) { - GotCallback = true; + TypeCheckCompletionCallback::sawSolution(S); Type ExpectedTy = getTypeForCompletion(S, CompletionExpr); if (!ExpectedTy) @@ -1417,10 +1394,11 @@ sawSolution(const constraints::Solution &S) { ValueDecl *FuncD = SelectedOverload->choice.getDeclOrNull(); Type FuncTy = S.simplifyType(SelectedOverload->openedType); - // For completion as the arg in a call to the implicit (keypath: _) method the - // solver can't know what kind of keypath is expected (e.g. KeyPath vs - // WritableKeyPath) so it ends up as a hole. Just assume KeyPath so we show - // the root type to users. + // For completion as the arg in a call to the implicit [keypath: _] subscript + // the solver can't know what kind of keypath is expected without an actual + // argument (e.g. a KeyPath vs WritableKeyPath) so it ends up as a hole. + // Just assume KeyPath so we show the expected keypath's root type to users + // rather than '_'. if (SelectedOverload->choice.getKind() == OverloadChoiceKind::KeyPathApplication) { auto Params = FuncTy->getAs()->getParams(); if (Params.size() == 1 && Params[0].getPlainType()->is()) { @@ -1462,19 +1440,6 @@ sawSolution(const constraints::Solution &S) { ClaimedParams.push_back(i); } - if (!ParamIdx && Bindings.size()) { - if (ClaimedParams.empty()) { - // If no parameters were claimed, assume the completion arg corresponds to - // the first paramter. - assert(!ParamIdx); - ParamIdx = 0; - } else if (ClaimedParams.back() < Bindings.size() - 1) { - // Otherwise assume it's the param after the last claimed parameter. - ParamIdx = ClaimedParams.back() + 1; - IsNoninitialVariadic = Bindings[*ParamIdx].size() > 1; - } - } - bool HasLabel = false; Expr *Arg = CS.getParentExpr(CompletionExpr); if (TupleExpr *TE = dyn_cast(Arg)) @@ -1498,6 +1463,6 @@ sawSolution(const constraints::Solution &S) { return; } - Results.push_back({ExpectedTy, ParentCall, FuncD, FuncTy, ArgIdx, ParamIdx, + Results.push_back({ExpectedTy, isa(ParentCall), FuncD, FuncTy, ArgIdx, ParamIdx, std::move(ClaimedParams), IsNoninitialVariadic, BaseTy, HasLabel}); } diff --git a/lib/Sema/TypeCheckConstraints.cpp b/lib/Sema/TypeCheckConstraints.cpp index 7a9d4c5ca7cc4..c86141f33e779 100644 --- a/lib/Sema/TypeCheckConstraints.cpp +++ b/lib/Sema/TypeCheckConstraints.cpp @@ -58,11 +58,13 @@ void TypeVariableType::Implementation::print(llvm::raw_ostream &OS) { SavedTypeVariableBinding::SavedTypeVariableBinding(TypeVariableType *typeVar) : TypeVar(typeVar), Options(typeVar->getImpl().getRawOptions()), - ParentOrFixed(typeVar->getImpl().ParentOrFixed) { } + ParentOrFixed(typeVar->getImpl().ParentOrFixed), + BoundLocator(typeVar->getImpl().BoundLocator) { } void SavedTypeVariableBinding::restore() { TypeVar->getImpl().setRawOptions(Options); TypeVar->getImpl().ParentOrFixed = ParentOrFixed; + TypeVar->getImpl().BoundLocator = BoundLocator; } GenericTypeParamType * diff --git a/test/IDE/complete_ambiguous.swift b/test/IDE/complete_ambiguous.swift index b5f5bf6ca9ca4..5aabc57aea677 100644 --- a/test/IDE/complete_ambiguous.swift +++ b/test/IDE/complete_ambiguous.swift @@ -492,9 +492,9 @@ func testAmbiguousArgs() { overloaded().someFunc(a: 2, #^ARG_NO_LABEL^#) // ARG_NO_LABEL: Begin completions, 3 items - // ARG_NO_LABEL-DAG: Pattern/Local/Flair[ArgLabels]: {#b: Int#}[#Int#]; name=b: Int - // ARG_NO_LABEL-DAG: Pattern/Local/Flair[ArgLabels]: {#c: String#}[#String#]; name=c: String - // ARG_NO_LABEL-DAG: Pattern/Local/Flair[ArgLabels]: {#d: String#}[#String#]; name=d: String + // ARG_NO_LABEL-DAG: Pattern/Local/Flair[ArgLabels]: {#b: Int#}[#Int#]; name=b: + // ARG_NO_LABEL-DAG: Pattern/Local/Flair[ArgLabels]: {#c: String#}[#String#]; name=c: + // ARG_NO_LABEL-DAG: Pattern/Local/Flair[ArgLabels]: {#d: String#}[#String#]; name=d: // ARG_NO_LABEL: End completions overloaded().someFunc(a: 2, b: #^ARG_LABEL^#) @@ -507,20 +507,21 @@ func testAmbiguousArgs() { overloaded().someFunc(a: 2, c: "Foo", #^ARG_NO_LABEL2^#) // ARG_NO_LABEL2: Begin completions, 1 item - // ARG_NO_LABEL2: Pattern/Local/Flair[ArgLabels]: {#d: String#}[#String#]; name=d: String + // ARG_NO_LABEL2: Pattern/Local/Flair[ArgLabels]: {#d: String#}[#String#]; name=d: // ARG_NO_LABEL2: End completions overloaded().someFunc(a: 2, wrongLabel: "Foo", #^ARG_NO_LABEL_PREV_WRONG^#) - // ARG_NO_LABEL_PREV_WRONG: Begin completions, 2 items - // ARG_NO_LABEL_PREV_WRONG-DAG: Pattern/Local/Flair[ArgLabels]: {#c: String#}[#String#]; name=c: String - // ARG_NO_LABEL_PREV_WRONG-DAG: Pattern/Local/Flair[ArgLabels]: {#d: String#}[#String#]; name=d: String + // ARG_NO_LABEL_PREV_WRONG: Begin completions, 3 items + // ARG_NO_LABEL_PREV_WRONG-DAG: Pattern/Local/Flair[ArgLabels]: {#b: Int#}[#Int#]; name=b: + // ARG_NO_LABEL_PREV_WRONG-DAG: Pattern/Local/Flair[ArgLabels]: {#c: String#}[#String#]; name=c: + // ARG_NO_LABEL_PREV_WRONG-DAG: Pattern/Local/Flair[ArgLabels]: {#d: String#}[#String#]; name=d: // ARG_NO_LABEL_PREV_WRONG: End completions overloaded().someFunc(a: 2, d: "Foo", #^ARG_NO_LABEL_OUT_OF_ORDER^#) // ARG_NO_LABEL_OUT_OF_ORDER: Begin completions // ARG_NO_LABEL_OUT_OF_ORDER-NOT: name=d: String - // ARG_NO_LABEL_OUT_OF_ORDER: Pattern/Local/Flair[ArgLabels]: {#c: String#}[#String#]; name=c: String + // ARG_NO_LABEL_OUT_OF_ORDER: Pattern/Local/Flair[ArgLabels]: {#c: String#}[#String#]; name=c: // ARG_NO_LABEL_OUT_OF_ORDER-NOT: name=d: String // ARG_NO_LABEL_OUT_OF_ORDER: End completions @@ -533,20 +534,20 @@ func testAmbiguousArgs() { overloaded().someFunc(a: 2, #^LATER_ARGS^#, d: "foo") // LATER_ARGS: Begin completions, 1 item - // LATER_ARGS: Pattern/Local/Flair[ArgLabels]: {#c: String#}[#String#]; name=c: String + // LATER_ARGS: Pattern/Local/Flair[ArgLabels]: {#c: String#}[#String#]; name=c: // LATER_ARGS: End completions overloaded().someFunc(a: 2, #^LATER_ARGS_WRONG^#, k: 4.5) // LATER_ARGS_WRONG: Begin completions, 3 items - // LATER_ARGS_WRONG-DAG: Pattern/Local/Flair[ArgLabels]: {#b: Int#}[#Int#]; name=b: Int - // LATER_ARGS_WRONG-DAG: Pattern/Local/Flair[ArgLabels]: {#c: String#}[#String#]; name=c: String - // LATER_ARGS_WRONG-DAG: Pattern/Local/Flair[ArgLabels]: {#d: String#}[#String#]; name=d: String + // LATER_ARGS_WRONG-DAG: Pattern/Local/Flair[ArgLabels]: {#b: Int#}[#Int#]; name=b: + // LATER_ARGS_WRONG-DAG: Pattern/Local/Flair[ArgLabels]: {#c: String#}[#String#]; name=c: + // LATER_ARGS_WRONG-DAG: Pattern/Local/Flair[ArgLabels]: {#d: String#}[#String#]; name=d: // LATER_ARGS_WRONG-DAG: End completions overloaded().variadic(y: 2, #^INITIAL_VARARG^#, 4) // INITIAL_VARARG: Begin completions, 1 item - // INITIAL_VARARG: Pattern/Local/Flair[ArgLabels]: {#x: Int...#}[#Int#]; name=x: Int... + // INITIAL_VARARG: Pattern/Local/Flair[ArgLabels]: {#x: Int...#}[#Int#]; name=x: // INITIAL VARARG: End completions overloaded().variadic(y: 2, x: 2, #^NONINITIAL_VARARG^#) diff --git a/test/IDE/complete_call_arg.swift b/test/IDE/complete_call_arg.swift index be80403915fc8..23e97342fcfe6 100644 --- a/test/IDE/complete_call_arg.swift +++ b/test/IDE/complete_call_arg.swift @@ -1035,6 +1035,15 @@ func testArgsAfterCompletion() { // VALID_UNRESOLVED-DAG: Decl[InstanceMethod]/CurrNominal/TypeRelation[Invalid]: hash({#(self): A#})[#(into: inout Hasher) -> Void#]; name=hash(:) // VALID_UNRESOLVED: End completions + overloaded(x: 1, .#^VALID_UNRESOLVED_NOCOMMA^# localB) + + // VALID_UNRESOLVED_NOCOMMA: Begin completions, 4 items + // VALID_UNRESOLVED_NOCOMMA-DAG: Decl[EnumElement]/CurrNominal/Flair[ExprSpecific]/TypeRelation[Identical]: a[#A#]; name=a + // VALID_UNRESOLVED_NOCOMMA-DAG: Decl[EnumElement]/CurrNominal/Flair[ExprSpecific]/TypeRelation[Identical]: b[#B#]; name=b + // VALID_UNRESOLVED_NOCOMMA-DAG: Decl[InstanceMethod]/CurrNominal/TypeRelation[Invalid]: hash({#(self): A#})[#(into: inout Hasher) -> Void#]; name=hash(:) + // VALID_UNRESOLVED_NOCOMMA-DAG: Decl[InstanceMethod]/CurrNominal/TypeRelation[Invalid]: hash({#(self): B#})[#(into: inout Hasher) -> Void#]; name=hash(:) + // VALID_UNRESOLVED_NOCOMMA: End completions + overloaded(x: 1, #^VALID_GLOBAL^#, localB) // VALID_GLOBAL: Begin completions // VALID_GLOBAL-DAG: Decl[LocalVar]/Local/TypeRelation[Identical]: localA[#A#]; name=localA @@ -1049,12 +1058,19 @@ func testArgsAfterCompletion() { // VALID_LABEL: Pattern/Local/Flair[ArgLabels]: {#firstA: A#}[#A#]; name=firstA: // VALID_LABEL: End completions + overloadedLabel(x: 1, #^VALID_LABEL_NOCOMMA^# second: localB) + // VALID_LABEL_NOCOMMA: Begin completions, 2 items + // VALID_LABEL_NOCOMMA-DAG: Pattern/Local/Flair[ArgLabels]: {#firstA: A#}[#A#]; name=firstA: + // VALID_LABEL_NOCOMMA-DAG: Pattern/Local/Flair[ArgLabels]: {#firstB: B#}[#B#]; name=firstB: + // VALID_LABEL_NOCOMMA: End completions + overloadedLabel(x: 1, #^INVALID_LABEL^#, wrongLabelRightType: localB) // INVALID_LABEL: Begin completions, 2 items // INVALID_LABEL-DAG: Pattern/Local/Flair[ArgLabels]: {#firstA: A#}[#A#]; name=firstA: // INVALID_LABEL-DAG: Pattern/Local/Flair[ArgLabels]: {#firstB: B#}[#B#]; name=firstB: // INVALID_LABEL: End completions + overloadedLabel(x: 1, #^INVALID_TYPE?check=INVALID_LABEL^#, second: 34) overloadedLabel(x: 1, #^INVALID_LABEL_TYPE?check=INVALID_LABEL^#, wrongLabelWrongType: 2) // invalid func overloadedArity(x: Int, firstA: A, second: B, third: Double) {} @@ -1065,6 +1081,8 @@ func testArgsAfterCompletion() { // VALID_ARITY: Pattern/Local/Flair[ArgLabels]: {#firstA: A#}[#A#]; name=firstA: // VALID_ARITY: End completions + overloadedArity(x: 1, #^VALID_ARITY_NOCOMMA?check=VALID_ARITY^# second: localB, third: 4.5) + overloadedArity(x: 1, #^INVALID_ARITY^#, wrong: localB) // INVALID_ARITY: Begin completions, 2 items // INVALID_ARITY-DAG: Pattern/Local/Flair[ArgLabels]: {#firstA: A#}[#A#]; name=firstA: @@ -1090,6 +1108,7 @@ func testArgsAfterCompletion() { // VALID_DEFAULTED_AFTER-DAG: Pattern/Local/Flair[ArgLabels]: {#y: A#}[#A#]; name=y: // VALID_DEFAULTED_AFTER: End completions + overloadedDefaulted(x: 1, #^VALID_DEFAULTED_AFTER_NOCOMMA?check=VALID_DEFAULTED^# z: localA) overloadedDefaulted(x: 1, #^INVALID_DEFAULTED?check=VALID_DEFAULTED^#, w: "hello") overloadedDefaulted(x: 1, #^INVALID_DEFAULTED_TYPO?check=VALID_DEFAULTED^#, zz: localA) overloadedDefaulted(x: 1, #^INVALID_DEFAULTED_TYPO_TYPE?check=VALID_DEFAULTED^#, zz: "hello") @@ -1104,8 +1123,10 @@ func testArgsAfterCompletion() { func overloadedGeneric(x: Int, p: String, q: Int) {} struct MissingConformance {} overloadedGeneric(x: 1, #^INVALID_MISSINGCONFORMANCE^#, z: MissingConformance(), y: MissingConformance()) - // FIXME: This should also suggest y. - // INVALID_MISSINGCONFORMANCE: Begin completions, 1 item - // INVALID_MISSINGCONFORMANCE: Pattern/Local/Flair[ArgLabels]: {#p: String#}[#String#]; name=p: + // INVALID_MISSINGCONFORMANCE: Begin completions, 2 items + // INVALID_MISSINGCONFORMANCE-DAG: Pattern/Local/Flair[ArgLabels]: {#p: String#}[#String#]; name=p: + // INVALID_MISSINGCONFORMANCE-DAG: Pattern/Local/Flair[ArgLabels]: {#y: String#}[#String#]; name=y: // INVALID_MISSINGCONFORMANCE: End completions + + overloadedGeneric(x: 1, #^INVALID_MISSINGCONFORMANCE_NOCOMMA?check=INVALID_MISSINGCONFORMANCE^# z: MisingConformance(), y: MissingConformance()) } From d83bff8469badf06dc4a0717950f954bcc7ba3b9 Mon Sep 17 00:00:00 2001 From: Nathan Hawes Date: Fri, 30 Jul 2021 14:05:29 +1000 Subject: [PATCH 4/7] more fixes --- include/swift/Sema/CSFix.h | 12 +-- lib/Sema/CSFix.cpp | 8 +- lib/Sema/CSSimplify.cpp | 112 ++++++++++++++------------- lib/Sema/TypeCheckCodeCompletion.cpp | 12 +++ test/IDE/complete_call_arg.swift | 8 ++ 5 files changed, 88 insertions(+), 64 deletions(-) diff --git a/include/swift/Sema/CSFix.h b/include/swift/Sema/CSFix.h index 5ff848037ba9c..8f70cdb4930ed 100644 --- a/include/swift/Sema/CSFix.h +++ b/include/swift/Sema/CSFix.h @@ -336,7 +336,7 @@ enum class FixKind : uint8_t { /// Ignore all failures in call or subscript arguments after the argument /// containing the code completion location. This is only produced when /// solving for code completion. - IgnoreArgumentsAfterCompletion, + IgnoreFailureAfterCompletionArg, }; class ConstraintFix { @@ -1736,10 +1736,10 @@ class SkipUnhandledConstructInResultBuilder final : public ConstraintFix { NominalTypeDecl *builder, ConstraintLocator *locator); }; -class IgnoreArgumentsAfterCompletion final : public ConstraintFix { - IgnoreArgumentsAfterCompletion(ConstraintSystem &cs, - ConstraintLocator *locator) - : ConstraintFix(cs, FixKind::IgnoreArgumentsAfterCompletion, locator) {} +class IgnoreFailureAfterCompletionArg final : public ConstraintFix { + IgnoreFailureAfterCompletionArg(ConstraintSystem &cs, + ConstraintLocator *locator) + : ConstraintFix(cs, FixKind::IgnoreFailureAfterCompletionArg, locator, true) {} public: std::string getName() const override { @@ -1751,7 +1751,7 @@ class IgnoreArgumentsAfterCompletion final : public ConstraintFix { return false; } - static IgnoreArgumentsAfterCompletion * + static IgnoreFailureAfterCompletionArg * create(ConstraintSystem &cs, ConstraintLocator *locator); }; diff --git a/lib/Sema/CSFix.cpp b/lib/Sema/CSFix.cpp index 01b43f9cd2b2a..90b886ba3a755 100644 --- a/lib/Sema/CSFix.cpp +++ b/lib/Sema/CSFix.cpp @@ -1182,10 +1182,10 @@ bool SkipUnhandledConstructInResultBuilder::diagnose(const Solution &solution, return failure.diagnose(asNote); } -IgnoreArgumentsAfterCompletion * -IgnoreArgumentsAfterCompletion:: create(ConstraintSystem &cs, - ConstraintLocator *locator) { - return new (cs.getAllocator()) IgnoreArgumentsAfterCompletion(cs, locator); +IgnoreFailureAfterCompletionArg * +IgnoreFailureAfterCompletionArg:: create(ConstraintSystem &cs, + ConstraintLocator *locator) { + return new (cs.getAllocator()) IgnoreFailureAfterCompletionArg(cs, locator); } bool AllowMutatingMemberOnRValueBase::diagnose(const Solution &solution, diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index c6b176ea27744..0faf30c822ebe 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -1210,7 +1210,7 @@ class ArgumentFailureTracker : public MatchCallArgumentListener { // produce diagnostics) so no need to record a fix for this. Still increase // the score to penalize this solution though. auto *locator = CS.getConstraintLocator(Locator); - auto *fix = IgnoreArgumentsAfterCompletion::create(CS, locator); + auto *fix = IgnoreFailureAfterCompletionArg::create(CS, CS.getConstraintLocator(locator->getAnchor())); return CS.recordFix(fix, /*impact=*/1); } @@ -6222,6 +6222,39 @@ ConstraintSystem::simplifyConstructionConstraint( return SolutionKind::Solved; } +/// Determines if a failure with the given locator appears within a call or +/// subscript argument list after the argument containing the code completion +/// location. +/// +/// \returns a locator pointing to the containing call/subscript if it meets +/// these conditions and \c nullptr otherwise +static ConstraintLocator *isInArgAfterCompletion(ConstraintLocator *loc, ConstraintSystem &cs) { + auto fixExpr = simplifyLocatorToAnchor(loc).dyn_cast(); + auto commonAncestor = fixExpr; + auto fixChild = fixExpr; + while (commonAncestor && !cs.containsCodeCompletionLoc(commonAncestor)) { + fixChild = commonAncestor; + commonAncestor = cs.getParentExpr(commonAncestor); + } + + if (!commonAncestor || commonAncestor == fixExpr) + return nullptr; + + if (auto *TE = dyn_cast(commonAncestor)) { + assert(TE->getNumElements() >= 2); + auto Parent = cs.getParentExpr(TE); + if (!Parent || (!isa(Parent) && !isa(Parent))) + return nullptr; + for (auto *E: TE->getElements()) { + if (E == fixChild) // found locator element first + return nullptr; + if (cs.containsCodeCompletionLoc(E)) // found completion element first + return cs.getConstraintLocator(Parent); + } + } + return nullptr; +} + ConstraintSystem::SolutionKind ConstraintSystem::simplifyConformsToConstraint( Type type, Type protocol, @@ -6339,33 +6372,26 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyConformsToConstraint( if (!shouldAttemptFixes()) return SolutionKind::Error; - // when the conformance check fails and is for code completion - // get the current saved binding and check the source locator - // if from argument conversion do check for which args it relates to here. - if (isForCodeCompletion() && typeVar) { - auto getArgIdx = [&](ConstraintLocator *loc) -> Optional { - SmallVector path; - if (auto applyArg = loc->findLast()) - return applyArg->getArgIdx(); - return None; - }; + auto protocolTy = protocol->getDeclaredInterfaceType(); + // If this conformance has been fixed already, let's just consider this done. + if (isFixedRequirement(loc, protocolTy)) + return SolutionKind::Solved; + + // If the non-conforming type originated from a call argument after an + // argument containing the code completion location, ignore this failure. + // We assume the user is trying to rewrite the callsite without having + // first removed the existing arguments in such cases, so failures due to + // later arguments shouldn't impact on the viability of this solution. + if (isForCodeCompletion() && typeVar) { if (auto loc = typeVar->getImpl().getBoundLocator()) { - auto completionInfo = getCompletionArgInfo(loc->getAnchor(), *this); - auto argIdx = getArgIdx(loc); - if (completionInfo && argIdx && completionInfo->isBefore(*argIdx)) { - auto *fix = IgnoreArgumentsAfterCompletion::create(*this, loc); + if (auto newLoc = isInArgAfterCompletion(loc, *this)) { + auto *fix = IgnoreFailureAfterCompletionArg::create(*this, newLoc); return recordFix(fix) ? SolutionKind::Error : SolutionKind::Solved; } } } - auto protocolTy = protocol->getDeclaredInterfaceType(); - - // If this conformance has been fixed already, let's just consider this done. - if (isFixedRequirement(loc, protocolTy)) - return SolutionKind::Solved; - // If this is a generic requirement let's try to record that // conformance is missing and consider this a success, which // makes it much easier to diagnose problems like that. @@ -11394,38 +11420,16 @@ bool ConstraintSystem::recordFix(ConstraintFix *fix, unsigned impact) { log << ")\n"; } - // All fixes within call arguments after the argument containing the code - // completion location are coalesced into a single fix with a fixed impact. - // This is so that the exact number and kind of such fixes won't impact the - // ranking of different overload choices. Fixes being required in arguments - // after the completion point are taken as an indication that the user is - // re-writing the call and intends to remove those arguments afterwards. - if (isForCodeCompletion()) { - auto loc = fix->getLocator(); - auto isIgnoreArgsFix = [&](ConstraintFix *otherFix) { - return otherFix->getKind() == FixKind::IgnoreArgumentsAfterCompletion && - otherFix->getLocator()->getAnchor() == loc->getAnchor(); - }; - auto getArgIdx = [&](ConstraintLocator *loc) -> Optional { - if (auto applyArg = loc->findLast()) - return applyArg->getArgIdx(); - return None; - }; - - - if (fix->getKind() != FixKind::IgnoreArgumentsAfterCompletion) { - // If this fix is within a call arg after the arg containing the code - // completion point, remap it to an IgnoreArgumentsAfterCompletion fix. - auto *loc = fix->getLocator(); - auto completionInfo = getCompletionArgInfo(loc->getAnchor(), *this); - auto argIdx = getArgIdx(loc); - if (completionInfo && argIdx && completionInfo->isBefore(*argIdx)) - return recordFix(IgnoreArgumentsAfterCompletion::create(*this, loc)); - } else if (llvm::any_of(getFixes(), isIgnoreArgsFix)) { - // If we've already recorded an IgnoreArgumentsAfterCompletion fix for - // this anchor, ignore this fix. - return false; - } + if (isForCodeCompletion() && !fix->isWarning()) { + // All fixes within call arguments after the argument containing the code + // completion point are remapped to a warning, as they're taken to mean the + // user intends to re-write the callsite. This ensures the exact number and + // kind of these fixes won't impact the scoring of different overload + // choices. We still track that this happend via the warning rather than + // just adjusting the impact so we can rank overloads that didn't require it + // higher in the completion results. + if (auto newLoc = isInArgAfterCompletion(fix->getLocator(), *this)) + return recordFix(IgnoreFailureAfterCompletionArg::create(*this, newLoc)); } // Record the fix. @@ -11850,7 +11854,7 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint( case FixKind::SpecifyClosureParameterType: case FixKind::SpecifyClosureReturnType: case FixKind::AddQualifierToAccessTopLevelName: - case FixKind::IgnoreArgumentsAfterCompletion: + case FixKind::IgnoreFailureAfterCompletionArg: llvm_unreachable("handled elsewhere"); } diff --git a/lib/Sema/TypeCheckCodeCompletion.cpp b/lib/Sema/TypeCheckCodeCompletion.cpp index 45ef4499eb188..f58f0064518f9 100644 --- a/lib/Sema/TypeCheckCodeCompletion.cpp +++ b/lib/Sema/TypeCheckCodeCompletion.cpp @@ -876,6 +876,18 @@ static void filterSolutions(SolutionApplicationTarget &target, return S.getFixedScore().Data[SK_Fix] != 0 && S.getFixedScore() > minScore; }); + + // If some of the remaining solutions didn't require fixes to ignore failures + // in call arguments after the argument containing the completion location, + // remove those that did. + auto hasIgnoredFailures = [](const Solution &S) { + return llvm::any_of(S.Fixes, [&](ConstraintFix *F) { + return F->getKind() == FixKind::IgnoreFailureAfterCompletionArg; + }); + }; + if (llvm::all_of(solutions, hasIgnoredFailures)) + return; + llvm::erase_if(solutions, hasIgnoredFailures); } bool TypeChecker::typeCheckForCodeCompletion( diff --git a/test/IDE/complete_call_arg.swift b/test/IDE/complete_call_arg.swift index 23e97342fcfe6..a774e6d3a1aa7 100644 --- a/test/IDE/complete_call_arg.swift +++ b/test/IDE/complete_call_arg.swift @@ -1018,6 +1018,11 @@ func test_SR14737() { } } +struct CondConfType { + init(_: U) +} +extension CondConfType: Equatable where U == Int {} + func testArgsAfterCompletion() { enum A { case a } enum B { case b } @@ -1129,4 +1134,7 @@ func testArgsAfterCompletion() { // INVALID_MISSINGCONFORMANCE: End completions overloadedGeneric(x: 1, #^INVALID_MISSINGCONFORMANCE_NOCOMMA?check=INVALID_MISSINGCONFORMANCE^# z: MisingConformance(), y: MissingConformance()) + overloadedGeneric(x: 1, #^INVALID_MISSINGCONFORMANCE_INDIRECT?check=INVALID_MISSINGCONFORMANCE^#, z: [MissingConformance()], y: [MissingConformance()]) + + overloadedGeneric(x: 1, #^INVALID_MISSINGCONFORMANCE_CONSTRAINT?check=INVALID_MISSINGCONFORMANCE^#, z: [CondConfType("foo")], y: [CondConfType("bar")]) } From 6eafb6ef018d86f02e962792284dc70e79f5aeb1 Mon Sep 17 00:00:00 2001 From: Nathan Hawes Date: Fri, 30 Jul 2021 14:05:50 +1000 Subject: [PATCH 5/7] formatting --- include/swift/Sema/CSFix.h | 12 ++-- .../swift/Sema/CodeCompletionTypeChecking.h | 72 ++++++++++--------- include/swift/Sema/ConstraintSystem.h | 22 +++--- lib/IDE/CodeCompletion.cpp | 38 +++++----- lib/Sema/CSFix.cpp | 4 +- lib/Sema/CSSimplify.cpp | 61 ++++++++-------- lib/Sema/CSSolver.cpp | 3 +- lib/Sema/TypeCheckCodeCompletion.cpp | 61 ++++++++-------- lib/Sema/TypeCheckConstraints.cpp | 6 +- 9 files changed, 144 insertions(+), 135 deletions(-) diff --git a/include/swift/Sema/CSFix.h b/include/swift/Sema/CSFix.h index 8f70cdb4930ed..d121e984f54e3 100644 --- a/include/swift/Sema/CSFix.h +++ b/include/swift/Sema/CSFix.h @@ -1737,13 +1737,15 @@ class SkipUnhandledConstructInResultBuilder final : public ConstraintFix { }; class IgnoreFailureAfterCompletionArg final : public ConstraintFix { - IgnoreFailureAfterCompletionArg(ConstraintSystem &cs, + IgnoreFailureAfterCompletionArg(ConstraintSystem &cs, ConstraintLocator *locator) - : ConstraintFix(cs, FixKind::IgnoreFailureAfterCompletionArg, locator, true) {} + : ConstraintFix(cs, FixKind::IgnoreFailureAfterCompletionArg, locator, + true) {} public: std::string getName() const override { - return "ignore arguments after the code completion position in a call or subscript"; + return "ignore arguments after the code completion position in a call or " + "subscript"; } bool diagnose(const Solution &solution, bool asNote = false) const override { @@ -1751,8 +1753,8 @@ class IgnoreFailureAfterCompletionArg final : public ConstraintFix { return false; } - static IgnoreFailureAfterCompletionArg * - create(ConstraintSystem &cs, ConstraintLocator *locator); + static IgnoreFailureAfterCompletionArg *create(ConstraintSystem &cs, + ConstraintLocator *locator); }; class AllowTupleSplatForSingleParameter final : public ConstraintFix { diff --git a/include/swift/Sema/CodeCompletionTypeChecking.h b/include/swift/Sema/CodeCompletionTypeChecking.h index 6a9550f29db8e..65685fe6c36aa 100644 --- a/include/swift/Sema/CodeCompletionTypeChecking.h +++ b/include/swift/Sema/CodeCompletionTypeChecking.h @@ -36,6 +36,7 @@ namespace swift { class TypeCheckCompletionCallback { bool GotCallback = false; + public: virtual ~TypeCheckCompletionCallback() {} @@ -76,7 +77,7 @@ namespace swift { public: DotExprTypeCheckCompletionCallback(CodeCompletionExpr *CompletionExpr) - : CompletionExpr(CompletionExpr) {} + : CompletionExpr(CompletionExpr) {} /// Get the results collected from any sawSolutions() callbacks recevied so /// far. @@ -136,44 +137,45 @@ namespace swift { void sawSolution(const constraints::Solution &solution) override; }; -class ArgumentTypeCheckCompletionCallback: public TypeCheckCompletionCallback { -public: - struct Result { - /// The type associated with the code completion expression itself. - Type ExpectedType; - /// True if this is a subscript rather than a function call. - bool IsSubscript; - /// The FuncDecl or SubscriptDecl associated with the call. - ValueDecl *FuncD; - /// The type of the function being called. - Type FuncTy; - /// The index of the argument containing the completion location - unsigned ArgIdx; - /// The index of the parameter corresponding to the completion argument. - Optional ParamIdx; - /// The indices of all params that were bound to non-synthesized arguments. - SmallVector ClaimedParamIndices; - /// True if the completion is a noninitial term in a variadic argument. - bool IsNoninitialVariadic; - /// The base type of the call/subscript (null for free functions). - Type BaseType; - /// True if an argument label precedes the completion location. - bool HasLabel; - }; - -private: - CodeCompletionExpr *CompletionExpr; - SmallVector Results; + class ArgumentTypeCheckCompletionCallback + : public TypeCheckCompletionCallback { + public: + struct Result { + /// The type associated with the code completion expression itself. + Type ExpectedType; + /// True if this is a subscript rather than a function call. + bool IsSubscript; + /// The FuncDecl or SubscriptDecl associated with the call. + ValueDecl *FuncD; + /// The type of the function being called. + Type FuncTy; + /// The index of the argument containing the completion location + unsigned ArgIdx; + /// The index of the parameter corresponding to the completion argument. + Optional ParamIdx; + /// The indices of all params that were bound to non-synthesized + /// arguments. + SmallVector ClaimedParamIndices; + /// True if the completion is a noninitial term in a variadic argument. + bool IsNoninitialVariadic; + /// The base type of the call/subscript (null for free functions). + Type BaseType; + /// True if an argument label precedes the completion location. + bool HasLabel; + }; -public: - ArgumentTypeCheckCompletionCallback(CodeCompletionExpr *CompletionExpr) - : CompletionExpr(CompletionExpr) {} + private: + CodeCompletionExpr *CompletionExpr; + SmallVector Results; - ArrayRef getResults() const { return Results; } + public: + ArgumentTypeCheckCompletionCallback(CodeCompletionExpr *CompletionExpr) + : CompletionExpr(CompletionExpr) {} - void sawSolution(const constraints::Solution &solution) override; -}; + ArrayRef getResults() const { return Results; } + void sawSolution(const constraints::Solution &solution) override; + }; } #endif diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 415f5b4159ed9..6cad52ec0b645 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -475,8 +475,7 @@ class TypeVariableType::Implementation { } /// Assign a fixed type to this equivalence class. - void assignFixedType(Type type, - constraints::ConstraintLocator *loc, + void assignFixedType(Type type, constraints::ConstraintLocator *loc, constraints::SavedTypeVariableBindings *record) { assert((!getFixedType(0) || getFixedType(0)->isEqual(type)) && "Already has a fixed type!"); @@ -3921,8 +3920,7 @@ class ConstraintSystem { /// \param notifyBindingInference Whether to notify binding inference about /// the change to this type variable. void assignFixedType(TypeVariableType *typeVar, Type type, - ConstraintLocator *loc, - bool updateState = true, + ConstraintLocator *loc, bool updateState = true, bool notifyBindingInference = true); /// Determine if the type in question is an Array and, if so, provide the @@ -5391,15 +5389,13 @@ getCompletionArgInfo(ASTNode anchor, constraints::ConstraintSystem &cs); /// \returns the bindings produced by performing this matching, or \c None if /// the match failed. Optional -matchCallArguments( - SmallVectorImpl &args, - ArrayRef params, - const ParameterListInfo ¶mInfo, - Optional unlabeledTrailingClosureIndex, - Optional completionInfo, - bool allowFixes, - MatchCallArgumentListener &listener, - Optional trailingClosureMatching); +matchCallArguments(SmallVectorImpl &args, + ArrayRef params, + const ParameterListInfo ¶mInfo, + Optional unlabeledTrailingClosureIndex, + Optional completionInfo, bool allowFixes, + MatchCallArgumentListener &listener, + Optional trailingClosureMatching); ConstraintSystem::TypeMatchResult matchCallArguments(ConstraintSystem &cs, diff --git a/lib/IDE/CodeCompletion.cpp b/lib/IDE/CodeCompletion.cpp index e20473acd4ccb..7d28837fe4540 100644 --- a/lib/IDE/CodeCompletion.cpp +++ b/lib/IDE/CodeCompletion.cpp @@ -6254,8 +6254,7 @@ static void addSuperKeyword(CodeCompletionResultSink &Sink, DeclContext *DC) { CodeCompletionResultBuilder Builder(Sink, CodeCompletionResult::ResultKind::Keyword, - SemanticContextKind::CurrentNominal, - {}); + SemanticContextKind::CurrentNominal, {}); if (auto *AFD = dyn_cast(DC)) { if (AFD->getOverriddenDecl() != nullptr) { Builder.addFlair(CodeCompletionFlairBit::CommonKeywordAtCurrentPosition); @@ -6750,9 +6749,10 @@ static void deliverCompletionResults(CodeCompletionContext &CompletionContext, /// match the lookup results against. /// /// \Returns true if global lookup should be performed. -static bool addPossibleParams( - const ArgumentTypeCheckCompletionCallback::Result &Ret, - SmallVectorImpl &Params, SmallVectorImpl &Types) { +static bool +addPossibleParams(const ArgumentTypeCheckCompletionCallback::Result &Ret, + SmallVectorImpl &Params, + SmallVectorImpl &Types) { if (!Ret.ParamIdx) return true; @@ -6777,7 +6777,7 @@ static bool addPossibleParams( assert(PL->size() == ParamsToPass.size()); bool ShowGlobalCompletions = false; - for (auto Idx: range(*Ret.ParamIdx, PL->size())) { + for (auto Idx : range(*Ret.ParamIdx, PL->size())) { bool IsCompletion = Idx == Ret.ParamIdx; // Stop at the first param claimed by other arguments. @@ -6785,11 +6785,14 @@ static bool addPossibleParams( break; const AnyFunctionType::Param *P = &ParamsToPass[Idx]; - bool Required = !PL->get(Idx)->isDefaultArgument() && !PL->get(Idx)->isVariadic(); + bool Required = + !PL->get(Idx)->isDefaultArgument() && !PL->get(Idx)->isVariadic(); if (P->hasLabel() && !(IsCompletion && Ret.IsNoninitialVariadic)) { PossibleParamInfo PP(P, Required); - if (!llvm::any_of(Params, [&](PossibleParamInfo &Existing){ return PP == Existing; })) + if (!llvm::any_of(Params, [&](PossibleParamInfo &Existing) { + return PP == Existing; + })) Params.push_back(std::move(PP)); } else { ShowGlobalCompletions = true; @@ -6815,7 +6818,7 @@ static void deliverArgumentResults( if (IncludeSignature && !Results.empty()) { Lookup.setHaveLParen(true); - for (auto &Result: Results) { + for (auto &Result : Results) { auto SemanticContext = SemanticContextKind::None; NominalTypeDecl *BaseNominal = nullptr; Type BaseTy = Result.BaseType; @@ -6824,7 +6827,9 @@ static void deliverArgumentResults( BaseTy = InstanceTy; if ((BaseNominal = BaseTy->getAnyNominal())) { SemanticContext = SemanticContextKind::CurrentNominal; - if (Result.FuncD && Result.FuncD->getDeclContext()->getSelfNominalTypeDecl() != BaseNominal) + if (Result.FuncD && + Result.FuncD->getDeclContext()->getSelfNominalTypeDecl() != + BaseNominal) SemanticContext = SemanticContextKind::Super; } else if (BaseTy->is()) { SemanticContext = SemanticContextKind::CurrentNominal; @@ -6847,7 +6852,7 @@ static void deliverArgumentResults( !Lookup.FoundFunctionCalls || Lookup.FoundFunctionsWithoutFirstKeyword; } else if (!Results.empty()) { SmallVector Params; - for (auto &Ret: Results) { + for (auto &Ret : Results) { shouldPerformGlobalCompletion |= addPossibleParams(Ret, Params, ExpectedTypes); } @@ -6855,7 +6860,7 @@ static void deliverArgumentResults( } if (shouldPerformGlobalCompletion) { - for (auto &Result: Results) { + for (auto &Result : Results) { ExpectedTypes.push_back(Result.ExpectedType); } Lookup.setExpectedTypes(ExpectedTypes, false); @@ -7040,15 +7045,16 @@ bool CodeCompletionCallbacksImpl::trySolverCompletion(bool MaybeFuncBody) { assert(CodeCompleteTokenExpr); assert(CurDeclContext); ArgumentTypeCheckCompletionCallback Lookup(CodeCompleteTokenExpr); - llvm::SaveAndRestore - CompletionCollector(Context.CompletionCallback, &Lookup); + llvm::SaveAndRestore CompletionCollector( + Context.CompletionCallback, &Lookup); typeCheckContextAt(CurDeclContext, CompletionLoc); if (!Lookup.gotCallback()) Lookup.fallbackTypeCheck(CurDeclContext); - deliverArgumentResults(Lookup.getResults(), ShouldCompleteCallPatternAfterParen, - CompletionLoc, CurDeclContext, CompletionContext, Consumer); + deliverArgumentResults(Lookup.getResults(), + ShouldCompleteCallPatternAfterParen, CompletionLoc, + CurDeclContext, CompletionContext, Consumer); return true; } default: diff --git a/lib/Sema/CSFix.cpp b/lib/Sema/CSFix.cpp index 90b886ba3a755..288c452124eca 100644 --- a/lib/Sema/CSFix.cpp +++ b/lib/Sema/CSFix.cpp @@ -1183,8 +1183,8 @@ bool SkipUnhandledConstructInResultBuilder::diagnose(const Solution &solution, } IgnoreFailureAfterCompletionArg * -IgnoreFailureAfterCompletionArg:: create(ConstraintSystem &cs, - ConstraintLocator *locator) { +IgnoreFailureAfterCompletionArg::create(ConstraintSystem &cs, + ConstraintLocator *locator) { return new (cs.getAllocator()) IgnoreFailureAfterCompletionArg(cs, locator); } diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index 0faf30c822ebe..380c6d2f1e731 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -42,7 +42,9 @@ MatchCallArgumentListener::~MatchCallArgumentListener() { } bool MatchCallArgumentListener::extraArgument(unsigned argIdx) { return true; } -bool MatchCallArgumentListener::invalidArgumentsAfterCompletion() { return true; } +bool MatchCallArgumentListener::invalidArgumentsAfterCompletion() { + return true; +} Optional MatchCallArgumentListener::missingArgument(unsigned paramIdx, @@ -167,10 +169,10 @@ static bool areConservativelyCompatibleArgumentLabels( auto params = fnType->getParams(); ParameterListInfo paramInfo(params, decl, hasAppliedSelf); - return matchCallArguments(args, params, paramInfo, - unlabeledTrailingClosureArgIndex, - /*completionInfo*/None, /*allow fixes*/ false, - listener, None).hasValue(); + return matchCallArguments( + args, params, paramInfo, unlabeledTrailingClosureArgIndex, + /*completionInfo*/ None, /*allow fixes*/ false, listener, None) + .hasValue(); } Expr *constraints::getArgumentLabelTargetExpr(Expr *fn) { @@ -263,14 +265,11 @@ static bool isCodeCompletionTypeVar(Type type) { return false; } - static bool matchCallArgumentsImpl( SmallVectorImpl &args, - ArrayRef params, - const ParameterListInfo ¶mInfo, + ArrayRef params, const ParameterListInfo ¶mInfo, Optional unlabeledTrailingClosureArgIndex, - Optional completionInfo, - bool allowFixes, + Optional completionInfo, bool allowFixes, TrailingClosureMatching trailingClosureMatching, MatchCallArgumentListener &listener, SmallVectorImpl ¶meterBindings) { @@ -1007,14 +1006,11 @@ static bool requiresBothTrailingClosureDirections( return false; } -Optional -constraints::matchCallArguments( +Optional constraints::matchCallArguments( SmallVectorImpl &args, - ArrayRef params, - const ParameterListInfo ¶mInfo, + ArrayRef params, const ParameterListInfo ¶mInfo, Optional unlabeledTrailingClosureArgIndex, - Optional completionInfo, - bool allowFixes, + Optional completionInfo, bool allowFixes, MatchCallArgumentListener &listener, Optional trailingClosureMatching) { @@ -1081,7 +1077,7 @@ constraints::matchCallArguments( } bool CompletionArgInfo::allowsMissingArgAt(unsigned argInsertIdx, - AnyFunctionType::Param param) { + AnyFunctionType::Param param) { // If the argument is before or at the index of the argument containing the // completion, the user would likely have already written it if they // intended this overload. @@ -1097,7 +1093,7 @@ bool CompletionArgInfo::allowsMissingArgAt(unsigned argInsertIdx, Type expectedTy = param.getPlainType()->lookThroughAllOptionalTypes(); return expectedTy->is() || expectedTy->isAny() || - expectedTy->isTypeVariableOrMember(); + expectedTy->isTypeVariableOrMember(); } return true; } @@ -1118,11 +1114,11 @@ constraints::getCompletionArgInfo(ASTNode anchor, ConstraintSystem &CS) { auto trailingIdx = arg->getUnlabeledTrailingClosureIndexOfPackedArgument(); if (auto *PE = dyn_cast(arg)) { if (CS.containsCodeCompletionLoc(PE->getSubExpr())) - return CompletionArgInfo{ 0, trailingIdx, 1}; + return CompletionArgInfo{0, trailingIdx, 1}; } else if (auto *TE = dyn_cast(arg)) { for (unsigned i: indices(TE->getElements())) { if (CS.containsCodeCompletionLoc(TE->getElement(i))) - return CompletionArgInfo{ i, trailingIdx, TE->getNumElements() }; + return CompletionArgInfo{i, trailingIdx, TE->getNumElements()}; } } return None; @@ -1168,8 +1164,9 @@ class ArgumentFailureTracker : public MatchCallArgumentListener { if (CS.isForCodeCompletion()) { if (!CompletionArgInfo) CompletionArgInfo = getCompletionArgInfo(Locator.getAnchor(), CS); - isAfterCodeCompletionLoc = CompletionArgInfo && - CompletionArgInfo->allowsMissingArgAt(argInsertIdx, param); + isAfterCodeCompletionLoc = + CompletionArgInfo && + CompletionArgInfo->allowsMissingArgAt(argInsertIdx, param); } auto *argLoc = CS.getConstraintLocator( @@ -1210,7 +1207,8 @@ class ArgumentFailureTracker : public MatchCallArgumentListener { // produce diagnostics) so no need to record a fix for this. Still increase // the score to penalize this solution though. auto *locator = CS.getConstraintLocator(Locator); - auto *fix = IgnoreFailureAfterCompletionArg::create(CS, CS.getConstraintLocator(locator->getAnchor())); + auto *fix = IgnoreFailureAfterCompletionArg::create( + CS, CS.getConstraintLocator(locator->getAnchor())); return CS.recordFix(fix, /*impact=*/1); } @@ -3257,7 +3255,6 @@ ConstraintSystem::matchTypesBindTypeVar( : getTypeMatchFailure(locator); } - auto loc = getConstraintLocator(locator); assignFixedType(typeVar, type, loc, /*updateState=*/true, /*notifyInference=*/!flags.contains(TMF_BindingTypeVariable)); @@ -6228,8 +6225,9 @@ ConstraintSystem::simplifyConstructionConstraint( /// /// \returns a locator pointing to the containing call/subscript if it meets /// these conditions and \c nullptr otherwise -static ConstraintLocator *isInArgAfterCompletion(ConstraintLocator *loc, ConstraintSystem &cs) { - auto fixExpr = simplifyLocatorToAnchor(loc).dyn_cast(); +static ConstraintLocator *isInArgAfterCompletion(ConstraintLocator *loc, + ConstraintSystem &cs) { + auto fixExpr = simplifyLocatorToAnchor(loc).dyn_cast(); auto commonAncestor = fixExpr; auto fixChild = fixExpr; while (commonAncestor && !cs.containsCodeCompletionLoc(commonAncestor)) { @@ -6245,7 +6243,7 @@ static ConstraintLocator *isInArgAfterCompletion(ConstraintLocator *loc, Constra auto Parent = cs.getParentExpr(TE); if (!Parent || (!isa(Parent) && !isa(Parent))) return nullptr; - for (auto *E: TE->getElements()) { + for (auto *E : TE->getElements()) { if (E == fixChild) // found locator element first return nullptr; if (cs.containsCodeCompletionLoc(E)) // found completion element first @@ -7303,8 +7301,8 @@ performMemberLookup(ConstraintKind constraintKind, DeclNameRef memberName, // subscript without a `keyPath:` label. Add it to the result so that it // can be caught by the missing argument label checking later. if (isForKeyPathSubscript(*this, memberLocator) || - (mayBeForKeyPathSubscriptWithoutLabel(*this, memberLocator) - && includeInaccessibleMembers)) { + (mayBeForKeyPathSubscriptWithoutLabel(*this, memberLocator) && + includeInaccessibleMembers)) { if (baseTy->isAnyObject()) { result.addUnviable( OverloadChoice(baseTy, OverloadChoiceKind::KeyPathApplication), @@ -9114,7 +9112,8 @@ bool ConstraintSystem::resolveClosure(TypeVariableType *typeVar, auto closureType = FunctionType::get(parameters, inferredClosureType->getResult(), closureExtInfo); - assignFixedType(typeVar, closureType, closureLocator, closureLocator); + assignFixedType(typeVar, closureType, closureLocator, + /*updateState*/closureLocator); // If there is a result builder to apply, do so now. if (resultBuilderType) { @@ -9883,7 +9882,7 @@ ConstraintSystem::simplifyKeyPathApplicationConstraint( recordAnyTypeVarAsPotentialHole(valueTy); return SolutionKind::Solved; } - + if (auto bgt = keyPathTy->getAs()) { // We have the key path type. Match it to the other ends of the constraint. auto kpRootTy = bgt->getGenericArgs()[0]; diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index 420de710a53bd..bbe75848e97d4 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -214,7 +214,8 @@ void ConstraintSystem::applySolution(const Solution &solution) { // If we don't already have a fixed type for this type variable, // assign the fixed type from the solution. if (!getFixedType(binding.first) && !binding.second->hasTypeVariable()) - assignFixedType(binding.first, binding.second, /*loc=*/nullptr, /*updateState=*/false); + assignFixedType(binding.first, binding.second, /*loc=*/nullptr, + /*updateState=*/false); } // Register overload choices. diff --git a/lib/Sema/TypeCheckCodeCompletion.cpp b/lib/Sema/TypeCheckCodeCompletion.cpp index f58f0064518f9..851341fb2c410 100644 --- a/lib/Sema/TypeCheckCodeCompletion.cpp +++ b/lib/Sema/TypeCheckCodeCompletion.cpp @@ -1132,8 +1132,7 @@ swift::lookupSemanticMember(DeclContext *DC, Type ty, DeclName name) { return TypeChecker::lookupMember(DC, ty, DeclNameRef(name), None); } -void TypeCheckCompletionCallback:: -fallbackTypeCheck(DeclContext *DC) { +void TypeCheckCompletionCallback::fallbackTypeCheck(DeclContext *DC) { assert(!GotCallback); CompletionContextFinder finder(DC); @@ -1148,7 +1147,7 @@ fallbackTypeCheck(DeclContext *DC) { CTP_Unused, Type(), /*isDiscared=*/true); TypeChecker::typeCheckForCodeCompletion( - completionTarget, /*needsPrecheck*/true, + completionTarget, /*needsPrecheck*/ true, [&](const Solution &S) { sawSolution(S); }); } @@ -1373,8 +1372,8 @@ void KeyPathTypeCheckCompletionCallback::sawSolution( } } -void ArgumentTypeCheckCompletionCallback:: -sawSolution(const constraints::Solution &S) { +void ArgumentTypeCheckCompletionCallback::sawSolution( + const constraints::Solution &S) { TypeCheckCompletionCallback::sawSolution(S); Type ExpectedTy = getTypeForCompletion(S, CompletionExpr); @@ -1411,11 +1410,13 @@ sawSolution(const constraints::Solution &S) { // argument (e.g. a KeyPath vs WritableKeyPath) so it ends up as a hole. // Just assume KeyPath so we show the expected keypath's root type to users // rather than '_'. - if (SelectedOverload->choice.getKind() == OverloadChoiceKind::KeyPathApplication) { + if (SelectedOverload->choice.getKind() == + OverloadChoiceKind::KeyPathApplication) { auto Params = FuncTy->getAs()->getParams(); if (Params.size() == 1 && Params[0].getPlainType()->is()) { auto *KPDecl = CS.getASTContext().getKeyPathDecl(); - Type KPTy = KPDecl->mapTypeIntoContext(KPDecl->getDeclaredInterfaceType()); + Type KPTy = + KPDecl->mapTypeIntoContext(KPDecl->getDeclaredInterfaceType()); Type KPValueTy = KPTy->castTo()->getGenericArgs()[1]; KPTy = BoundGenericType::get(KPDecl, Type(), {BaseTy, KPValueTy}); FuncTy = FunctionType::get({Params[0].withType(KPTy)}, KPValueTy); @@ -1424,8 +1425,10 @@ sawSolution(const constraints::Solution &S) { auto ArgInfo = getCompletionArgInfo(ParentCall, CS); auto ArgIdx = ArgInfo->completionIdx; - auto Bindings = S.argumentMatchingChoices.find(CS.getConstraintLocator( - Locator, ConstraintLocator::ApplyArgument))->second.parameterBindings; + auto Bindings = S.argumentMatchingChoices + .find(CS.getConstraintLocator( + Locator, ConstraintLocator::ApplyArgument)) + ->second.parameterBindings; // Find the parameter the completion was bound to (if any), as well as which // parameters are already bound (so we don't suggest them even when the args @@ -1434,15 +1437,14 @@ sawSolution(const constraints::Solution &S) { SmallVector ClaimedParams; bool IsNoninitialVariadic = false; - for (auto i: indices(Bindings)) { + for (auto i : indices(Bindings)) { bool Claimed = false; - for (auto j: Bindings[i]) { + for (auto j : Bindings[i]) { if (j == ArgIdx) { assert(!ParamIdx); ParamIdx = i; - IsNoninitialVariadic = llvm::any_of(Bindings[i], [j](unsigned other) { - return other < j; - }); + IsNoninitialVariadic = llvm::any_of( + Bindings[i], [j](unsigned other) { return other < j; }); } // Synthesized args don't count. if (j < ArgInfo->argCount) @@ -1459,22 +1461,23 @@ sawSolution(const constraints::Solution &S) { // If this is a duplicate of any other result, ignore this solution. if (llvm::any_of(Results, [&](const Result &R) { - if (R.FuncD != FuncD) - return false; - if (!R.FuncTy->isEqual(FuncTy)) - return false; - if (R.BaseType) { - if (!BaseTy || !BaseTy->isEqual(R.BaseType)) - return false; - } else if (BaseTy) { - return false; - } - return R.ParamIdx == ParamIdx && - R.IsNoninitialVariadic == IsNoninitialVariadic; - })) { + if (R.FuncD != FuncD) + return false; + if (!R.FuncTy->isEqual(FuncTy)) + return false; + if (R.BaseType) { + if (!BaseTy || !BaseTy->isEqual(R.BaseType)) + return false; + } else if (BaseTy) { + return false; + } + return R.ParamIdx == ParamIdx && + R.IsNoninitialVariadic == IsNoninitialVariadic; + })) { return; } - Results.push_back({ExpectedTy, isa(ParentCall), FuncD, FuncTy, ArgIdx, ParamIdx, - std::move(ClaimedParams), IsNoninitialVariadic, BaseTy, HasLabel}); + Results.push_back({ExpectedTy, isa(ParentCall), FuncD, FuncTy, + ArgIdx, ParamIdx, std::move(ClaimedParams), + IsNoninitialVariadic, BaseTy, HasLabel}); } diff --git a/lib/Sema/TypeCheckConstraints.cpp b/lib/Sema/TypeCheckConstraints.cpp index c86141f33e779..f53c96a907d57 100644 --- a/lib/Sema/TypeCheckConstraints.cpp +++ b/lib/Sema/TypeCheckConstraints.cpp @@ -57,9 +57,9 @@ void TypeVariableType::Implementation::print(llvm::raw_ostream &OS) { } SavedTypeVariableBinding::SavedTypeVariableBinding(TypeVariableType *typeVar) - : TypeVar(typeVar), Options(typeVar->getImpl().getRawOptions()), - ParentOrFixed(typeVar->getImpl().ParentOrFixed), - BoundLocator(typeVar->getImpl().BoundLocator) { } + : TypeVar(typeVar), Options(typeVar->getImpl().getRawOptions()), + ParentOrFixed(typeVar->getImpl().ParentOrFixed), + BoundLocator(typeVar->getImpl().BoundLocator) {} void SavedTypeVariableBinding::restore() { TypeVar->getImpl().setRawOptions(Options); From 3808cb1b00305128ec7b54985e4e2eb52ac2c50b Mon Sep 17 00:00:00 2001 From: Nathan Hawes Date: Wed, 4 Aug 2021 14:18:32 +1000 Subject: [PATCH 6/7] add subscript tests + stress tester assertion hit fixes --- include/swift/Sema/ConstraintSystem.h | 4 +- lib/IDE/CodeCompletion.cpp | 35 ++-- lib/Sema/BuilderTransform.cpp | 6 + lib/Sema/ConstraintSystem.cpp | 4 +- lib/Sema/TypeCheckCodeCompletion.cpp | 108 ++++++++---- test/IDE/complete_call_arg.swift | 159 ++++++++++++++++-- ..._enum_unresolved_dot_argument_labels.swift | 54 +++++- 7 files changed, 297 insertions(+), 73 deletions(-) diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 6cad52ec0b645..3e2c44afec55e 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -3069,9 +3069,9 @@ class ConstraintSystem { return TypeVariables.count(typeVar) > 0; } - /// Whether the given expression's source range contains the code + /// Whether the given ASTNode's source range contains the code /// completion location. - bool containsCodeCompletionLoc(Expr *expr) const; + bool containsCodeCompletionLoc(ASTNode node) const; void setClosureType(const ClosureExpr *closure, FunctionType *type) { assert(closure); diff --git a/lib/IDE/CodeCompletion.cpp b/lib/IDE/CodeCompletion.cpp index 7d28837fe4540..c33dcdc41c96e 100644 --- a/lib/IDE/CodeCompletion.cpp +++ b/lib/IDE/CodeCompletion.cpp @@ -6764,20 +6764,24 @@ addPossibleParams(const ArgumentTypeCheckCompletionCallback::Result &Ret, ArrayRef ParamsToPass = Ret.FuncTy->getAs()->getParams(); - ParameterList *PL = nullptr; - if (auto *FD = dyn_cast(Ret.FuncD)) { - PL = FD->getParameters(); - } else if (auto *SD = dyn_cast(Ret.FuncD)) { - PL = SD->getIndices(); - } else { - assert(false && "Unhandled decl type?"); - return true; + ParameterList *PL = nullptr; + if (Ret.FuncD) { + if (auto *FD = dyn_cast(Ret.FuncD)) { + PL = FD->getParameters(); + } else if (auto *SD = dyn_cast(Ret.FuncD)) { + PL = SD->getIndices(); + } else if (auto *EED = dyn_cast(Ret.FuncD)) { + PL = EED->getParameterList(); + } else { + // No parameter list for function-type params/variables. + assert(isa(Ret.FuncD) && "Unhandled decl type?"); + } } - assert(PL->size() == ParamsToPass.size()); + assert(!PL || PL->size() == ParamsToPass.size()); bool ShowGlobalCompletions = false; - for (auto Idx : range(*Ret.ParamIdx, PL->size())) { + for (auto Idx : range(*Ret.ParamIdx, ParamsToPass.size())) { bool IsCompletion = Idx == Ret.ParamIdx; // Stop at the first param claimed by other arguments. @@ -6786,7 +6790,7 @@ addPossibleParams(const ArgumentTypeCheckCompletionCallback::Result &Ret, const AnyFunctionType::Param *P = &ParamsToPass[Idx]; bool Required = - !PL->get(Idx)->isDefaultArgument() && !PL->get(Idx)->isVariadic(); + !(PL && PL->get(Idx)->isDefaultArgument()) && !P->isVariadic(); if (P->hasLabel() && !(IsCompletion && Ret.IsNoninitialVariadic)) { PossibleParamInfo PP(P, Required); @@ -6821,8 +6825,8 @@ static void deliverArgumentResults( for (auto &Result : Results) { auto SemanticContext = SemanticContextKind::None; NominalTypeDecl *BaseNominal = nullptr; - Type BaseTy = Result.BaseType; - if (BaseTy) { + if (Result.BaseType) { + Type BaseTy = Result.BaseType; if (auto InstanceTy = BaseTy->getMetatypeInstanceType()) BaseTy = InstanceTy; if ((BaseNominal = BaseTy->getAnyNominal())) { @@ -6831,12 +6835,13 @@ static void deliverArgumentResults( Result.FuncD->getDeclContext()->getSelfNominalTypeDecl() != BaseNominal) SemanticContext = SemanticContextKind::Super; - } else if (BaseTy->is()) { + } else if (BaseTy->is() || + BaseTy->is()) { SemanticContext = SemanticContextKind::CurrentNominal; } } if (Result.IsSubscript) { - assert(SemanticContext != SemanticContextKind::None && BaseTy); + assert(SemanticContext != SemanticContextKind::None); auto *SD = dyn_cast_or_null(Result.FuncD); Lookup.addSubscriptCallPattern(Result.FuncTy->getAs(), SD, SemanticContext); diff --git a/lib/Sema/BuilderTransform.cpp b/lib/Sema/BuilderTransform.cpp index 8660d22bd17a2..90207b1090d5f 100644 --- a/lib/Sema/BuilderTransform.cpp +++ b/lib/Sema/BuilderTransform.cpp @@ -1787,6 +1787,12 @@ ConstraintSystem::matchResultBuilder(AnyFunctionRef fn, Type builderType, return getTypeMatchFailure(locator); } + // If we're solving for code completion and the body contains the code + // completion location, skipping it won't get us to a useful solution so + // just bail. + if (isForCodeCompletion() && containsCodeCompletionLoc(fn.getBody())) + return getTypeMatchFailure(locator); + // Record the first unhandled construct as a fix. if (recordFix( SkipUnhandledConstructInResultBuilder::create( diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index 313d568bfd7d1..afe75f554e4ff 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -378,8 +378,8 @@ getAlternativeLiteralTypes(KnownProtocolKind kind) { return *AlternativeLiteralTypes[index]; } -bool ConstraintSystem::containsCodeCompletionLoc(Expr *expr) const { - SourceRange range = expr->getSourceRange(); +bool ConstraintSystem::containsCodeCompletionLoc(ASTNode node) const { + SourceRange range = node.getSourceRange(); if (range.isInvalid()) return false; return Context.SourceMgr.rangeContainsCodeCompletionLoc(range); diff --git a/lib/Sema/TypeCheckCodeCompletion.cpp b/lib/Sema/TypeCheckCodeCompletion.cpp index 851341fb2c410..a003e4f7ce6be 100644 --- a/lib/Sema/TypeCheckCodeCompletion.cpp +++ b/lib/Sema/TypeCheckCodeCompletion.cpp @@ -833,11 +833,33 @@ static bool isForPatternMatch(SolutionApplicationTarget &target) { return false; } +static bool hasTypeForCompletion(Solution &solution, CompletionContextFinder &contextAnalyzer) { + if (contextAnalyzer.hasCompletionExpr()) + return solution.hasType(contextAnalyzer.getCompletionExpr()); + + assert(contextAnalyzer.hasCompletionKeyPathComponent()); + return solution.hasType( + contextAnalyzer.getKeyPathContainingCompletionComponent(), + contextAnalyzer.getKeyPathCompletionComponentIndex()); +} + /// Remove any solutions from the provided vector that both require fixes and have a /// score worse than the best. static void filterSolutions(SolutionApplicationTarget &target, SmallVectorImpl &solutions, - CodeCompletionExpr *completionExpr) { + CompletionContextFinder &contextAnalyzer) { + // Ignore solutions that didn't end up involving the completion (e.g. due to + // a fix to skip over/ignore it). + llvm::erase_if(solutions, [&](Solution &S) { + if (hasTypeForCompletion(S, contextAnalyzer)) + return false; + // Catch these in assert builds as solving for code completion shouldn't + // allow fixes that skip over or ignore the completion expression. + // Continuing on to produce a solution after such fixes is pointless. + assert(false && "solution doesn't involve completion"); + return true; + }); + // FIXME: this is only needed because in pattern matching position, the // code completion expression always becomes an expression pattern, which // requires the ~= operator to be defined on the type being matched against. @@ -852,8 +874,9 @@ static void filterSolutions(SolutionApplicationTarget &target, // the other formed solutions (which require fixes). We should generate enum // pattern completions separately, but for now ignore the // _OptionalNilComparisonType solution. - if (isForPatternMatch(target) && completionExpr) { - solutions.erase(llvm::remove_if(solutions, [&](const Solution &S) { + if (isForPatternMatch(target) && contextAnalyzer.hasCompletionExpr()) { + auto completionExpr = contextAnalyzer.getCompletionExpr(); + llvm::erase_if(solutions, [&](const Solution &S) { ASTContext &ctx = S.getConstraintSystem().getASTContext(); if (!S.hasType(completionExpr)) return false; @@ -861,7 +884,7 @@ static void filterSolutions(SolutionApplicationTarget &target, if (auto *NTD = ty->getAnyNominal()) return NTD->getBaseIdentifier() == ctx.Id_OptionalNilComparisonType; return false; - }), solutions.end()); + }); } if (solutions.size() <= 1) @@ -965,15 +988,6 @@ bool TypeChecker::typeCheckForCodeCompletion( if (!cs.solveForCodeCompletion(target, solutions)) return CompletionResult::Fallback; - // FIXME: instead of filtering, expose the score and viability to clients. - // Remove any solutions that both require fixes and have a score that is - // worse than the best. - CodeCompletionExpr *completionExpr = nullptr; - if (contextAnalyzer.hasCompletionExpr()) { - completionExpr = contextAnalyzer.getCompletionExpr(); - } - filterSolutions(target, solutions, completionExpr); - // Similarly, if the type-check didn't produce any solutions, fall back // to type-checking a sub-expression in isolation. if (solutions.empty()) @@ -983,19 +997,7 @@ bool TypeChecker::typeCheckForCodeCompletion( // closure body it could either be type-checked together with the context // or not, it's impossible to say without checking. if (contextAnalyzer.locatedInMultiStmtClosure()) { - auto &solution = solutions.front(); - - bool HasTypeForCompletionNode = false; - if (completionExpr) { - HasTypeForCompletionNode = solution.hasType(completionExpr); - } else { - assert(contextAnalyzer.hasCompletionKeyPathComponent()); - HasTypeForCompletionNode = solution.hasType( - contextAnalyzer.getKeyPathContainingCompletionComponent(), - contextAnalyzer.getKeyPathCompletionComponentIndex()); - } - - if (!HasTypeForCompletionNode) { + if (!hasTypeForCompletion(solutions.front(), contextAnalyzer)) { // At this point we know the code completion node wasn't checked with // the closure's surrounding context, so can defer to regular // type-checking for the current call to typeCheckExpression. If that @@ -1008,6 +1010,11 @@ bool TypeChecker::typeCheckForCodeCompletion( } } + // FIXME: instead of filtering, expose the score and viability to clients. + // Remove solutions that skipped over/ignored the code completion point + // or that require fixes and have a score that is worse than the best. + filterSolutions(target, solutions, contextAnalyzer); + llvm::for_each(solutions, callback); return CompletionResult::Ok; }; @@ -1372,6 +1379,19 @@ void KeyPathTypeCheckCompletionCallback::sawSolution( } } +static bool isInPatternMatch(CodeCompletionExpr *CE, ConstraintSystem &CS) { + if (auto *TE = dyn_cast_or_null(CS.getParentExpr(CE))) { + if (!TE->isImplicit() || TE->getNumElements() != 2 || + TE->getElement(0) != CE) + return false; + if (auto *DRE = dyn_cast(TE->getElement(1))) { + auto Name = DRE->getDecl()->getName(); + return Name && Name.isSimpleName("$match"); + } + } + return false; +} + void ArgumentTypeCheckCompletionCallback::sawSolution( const constraints::Solution &S) { TypeCheckCompletionCallback::sawSolution(S); @@ -1384,14 +1404,34 @@ void ArgumentTypeCheckCompletionCallback::sawSolution( Expr *ParentCall = CompletionExpr; while (ParentCall && - !(isa(ParentCall) || isa(ParentCall))) + !(isa(ParentCall) || isa(ParentCall))) ParentCall = CS.getParentExpr(ParentCall); if (!ParentCall || ParentCall == CompletionExpr) { + // FIXME: Handle switch case pattern matching: + // enum Foo { case member(x: Int, y: Int, z: Int) } + // switch someVal { + // case .member(let x, y: 2, #^HERE^# + // } + // The expression we get here is ~= $match by this point, so + // we need to dig out/track more info to suggest all the valid completions + // (i.e. `z:`, `let z`, `_`, any integer) and those completions don't + // fit well to call/subscript argument completion. For now just maintain + // the existing behavior and fall back to global completion. + if (!ParentCall && isInPatternMatch(CompletionExpr, CS)) + return; + assert(false && "no containing call?"); return; } + auto ArgInfo = getCompletionArgInfo(ParentCall, CS); + if (!ArgInfo) { + assert(false && "bad parent call match?"); + return; + } + auto ArgIdx = ArgInfo->completionIdx; + auto *Locator = CS.getConstraintLocator(ParentCall); auto *CalleeLocator = S.getCalleeLocator(Locator); auto SelectedOverload = S.getOverloadChoiceIfAvailable(CalleeLocator); @@ -1400,10 +1440,10 @@ void ArgumentTypeCheckCompletionCallback::sawSolution( Type BaseTy = SelectedOverload->choice.getBaseType(); if (BaseTy) - BaseTy = S.simplifyType(BaseTy); + BaseTy = S.simplifyType(BaseTy)->getRValueType(); ValueDecl *FuncD = SelectedOverload->choice.getDeclOrNull(); - Type FuncTy = S.simplifyType(SelectedOverload->openedType); + Type FuncTy = S.simplifyType(SelectedOverload->openedType)->getRValueType(); // For completion as the arg in a call to the implicit [keypath: _] subscript // the solver can't know what kind of keypath is expected without an actual @@ -1423,8 +1463,6 @@ void ArgumentTypeCheckCompletionCallback::sawSolution( } } - auto ArgInfo = getCompletionArgInfo(ParentCall, CS); - auto ArgIdx = ArgInfo->completionIdx; auto Bindings = S.argumentMatchingChoices .find(CS.getConstraintLocator( Locator, ConstraintLocator::ApplyArgument)) @@ -1465,12 +1503,10 @@ void ArgumentTypeCheckCompletionCallback::sawSolution( return false; if (!R.FuncTy->isEqual(FuncTy)) return false; - if (R.BaseType) { - if (!BaseTy || !BaseTy->isEqual(R.BaseType)) - return false; - } else if (BaseTy) { + if (R.BaseType.isNull() != BaseTy.isNull()) + return false; + if (R.BaseType && !R.BaseType->isEqual(BaseTy)) return false; - } return R.ParamIdx == ParamIdx && R.IsNoninitialVariadic == IsNoninitialVariadic; })) { diff --git a/test/IDE/complete_call_arg.swift b/test/IDE/complete_call_arg.swift index a774e6d3a1aa7..b8d46d4464487 100644 --- a/test/IDE/complete_call_arg.swift +++ b/test/IDE/complete_call_arg.swift @@ -253,8 +253,8 @@ extension C3 { func f7(obj: C3) { let _ = obj.hasError(#^HASERROR1^# - let _ = obj.hasError(a1: #^HASERROR2?xfail=FIXME^# - let _ = obj.hasError(a1: IC1, #^HASERROR3?xfail=FIXME^# + let _ = obj.hasError(a1: #^HASERROR2?xfail=SR-14992^# + let _ = obj.hasError(a1: IC1, #^HASERROR3?xfail=SR-14992^# let _ = obj.hasError(a1: IC1, b1: #^HASERROR4^# } } @@ -1010,10 +1010,8 @@ extension SR14737 where T == Int { func test_SR14737() { invalidCallee { SR14737(arg1: true, #^GENERIC_INIT_IN_INVALID^#) -// FIXME: 'onlyInt' shouldn't be offered because 'arg1' is Bool. -// GENERIC_INIT_IN_INVALID: Begin completions, 2 items +// GENERIC_INIT_IN_INVALID: Begin completions, 1 item // GENERIC_INIT_IN_INVALID-DAG: Pattern/Local/Flair[ArgLabels]: {#arg2: Bool#}[#Bool#]; -// GENERIC_INIT_IN_INVALID-DAG: Pattern/Local/Flair[ArgLabels]: {#onlyInt: Bool#}[#Bool#]; // GENERIC_INIT_IN_INVALID: End completions } } @@ -1032,8 +1030,13 @@ func testArgsAfterCompletion() { func overloaded(x: Int, _ first: A, _ second: B) {} func overloaded(x: Int, _ first: B, _ second: A) {} + struct SubOverloaded { + subscript(x x: Int, first: A, second: B) -> Int { return 1 } + subscript(x x: Int, first: B, second: A) -> Int { return 1} + } overloaded(x: 1, .#^VALID_UNRESOLVED^#, localB) + SubOverloaded()[x: 1, .#^VALID_UNRESOLVED_SUB?check=VALID_UNRESOLVED^#, localB] // VALID_UNRESOLVED: Begin completions, 2 items // VALID_UNRESOLVED-DAG: Decl[EnumElement]/CurrNominal/Flair[ExprSpecific]/TypeRelation[Identical]: a[#A#]; name=a @@ -1041,6 +1044,7 @@ func testArgsAfterCompletion() { // VALID_UNRESOLVED: End completions overloaded(x: 1, .#^VALID_UNRESOLVED_NOCOMMA^# localB) + SubOverloaded()[x: 1, .#^VALID_UNRESOLVED_NOCOMA_SUB?check=VALID_UNRESOLVED_NOCOMMA^# localB] // VALID_UNRESOLVED_NOCOMMA: Begin completions, 4 items // VALID_UNRESOLVED_NOCOMMA-DAG: Decl[EnumElement]/CurrNominal/Flair[ExprSpecific]/TypeRelation[Identical]: a[#A#]; name=a @@ -1049,46 +1053,76 @@ func testArgsAfterCompletion() { // VALID_UNRESOLVED_NOCOMMA-DAG: Decl[InstanceMethod]/CurrNominal/TypeRelation[Invalid]: hash({#(self): B#})[#(into: inout Hasher) -> Void#]; name=hash(:) // VALID_UNRESOLVED_NOCOMMA: End completions + overloaded(x: 1, .#^INVALID_UNRESOLVED?check=VALID_UNRESOLVED_NOCOMMA^#, "wrongType") + SubOverloaded()[x: 1, .#^INVALID_UNRESOLVED_SUB?check=VALID_UNRESOLVED_NOCOMMA^#, "wrongType"] + overloaded(x: 1, #^VALID_GLOBAL^#, localB) + SubOverloaded()[x: 1, #^VALID_GLOBAL_SUB?check=VALID_GLOBAL^#, localB] // VALID_GLOBAL: Begin completions // VALID_GLOBAL-DAG: Decl[LocalVar]/Local/TypeRelation[Identical]: localA[#A#]; name=localA // VALID_GLOBAL-DAG: Decl[LocalVar]/Local: localB[#B#]; name=localB // VALID_GLOBAL: End completions + func takesIntGivesB(_ x: Int) -> B { return B.b } + overloaded(x: 1, .#^VALID_NESTED?check=VALID_UNRESOLVED^#, takesIntGivesB(1)) + overloaded(x: 1, .#^INVALID_NESTED?check=VALID_UNRESOLVED_NOCOMMA^#, takesIntGivesB("string")) + func overloadedLabel(x: Int, firstA: A, second: B) {} func overloadedLabel(x: Int, firstB: B, second: A) {} + struct SubOverloadedLabel { + subscript(x x: Int, firstA firstA: A, second second: B) -> Int { return 1 } + subscript(x x: Int, firstB firstB: B, second second: A) -> Int { return 1 } + } overloadedLabel(x: 1, #^VALID_LABEL^#, second: localB) + SubOverloadedLabel()[x: 1, #^VALID_LABEL_SUB?check=VALID_LABEL^#, second: localB] // VALID_LABEL: Begin completions, 1 items // VALID_LABEL: Pattern/Local/Flair[ArgLabels]: {#firstA: A#}[#A#]; name=firstA: // VALID_LABEL: End completions overloadedLabel(x: 1, #^VALID_LABEL_NOCOMMA^# second: localB) + SubOverloadedLabel()[x: 1, #^VALID_LABEL_NOCOMMA_SUB?check=VALID_LABEL_NOCOMMA^# second: localB] + // VALID_LABEL_NOCOMMA: Begin completions, 2 items // VALID_LABEL_NOCOMMA-DAG: Pattern/Local/Flair[ArgLabels]: {#firstA: A#}[#A#]; name=firstA: // VALID_LABEL_NOCOMMA-DAG: Pattern/Local/Flair[ArgLabels]: {#firstB: B#}[#B#]; name=firstB: // VALID_LABEL_NOCOMMA: End completions + // The parser eats the existing localB arg, so we still suggest both labels here. + overloadedLabel(x: 1, #^VALID_LABEL_NOCOMMA_NOLABEL?check=VALID_LABEL_NOCOMMA^# localB) + SubOverloadedLabel()[x: 1, #^VALID_LABEL_NOCOMMA_NOLABEL_SUB?check=VALID_LABEL_NOCOMMA^# localB] + overloadedLabel(x: 1, #^INVALID_LABEL^#, wrongLabelRightType: localB) + SubOverloadedLabel()[x: 1, #^INVALID_LABEL_SUB?check=INVALID_LABEL^#, wrongLabelRightType: localB] // INVALID_LABEL: Begin completions, 2 items // INVALID_LABEL-DAG: Pattern/Local/Flair[ArgLabels]: {#firstA: A#}[#A#]; name=firstA: // INVALID_LABEL-DAG: Pattern/Local/Flair[ArgLabels]: {#firstB: B#}[#B#]; name=firstB: // INVALID_LABEL: End completions overloadedLabel(x: 1, #^INVALID_TYPE?check=INVALID_LABEL^#, second: 34) - overloadedLabel(x: 1, #^INVALID_LABEL_TYPE?check=INVALID_LABEL^#, wrongLabelWrongType: 2) // invalid + SubOverloadedLabel()[x: 1, #^INVALID_TYPE_SUB?check=INVALID_LABEL^#, second: 34] + + overloadedLabel(x: 1, #^INVALID_LABEL_TYPE?check=INVALID_LABEL^#, wrongLabelWrongType: 2) + SubOverloadedLabel()[x: 1, #^INVALID_LABEL_TYPE_SUB?check=INVALID_LABEL^#, wrongLabelWrongType: 2] func overloadedArity(x: Int, firstA: A, second: B, third: Double) {} func overloadedArity(x: Int, firstB: B, second: A) {} + struct SubOverloadedArity { + subscript(x x: Int, firstA firstA: A, second second: B, third third: Double) -> Int { return 1} + subscript(x x: Int, firstB firstB: B, second second: A) -> Int { return 1} + } overloadedArity(x: 1, #^VALID_ARITY^#, second: localB, third: 4.5) + SubOverloadedArity()[x: 1, #^VALID_ARITY_SUB?check=VALID_ARITY^#, second: localB, third: 4.5] // VALID_ARITY: Begin completions, 1 items // VALID_ARITY: Pattern/Local/Flair[ArgLabels]: {#firstA: A#}[#A#]; name=firstA: // VALID_ARITY: End completions overloadedArity(x: 1, #^VALID_ARITY_NOCOMMA?check=VALID_ARITY^# second: localB, third: 4.5) + SubOverloadedArity()[x: 1, #^VALID_ARITY_NOCOMMA_SUB?check=VALID_ARITY^# second: localB, third: 4.5] overloadedArity(x: 1, #^INVALID_ARITY^#, wrong: localB) + SubOverloadedArity()[x: 1, #^INVALID_ARITY_SUB?check=INVALID_ARITY^#, wrong: localB] // INVALID_ARITY: Begin completions, 2 items // INVALID_ARITY-DAG: Pattern/Local/Flair[ArgLabels]: {#firstA: A#}[#A#]; name=firstA: // INVALID_ARITY-DAG: Pattern/Local/Flair[ArgLabels]: {#firstB: B#}[#B#]; name=firstB: @@ -1096,12 +1130,20 @@ func testArgsAfterCompletion() { // type mismatch in 'second' vs extra arg 'third'. overloadedArity(x: 1, #^INVALID_ARITY_TYPE?check=INVALID_ARITY^#, second: localA, third: 2.5) + SubOverloadedArity()[x: 1, #^INVALID_ARITY_TYPE_SUB?check=INVALID_ARITY^#, second: localA, third: 2.5] overloadedArity(x: 1, #^INVALID_ARITY_TYPE_2?check=INVALID_ARITY^#, second: localA, third: "wrong") + SubOverloadedArity()[x: 1, #^INVALID_ARITY_TYPE_2_SUB?check=INVALID_ARITY^#, second: localA, third: "wrong"] + func overloadedDefaulted(x: Int, p: A) {} func overloadedDefaulted(x: Int, y: A = A.a, z: A = A.a) {} + struct SubOverloadedDefaulted { + subscript(x x: Int, p p: A) -> Int { return 1 } + subscript(x x: Int, y y: A = A.a, z z: A = A.a) -> Int { return 1 } + } overloadedDefaulted(x: 1, #^VALID_DEFAULTED^#) + SubOverloadedDefaulted()[x: 1, #^VALID_DEFAULTED_SUB?check=VALID_DEFAULTED^#] // VALID_DEFAULTED: Begin completions, 3 items // VALID_DEFAULTED-DAG: Pattern/Local/Flair[ArgLabels]: {#p: A#}[#A#]; name=p: // VALID_DEFAULTED-DAG: Pattern/Local/Flair[ArgLabels]: {#y: A#}[#A#]; name=y: @@ -1109,6 +1151,7 @@ func testArgsAfterCompletion() { // VALID_DEFAULTED: End completions overloadedDefaulted(x: 1, #^VALID_DEFAULTED_AFTER^#, z: localA) + SubOverloadedDefaulted()[x: 1, #^VALID_DEFAULTED_AFTER_SUB?check=VALID_DEFAULTED_AFTER^#, z: localA] // VALID_DEFAULTED_AFTER: Begin completions, 1 items // VALID_DEFAULTED_AFTER-DAG: Pattern/Local/Flair[ArgLabels]: {#y: A#}[#A#]; name=y: // VALID_DEFAULTED_AFTER: End completions @@ -1117,24 +1160,118 @@ func testArgsAfterCompletion() { overloadedDefaulted(x: 1, #^INVALID_DEFAULTED?check=VALID_DEFAULTED^#, w: "hello") overloadedDefaulted(x: 1, #^INVALID_DEFAULTED_TYPO?check=VALID_DEFAULTED^#, zz: localA) overloadedDefaulted(x: 1, #^INVALID_DEFAULTED_TYPO_TYPE?check=VALID_DEFAULTED^#, zz: "hello") + SubOverloadedDefaulted()[x: 1, #^VALID_DEFAULTED_AFTER_NOCOMMA_SUB?check=VALID_DEFAULTED^# z: localA] + SubOverloadedDefaulted()[x: 1, #^INVALID_DEFAULTED_SUB?check=VALID_DEFAULTED^#, w: "hello"] + SubOverloadedDefaulted()[x: 1, #^INVALID_DEFAULTED_TYPO_SUB?check=VALID_DEFAULTED^#, zz: localA] + SubOverloadedDefaulted()[x: 1, #^INVALID_DEFAULTED_TYPO_TYPE_SUB?check=VALID_DEFAULTED^#, zz: "hello"] overloadedDefaulted(x: 1, #^INVALID_DEFAULTED_TYPE^#, z: localB) + SubOverloadedDefaulted()[x: 1, #^INVALID_DEFAULTED_TYPE_SUB?check=INVALID_DEFAULTED_TYPE^#, z: localB] // INVALID_DEFAULTED_TYPE: Begin completions, 2 items // INVALID_DEFAULTED_TYPE-DAG: Pattern/Local/Flair[ArgLabels]: {#p: A#}[#A#]; name=p: // INVALID_DEFAULTED_TYPE-DAG: Pattern/Local/Flair[ArgLabels]: {#y: A#}[#A#]; name=y: // INVALID_DEFAULTED_TYPE: End completions - func overloadedGeneric(x: Int, y: String, z: T, y: T) {} + func overloadedGeneric(x: Int, y: String, z: T, zz: T) {} func overloadedGeneric(x: Int, p: String, q: Int) {} + struct SubOverloadedGeneric { + subscript(x x: Int, y y: String, z z: T, zz zz: T) -> Int { return 1} + subscript(x x: Int, p p: String, q q: Int) -> Int { return 1 } + } + struct MissingConformance {} - overloadedGeneric(x: 1, #^INVALID_MISSINGCONFORMANCE^#, z: MissingConformance(), y: MissingConformance()) + overloadedGeneric(x: 1, #^INVALID_MISSINGCONFORMANCE^#, z: MissingConformance(), zz: MissingConformance()) + SubOverloadedGeneric()[x: 1, #^INVALID_MISSINGCONFORMANCE_SUB?check=INVALID_MISSINGCONFORMANCE^#, z: MissingConformance(), zz: MissingConformance()] // INVALID_MISSINGCONFORMANCE: Begin completions, 2 items // INVALID_MISSINGCONFORMANCE-DAG: Pattern/Local/Flair[ArgLabels]: {#p: String#}[#String#]; name=p: // INVALID_MISSINGCONFORMANCE-DAG: Pattern/Local/Flair[ArgLabels]: {#y: String#}[#String#]; name=y: // INVALID_MISSINGCONFORMANCE: End completions - overloadedGeneric(x: 1, #^INVALID_MISSINGCONFORMANCE_NOCOMMA?check=INVALID_MISSINGCONFORMANCE^# z: MisingConformance(), y: MissingConformance()) - overloadedGeneric(x: 1, #^INVALID_MISSINGCONFORMANCE_INDIRECT?check=INVALID_MISSINGCONFORMANCE^#, z: [MissingConformance()], y: [MissingConformance()]) + overloadedGeneric(x: 1, #^INVALID_MISSINGCONFORMANCE_NOCOMMA?check=INVALID_MISSINGCONFORMANCE^# z: MisingConformance(), zz: MissingConformance()) + overloadedGeneric(x: 1, #^INVALID_MISSINGCONFORMANCE_INDIRECT?check=INVALID_MISSINGCONFORMANCE^#, z: [MissingConformance()], zz: [MissingConformance()]) + overloadedGeneric(x: 1, #^INVALID_MISSINGCONFORMANCE_CONSTRAINT?check=INVALID_MISSINGCONFORMANCE^#, z: [CondConfType("foo")], zz: [CondConfType("bar")]) + SubOverloadedGeneric()[x: 1, #^INVALID_MISSINGCONFORMANCE_NOCOMMA_SUB?check=INVALID_MISSINGCONFORMANCE^# z: MisingConformance(), zz: MissingConformance()] + SubOverloadedGeneric()[x: 1, #^INVALID_MISSINGCONFORMANCE_INDIRECT_SUB?check=INVALID_MISSINGCONFORMANCE^#, z: [MissingConformance()], zz: [MissingConformance()]] + SubOverloadedGeneric()[x: 1, #^INVALID_MISSINGCONFORMANCE_CONSTRAINT_SUB?check=INVALID_MISSINGCONFORMANCE^#, z: [CondConfType("foo")], zz: [CondConfType("bar")]] +} + +func testFuncTyVars(param: (Int, String, Double) -> ()) { + var local = { (a: Int, b: String, c: Double) in } + + let someInt = 2 + let someString = "hello" + let someDouble = 3.5 + + param(2, #^PARAM_ARG2?check=FUNCTY_STRING^#) + local(2, #^LOCAL_ARG2?check=FUNCTY_STRING^#) + param(2, "hello", #^PARAM_ARG3?check=FUNCTY_DOUBLE^#) + local(2, "hello", #^LOCAL_ARG3?check=FUNCTY_DOUBLE^#) + + // FUNCTY_STRING: Begin completions + // FUNCTY_STRING-DAG: Decl[LocalVar]/Local/TypeRelation[Identical]: someString[#String#]; + // FUNCTY_STRING-DAG: Decl[LocalVar]/Local: someDouble[#Double#]; + // FUNCTY_STRING-DAG: Decl[LocalVar]/Local: someInt[#Int#]; + // FUNCTY_STRING: End completions + + // FUNCTY_DOUBLE: Begin completions + // FUNCTY_DOUBLE-DAG: Decl[LocalVar]/Local/TypeRelation[Identical]: someDouble[#Double#]; + // FUNCTY_DOUBLE-DAG: Decl[LocalVar]/Local: someString[#String#]; + // FUNCTY_DOUBLE-DAG: Decl[LocalVar]/Local: someInt[#Int#]; + // FUNCTY_DOUBLE: End completions +} + + +private extension Sequence { + func SubstitutableBaseTyOfSubscript(by keyPath: KeyPath) -> [Element] { + return sorted { a, b in a[#^GENERICBASE_SUB^#] } + // GENERICBASE_SUB: Begin completions, 1 item + // GENERICBASE_SUB: Pattern/CurrNominal/Flair[ArgLabels]: ['[']{#keyPath: KeyPath#}[']'][#Value#]; + // GENERICBASE_SUB: End completions + } +} + + +func testLValueBaseTyOfSubscript() { + var cache: [String: Codable] = [:] + if let cached = cache[#^LVALUEBASETY^# + + // LVALUEBASETY: Begin completions + // LVALUEBASETY-DAG: Decl[Subscript]/CurrNominal/Flair[ArgLabels]/IsSystem: ['[']{#(position): Dictionary.Index#}[']'][#(key: String, value: Codable)#]; + // LVALUEBASETY-DAG: Decl[Subscript]/CurrNominal/Flair[ArgLabels]/IsSystem: ['[']{#(key): String#}[']'][#@lvalue Codable?#]; + // LVALUEBASETY: End completions +} + +func testSkippedCallArgInInvalidResultBuilderBody() { + protocol MyView { + associatedtype Body + var body: Body { get } + } + struct MyEmptyView: MyView { + var body: Never + } + + @resultBuilder public struct MyViewBuilder { + public static func buildBlock() -> MyEmptyView { fatalError() } + public static func buildBlock(_ content: Content) -> Content where Content : MyView { fatalError() } + } + + struct MyImage : MyView { + var body: Never + public init(systemName: String, otherArg: Int) {} + } + + struct Other { + public init(action: Int, @MyViewBuilder label: () -> Label) {} + public init(_ titleKey: Int, action: Int) {} + } + + func foo() -> Bool { + Other(action: 2) { + MyImage(systemName: "", #^INVALID_RESULTBUILDER_ARG^# + struct Invalid + } - overloadedGeneric(x: 1, #^INVALID_MISSINGCONFORMANCE_CONSTRAINT?check=INVALID_MISSINGCONFORMANCE^#, z: [CondConfType("foo")], y: [CondConfType("bar")]) + // INVALID_RESULTBUILDER_ARG: Begin completions, 1 item + // INVALID_RESULTBUILDER_ARG: Pattern/Local/Flair[ArgLabels]: {#otherArg: Int#}[#Int#]; + // INVALID_RESULTBUILDER_ARG: End completions } diff --git a/test/IDE/complete_enum_unresolved_dot_argument_labels.swift b/test/IDE/complete_enum_unresolved_dot_argument_labels.swift index cad2f79bd602e..c4b049ec32841 100644 --- a/test/IDE/complete_enum_unresolved_dot_argument_labels.swift +++ b/test/IDE/complete_enum_unresolved_dot_argument_labels.swift @@ -1,15 +1,55 @@ -// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=COMPLETE | %FileCheck %s +// RUN: %empty-directory(%t) +// RUN: %target-swift-ide-test -batch-code-completion -source-filename %s -filecheck %raw-FileCheck -completion-output-dir %t enum DragState { case inactive - case dragging(translation: Int) + case dragging(translationX: Int, translationY: Int) + case defaulted(x: Int = 2, y: Int = 3, z: Int, extra: Int) } -func foo() { +func testInit() { var state = DragState.inactive - state = .dragging(#^COMPLETE^# + state = .dragging(#^SIGNATURE^#) + // SIGNATURE: Begin completions, 1 item + // SIGNATURE: Pattern/CurrModule/Flair[ArgLabels]/TypeRelation[Identical]: ['(']{#translationX: Int#}, {#translationY: Int#}[')'][#DragState#]; + // SIGNATURE: End completions + + state = .dragging(translationX: 2, #^ARGUMENT^#) + // ARGUMENT: Begin completions, 1 item + // ARGUMENT: Pattern/Local/Flair[ArgLabels]: {#translationY: Int#}[#Int#]; + // ARGUMENT: End completions + + state = .defaulted(#^DEFAULTED^#) + // DEFAULTED: Begin completions, 1 items + // DEFAULTED: Pattern/CurrModule/Flair[ArgLabels]/TypeRelation[Identical]: ['(']{#x: Int#}, {#y: Int#}, {#z: Int#}, {#extra: Int#}[')'][#DragState#]; + // DEFAULTED: End completions + + state = .defaulted(x: 1, #^DEFAULTEDARG^#) + state = .defaulted(x: "Wrong type", #^ERRORTYPE?check=DEFAULTEDARG^#) + // DEFAULTEDARG: Begin completions, 2 items + // DEFAULTEDARG: Pattern/Local/Flair[ArgLabels]: {#y: Int#}[#Int#]; + // DEFAULTEDARG: Pattern/Local/Flair[ArgLabels]: {#z: Int#}[#Int#]; + // DEFAULTEDARG: End completions + + state = .defaulted(wrongLabel: 2, #^ERRORARG^#) + // ERRORARG: Begin completions, 3 items + // ERRORARG: Pattern/Local/Flair[ArgLabels]: {#x: Int#}[#Int#]; + // ERRORARG: Pattern/Local/Flair[ArgLabels]: {#y: Int#}[#Int#]; + // ERRORARG: Pattern/Local/Flair[ArgLabels]: {#z: Int#}[#Int#]; } -// CHECK: Begin completions, 1 item -// CHECK: Pattern/CurrModule/Flair[ArgLabels]/TypeRelation[Identical]: ['(']{#translation: Int#}[')'][#DragState#]; -// CHECK: End completions +func testMatch() { + var state = DragState.inactive + let localInt = 42 + switch state { + case .dragging(translationX: 2, #^MATCH_ARGY^#): + // MATCH_ARGY: Begin completions + // FIXME: This should have an identical type relation + // MATCH_ARGY: Decl[LocalVar]/Local: localInt[#Int#]; name=localInt + // FIXME: We should offer 'translationY:' (without any flair since it's optional), `let translationY`, and `_` + // MATCH_ARGY: End completions + break + default: + break + } +} From 85fea8933c0a54a49b06421c104e21da240b7d3b Mon Sep 17 00:00:00 2001 From: Nathan Hawes Date: Sat, 21 Aug 2021 11:12:32 +1000 Subject: [PATCH 7/7] fix two more issues found by the stress tester --- lib/Sema/CSSimplify.cpp | 9 +++++++-- lib/Sema/TypeCheckCodeCompletion.cpp | 9 ++++++++- test/IDE/complete_ambiguous.swift | 18 ++++++++++++++++++ test/IDE/complete_subscript.swift | 18 ++++++++++++++++++ 4 files changed, 51 insertions(+), 3 deletions(-) diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index 380c6d2f1e731..ba031a4911c0b 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -5709,8 +5709,13 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind, if (!type1->is() && type2->isExistentialType()) { - // Penalize conversions to Any. - if (kind >= ConstraintKind::Conversion && type2->isAny()) + // Penalize conversions to Any, unless we're solving for code completion. + // Code completion should offer completions both from solutions with + // overloads involving Any and those with more specific types because the + // completion the user then choses can force the Any overload to be + // picked. + if (kind >= ConstraintKind::Conversion && type2->isAny() && + !isForCodeCompletion()) increaseScore(ScoreKind::SK_EmptyExistentialConversion); conversionsOrFixes.push_back(ConversionRestrictionKind::Existential); diff --git a/lib/Sema/TypeCheckCodeCompletion.cpp b/lib/Sema/TypeCheckCodeCompletion.cpp index a003e4f7ce6be..421aca61919f1 100644 --- a/lib/Sema/TypeCheckCodeCompletion.cpp +++ b/lib/Sema/TypeCheckCodeCompletion.cpp @@ -183,13 +183,20 @@ class SanitizeExpr : public ASTWalker { continue; } - // Restore '@autoclosure'd value. if (auto ACE = dyn_cast(expr)) { + // Restore '@autoclosure'd value. // This is only valid if the closure doesn't have parameters. if (ACE->getParameters()->size() == 0) { expr = ACE->getSingleExpressionBody(); continue; } + + // Restore autoclosure'd function reference. + if (auto *unwrapped = ACE->getUnwrappedCurryThunkExpr()) { + expr = unwrapped; + continue; + } + llvm_unreachable("other AutoClosureExpr must be handled specially"); } diff --git a/test/IDE/complete_ambiguous.swift b/test/IDE/complete_ambiguous.swift index 5aabc57aea677..891d1ed6bf0aa 100644 --- a/test/IDE/complete_ambiguous.swift +++ b/test/IDE/complete_ambiguous.swift @@ -383,6 +383,10 @@ _ = testing([Point(4, 89)]) { arg in struct Thing { init(_ block: (Point) -> Void) {} + + enum ThingEnum { case first, second } + func doStuff(_ x: ThingEnum, _ y: Int) -> Thing { return self } + func takesRef(_ ref: () -> ()) -> Thing { return self } } @resultBuilder struct ThingBuilder { @@ -429,6 +433,20 @@ CreateThings { Thing. // ErrorExpr } +struct TestFuncBodyBuilder { + func someFunc() {} + + @ThingBuilder func foo() -> [Thing] { + Thing() + .doStuff(.#^FUNCBUILDER_FUNCBODY^#, 3) + .takesRef(someFunc) + } +} +// FUNCBUILDER_FUNCBODY: Begin completions, 3 items +// FUNCBUILDER_FUNCBODY-DAG: Decl[EnumElement]/CurrNominal/Flair[ExprSpecific]/TypeRelation[Identical]: first[#Thing.ThingEnum#]; +// FUNCBUILDER_FUNCBODY-DAG: Decl[EnumElement]/CurrNominal/Flair[ExprSpecific]/TypeRelation[Identical]: second[#Thing.ThingEnum#]; +// FUNCBUILDER_FUNCBODY-DAG: Decl[InstanceMethod]/CurrNominal/TypeRelation[Invalid]: hash({#(self): Thing.ThingEnum#})[#(into: inout Hasher) -> Void#]; +// FUNCBUILDER_FUNCBODY: End completions func takesClosureOfPoint(_: (Point)->()) {} func overloadedWithDefaulted(_: ()->()) {} diff --git a/test/IDE/complete_subscript.swift b/test/IDE/complete_subscript.swift index 1539a3119ef09..d5ea97656488c 100644 --- a/test/IDE/complete_subscript.swift +++ b/test/IDE/complete_subscript.swift @@ -17,6 +17,7 @@ // RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=LABELED_SUBSCRIPT | %FileCheck %s -check-prefix=LABELED_SUBSCRIPT // RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=TUPLE | %FileCheck %s -check-prefix=TUPLE +// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=SETTABLE_SUBSCRIPT | %FileCheck %s -check-prefix=SETTABLE_SUBSCRIPT struct MyStruct { static subscript(x: Int, static defValue: T) -> MyStruct { @@ -159,3 +160,20 @@ func testSubcscriptTuple(val: (x: Int, String)) { // TUPLE-DAG: Pattern/CurrNominal/Flair[ArgLabels]: ['[']{#keyPath: KeyPath<(x: Int, String), Value>#}[']'][#Value#]; // TUPLE: End completions } + +struct HasSettableSub { + subscript(a: String) -> Any { + get { return 1 } + set { } + } +} + +func testSettableSub(x: inout HasSettableSub) { + let local = "some string" + x[#^SETTABLE_SUBSCRIPT^#] = 32 +} +// SETTABLE_SUBSCRIPT: Begin completions +// SETTABLE_SUBSCRIPT-DAG: Pattern/CurrNominal/Flair[ArgLabels]: ['[']{#keyPath: KeyPath#}[']'][#Value#]; +// SETTABLE_SUBSCRIPT-DAG: Decl[Subscript]/CurrNominal/Flair[ArgLabels]: ['[']{#(a): String#}[']'][#@lvalue Any#]; +// SETTABLE_SUBSCRIPT-DAG: Decl[LocalVar]/Local/TypeRelation[Identical]: local[#String#]; name=local +// SETTABLE_SUBSCRIPT: End completions