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

ahoppen · ahoppen · commit 6d8b1f42c7e3 · 2023-05-05T15:18:57.000-07:00
We previously asserted that for a call the function type had the same number of parameters as the declaration. But that’s not true for parameter packs anymore because the parameter pack will be exploded in the function type to account for passing multiple arguments to the pack.

To fix this, use `ConcreteDeclRef` instead of a `ValueDecl`, which has a substitution map and is able to account for the exploded parameter packs when accessed using `getParameterAt`.

rdar://100066716
diff --git a/include/swift/IDE/ArgumentCompletion.h b/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;
diff --git a/include/swift/IDE/SelectedOverloadInfo.h b/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
diff --git a/lib/IDE/ArgumentCompletion.cpp b/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);
             }
           }
diff --git a/lib/IDE/CursorInfo.cpp b/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:
diff --git a/lib/IDE/SelectedOverloadInfo.cpp b/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);
 
diff --git a/test/IDE/complete_call_arg.swift b/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
+}

Original file line number	Diff line number	Diff line change
`@@ -40,7 +40,9 @@ swift::ide::getSelectedOverloadInfo(const Solution &S,`
`40`	`40`	`Result.BaseTy = nullptr;`
`41`	`41`	`}`
`42`	`42`
`43`		`- Result.Value = SelectedOverload->choice.getDeclOrNull();`
	`43`	`+ if (auto ReferencedDecl = SelectedOverload->choice.getDeclOrNull()) {`
	`44`	`+ Result.ValueRef = S.resolveConcreteDeclRef(ReferencedDecl, CalleeLocator);`
	`45`	`+ }`
`44`	`46`	`Result.ValueTy =`
`45`	`47`	`S.simplifyTypeForCodeCompletion(SelectedOverload->adjustedOpenedType);`
`46`	`48`