Merge pull request #65720 from ahoppen/ahoppen/complete-parameter-pack-arg

[CodeCompletion] Fix a crash when completing an argument to a function taking a parameter pack

Author: ahoppen

include/swift/IDE/ArgumentCompletion.h

@@ -29,8 +29,8 @@ class ArgumentTypeCheckCompletionCallback : public TypeCheckCompletionCallback {
     Type ExpectedCallType;
     /// True if this is a subscript rather than a function call.
     bool IsSubscript;
-    /// The FuncDecl or SubscriptDecl associated with the call.
-    ValueDecl *FuncD;
+    /// The reference to the FuncDecl or SubscriptDecl associated with the call.
+    ConcreteDeclRef FuncDeclRef;
     /// The type of the function being called.
     AnyFunctionType *FuncTy;
     /// The index of the argument containing the completion location
@@ -55,6 +55,9 @@ class ArgumentTypeCheckCompletionCallback : public TypeCheckCompletionCallback {
     /// this result. This in particular includes parameters of closures that
     /// were type-checked with the code completion expression.
     llvm::SmallDenseMap<const VarDecl *, Type> SolutionSpecificVarTypes;
+
+    /// Return the \c FuncDecl or \c SubscriptDecl associated with this call.
+    ValueDecl *getFuncD() const { return FuncDeclRef.getDecl(); }
   };
 
   CodeCompletionExpr *CompletionExpr;

include/swift/IDE/SelectedOverloadInfo.h

@@ -25,13 +25,15 @@ using namespace swift::constraints;
 /// \c SelectedOverload.
 struct SelectedOverloadInfo {
   /// The function that is being called or the value that is being accessed.
-  ValueDecl *Value = nullptr;
+  ConcreteDeclRef ValueRef;
   /// For a function, type of the called function itself (not its result type),
   /// for an arbitrary value the type of that value.
   Type ValueTy;
   /// The type on which the overload is being accessed. \c null if it does not
   /// have a base type, e.g. for a free function.
   Type BaseTy;
+
+  ValueDecl *getValue() const { return ValueRef.getDecl(); }
 };
 
 /// Extract additional information about the overload that is being called by

lib/IDE/ArgumentCompletion.cpp

@@ -38,12 +38,6 @@ bool ArgumentTypeCheckCompletionCallback::addPossibleParams(
 
   ArrayRef<AnyFunctionType::Param> ParamsToPass = Res.FuncTy->getParams();
 
-  ParameterList *PL = nullptr;
-  if (Res.FuncD) {
-    PL = swift::getParameterList(Res.FuncD);
-  }
-  assert(!PL || PL->size() == ParamsToPass.size());
-
   bool ShowGlobalCompletions = false;
   for (auto Idx : range(*Res.ParamIdx, ParamsToPass.size())) {
     bool IsCompletion = (Idx == Res.ParamIdx);
@@ -53,22 +47,35 @@ bool ArgumentTypeCheckCompletionCallback::addPossibleParams(
       break;
     }
 
-    const AnyFunctionType::Param *P = &ParamsToPass[Idx];
-    bool Required =
-        !(PL && PL->get(Idx)->isDefaultArgument()) && !P->isVariadic();
+    // We work with the parameter from the function type and the declaration
+    // because they contain different information that we need.
+    //
+    // Since not all function types are backed by declarations (e.g. closure
+    // paramters), `DeclParam` might be `nullptr`.
+    const AnyFunctionType::Param *TypeParam = &ParamsToPass[Idx];
+    const ParamDecl *DeclParam = getParameterAt(Res.FuncDeclRef, Idx);
+
+    bool Required = true;
+    if (DeclParam && DeclParam->isDefaultArgument()) {
+      Required = false;
+    } else if (DeclParam && DeclParam->getType()->is<PackExpansionType>()) {
+      Required = false;
+    } else if (TypeParam->isVariadic()) {
+      Required = false;
+    }
 
-    if (P->hasLabel() && !(IsCompletion && Res.IsNoninitialVariadic)) {
+    if (TypeParam->hasLabel() && !(IsCompletion && Res.IsNoninitialVariadic)) {
       // Suggest parameter label if parameter has label, we are completing in it
       // and it is not a variadic parameter that already has arguments
-      PossibleParamInfo PP(P, Required);
+      PossibleParamInfo PP(TypeParam, Required);
       if (!llvm::is_contained(Params, PP)) {
         Params.push_back(std::move(PP));
       }
     } else {
       // We have a parameter that doesn't require a label. Suggest global
       // results for that type.
       ShowGlobalCompletions = true;
-      Types.push_back(P->getPlainType());
+      Types.push_back(TypeParam->getPlainType());
     }
     if (Required) {
       // The user should only be suggested the first required param. Stop.
@@ -157,7 +164,7 @@ void ArgumentTypeCheckCompletionCallback::sawSolutionImpl(const Solution &S) {
   auto *CalleeLocator = S.getCalleeLocator(CallLocator);
 
   auto Info = getSelectedOverloadInfo(S, CalleeLocator);
-  if (Info.Value && Info.Value->shouldHideFromEditor()) {
+  if (Info.getValue() && Info.getValue()->shouldHideFromEditor()) {
     return;
   }
   // Disallow invalid initializer references
@@ -215,7 +222,7 @@ void ArgumentTypeCheckCompletionCallback::sawSolutionImpl(const Solution &S) {
 
   // If this is a duplicate of any other result, ignore this solution.
   if (llvm::any_of(Results, [&](const Result &R) {
-        return R.FuncD == Info.Value &&
+        return R.FuncDeclRef == Info.ValueRef &&
                nullableTypesEqual(R.FuncTy, Info.ValueTy) &&
                nullableTypesEqual(R.BaseType, Info.BaseTy) &&
                R.ParamIdx == ParamIdx &&
@@ -232,9 +239,10 @@ void ArgumentTypeCheckCompletionCallback::sawSolutionImpl(const Solution &S) {
     FuncTy = Info.ValueTy->lookThroughAllOptionalTypes()->getAs<AnyFunctionType>();
   }
   Results.push_back({ExpectedTy, ExpectedCallType,
-                     isa<SubscriptExpr>(ParentCall), Info.Value, FuncTy, ArgIdx,
-                     ParamIdx, std::move(ClaimedParams), IsNoninitialVariadic,
-                     Info.BaseTy, HasLabel, IsAsync, SolutionSpecificVarTypes});
+                     isa<SubscriptExpr>(ParentCall), Info.ValueRef, FuncTy,
+                     ArgIdx, ParamIdx, std::move(ClaimedParams),
+                     IsNoninitialVariadic, Info.BaseTy, HasLabel, IsAsync,
+                     SolutionSpecificVarTypes});
 }
 
 void ArgumentTypeCheckCompletionCallback::computeShadowedDecls(
@@ -243,24 +251,25 @@ void ArgumentTypeCheckCompletionCallback::computeShadowedDecls(
     auto &ResultA = Results[i];
     for (size_t j = i + 1; j < Results.size(); ++j) {
       auto &ResultB = Results[j];
-      if (!ResultA.FuncD || !ResultB.FuncD || !ResultA.FuncTy || !ResultB.FuncTy) {
+      if (!ResultA.getFuncD() || !ResultB.getFuncD() || !ResultA.FuncTy ||
+          !ResultB.FuncTy) {
         continue;
       }
-      if (ResultA.FuncD->getName() != ResultB.FuncD->getName()) {
+      if (ResultA.getFuncD()->getName() != ResultB.getFuncD()->getName()) {
         continue;
       }
       if (!ResultA.FuncTy->isEqual(ResultB.FuncTy)) {
         continue;
       }
       ProtocolDecl *inProtocolExtensionA =
-          ResultA.FuncD->getDeclContext()->getExtendedProtocolDecl();
+          ResultA.getFuncD()->getDeclContext()->getExtendedProtocolDecl();
       ProtocolDecl *inProtocolExtensionB =
-          ResultB.FuncD->getDeclContext()->getExtendedProtocolDecl();
+          ResultB.getFuncD()->getDeclContext()->getExtendedProtocolDecl();
 
       if (inProtocolExtensionA && !inProtocolExtensionB) {
-        ShadowedDecls.insert(ResultA.FuncD);
+        ShadowedDecls.insert(ResultA.getFuncD());
       } else if (!inProtocolExtensionA && inProtocolExtensionB) {
-        ShadowedDecls.insert(ResultB.FuncD);
+        ShadowedDecls.insert(ResultB.getFuncD());
       }
     }
   }
@@ -301,35 +310,37 @@ void ArgumentTypeCheckCompletionCallback::deliverResults(
         }
         if ((BaseNominal = BaseTy->getAnyNominal())) {
           SemanticContext = SemanticContextKind::CurrentNominal;
-          if (Result.FuncD &&
-              Result.FuncD->getDeclContext()->getSelfNominalTypeDecl() !=
+          if (Result.getFuncD() &&
+              Result.getFuncD()->getDeclContext()->getSelfNominalTypeDecl() !=
                   BaseNominal) {
             SemanticContext = SemanticContextKind::Super;
           }
         } else if (BaseTy->is<TupleType>() || BaseTy->is<SubstitutableType>()) {
           SemanticContext = SemanticContextKind::CurrentNominal;
         }
       }
-      if (SemanticContext == SemanticContextKind::None && Result.FuncD) {
-        if (Result.FuncD->getDeclContext()->isTypeContext()) {
+      if (SemanticContext == SemanticContextKind::None && Result.getFuncD()) {
+        if (Result.getFuncD()->getDeclContext()->isTypeContext()) {
           SemanticContext = SemanticContextKind::CurrentNominal;
-        } else if (Result.FuncD->getDeclContext()->isLocalContext()) {
+        } else if (Result.getFuncD()->getDeclContext()->isLocalContext()) {
           SemanticContext = SemanticContextKind::Local;
-        } else if (Result.FuncD->getModuleContext() == DC->getParentModule()) {
+        } else if (Result.getFuncD()->getModuleContext() ==
+                   DC->getParentModule()) {
           SemanticContext = SemanticContextKind::CurrentModule;
         }
       }
       if (Result.FuncTy) {
         if (auto FuncTy = Result.FuncTy) {
-          if (ShadowedDecls.count(Result.FuncD) == 0) {
+          if (ShadowedDecls.count(Result.getFuncD()) == 0) {
             // Don't show call pattern completions if the function is
             // overridden.
             if (Result.IsSubscript) {
               assert(SemanticContext != SemanticContextKind::None);
-              auto *SD = dyn_cast_or_null<SubscriptDecl>(Result.FuncD);
+              auto *SD = dyn_cast_or_null<SubscriptDecl>(Result.getFuncD());
               Lookup.addSubscriptCallPattern(FuncTy, SD, SemanticContext);
             } else {
-              auto *FD = dyn_cast_or_null<AbstractFunctionDecl>(Result.FuncD);
+              auto *FD =
+                  dyn_cast_or_null<AbstractFunctionDecl>(Result.getFuncD());
               Lookup.addFunctionCallPattern(FuncTy, FD, SemanticContext);
             }
           }

lib/IDE/CursorInfo.cpp

@@ -311,7 +311,7 @@ class CursorInfoTypeCheckSolutionCallback : public TypeCheckCompletionCallback {
     auto Locator = CS.getConstraintLocator(ResolveExpr);
     auto CalleeLocator = S.getCalleeLocator(Locator);
     auto OverloadInfo = getSelectedOverloadInfo(S, CalleeLocator);
-    if (!OverloadInfo.Value) {
+    if (!OverloadInfo.ValueRef) {
       // We could not resolve the referenced declaration. Skip the solution.
       return;
     }
@@ -322,11 +322,12 @@ class CursorInfoTypeCheckSolutionCallback : public TypeCheckCompletionCallback {
     if (auto BaseExpr =
             simplifyLocatorToAnchor(BaseLocator).dyn_cast<Expr *>()) {
       IsDynamicRef =
-          ide::isDynamicRef(BaseExpr, OverloadInfo.Value,
+          ide::isDynamicRef(BaseExpr, OverloadInfo.getValue(),
                             [&S](Expr *E) { return S.getResolvedType(E); });
     }
 
-    Results.push_back({OverloadInfo.BaseTy, IsDynamicRef, OverloadInfo.Value});
+    Results.push_back(
+        {OverloadInfo.BaseTy, IsDynamicRef, OverloadInfo.getValue()});
   }
 
 public:

lib/IDE/SelectedOverloadInfo.cpp

@@ -40,7 +40,9 @@ swift::ide::getSelectedOverloadInfo(const Solution &S,
       Result.BaseTy = nullptr;
     }
 
-    Result.Value = SelectedOverload->choice.getDeclOrNull();
+    if (auto ReferencedDecl = SelectedOverload->choice.getDeclOrNull()) {
+      Result.ValueRef = S.resolveConcreteDeclRef(ReferencedDecl, CalleeLocator);
+    }
     Result.ValueTy =
         S.simplifyTypeForCodeCompletion(SelectedOverload->adjustedOpenedType);
 

test/IDE/complete_call_arg.swift

@@ -1303,3 +1303,10 @@ func testMacroArg() {
 // MACRO_CALL_ARG-DAG: Literal[Boolean]/None:              true[#Bool#]; name=true
 // MACRO_CALL_ARG: End completions
 }
+
+func testParameterPack(intArray: [Int]) {
+  func myZip<each S>(_ sequence: repeat each S, otherParam: Int) where repeat each S: Sequence {}
+  myZip([1], #^PARAMETER_PACK_ARG^#)
+  // PARAMETER_PACK_ARG: Pattern/Local/Flair[ArgLabels]:     {#otherParam: Int#}[#Int#]; name=otherParam:
+  // PARAMETER_PACK_ARG: Decl[LocalVar]/Local/TypeRelation[Convertible]: intArray[#[Int]#]; name=intArray
+}