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

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

Author: ahoppen

include/swift/IDE/ArgumentCompletion.h

@@ -50,6 +50,13 @@ class ArgumentTypeCheckCompletionCallback : public TypeCheckCompletionCallback {
     /// Whether the surrounding context is async and thus calling async
     /// functions is supported.
     bool IsInAsyncContext;
+    /// A bitfield to mark whether the parameter at a given index is optional.
+    /// Parameters can be optional if they have a default argument or belong to
+    /// a parameter pack.
+    /// Indicies are based on the parameters in \c FuncTy. Note that the number
+    /// of parameters in \c FuncTy and \c FuncD is different when a parameter
+    /// pack has been exploded.
+    std::vector<bool> DeclParamIsOptional;
 
     /// Types of variables that were determined in the solution that produced
     /// this result. This in particular includes parameters of closures that

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,21 @@ bool ArgumentTypeCheckCompletionCallback::addPossibleParams(
       break;
     }
 
-    const AnyFunctionType::Param *P = &ParamsToPass[Idx];
-    bool Required =
-        !(PL && PL->get(Idx)->isDefaultArgument()) && !P->isVariadic();
+    const AnyFunctionType::Param *TypeParam = &ParamsToPass[Idx];
+    bool Required = !Res.DeclParamIsOptional[Idx];
 
-    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 +150,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 +208,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.FuncD == Info.getValue() &&
                nullableTypesEqual(R.FuncTy, Info.ValueTy) &&
                nullableTypesEqual(R.BaseType, Info.BaseTy) &&
                R.ParamIdx == ParamIdx &&
@@ -231,10 +224,34 @@ void ArgumentTypeCheckCompletionCallback::sawSolutionImpl(const Solution &S) {
   if (Info.ValueTy) {
     FuncTy = Info.ValueTy->lookThroughAllOptionalTypes()->getAs<AnyFunctionType>();
   }
+
+  // Determine which parameters is optional. We need to do this in
+  // `sawSolutionImpl` because it accesses the substitution map in
+  // `Info.ValueRef`. This substitution map might contain tyep variables that
+  // are allocated in the constraint system's arena and are freed once we reach
+  // `deliverResults`.
+  std::vector<bool> DeclParamIsOptional;
+  if (FuncTy) {
+    ArrayRef<AnyFunctionType::Param> ParamsToPass = FuncTy->getParams();
+    for (auto Idx : range(0, ParamsToPass.size())) {
+      bool Optional = false;
+      if (Info.ValueRef) {
+        if (const ParamDecl *DeclParam = getParameterAt(Info.ValueRef, Idx)) {
+          Optional |= DeclParam->isDefaultArgument();
+          Optional |= DeclParam->getType()->is<PackExpansionType>();
+        }
+      }
+      const AnyFunctionType::Param *TypeParam = &ParamsToPass[Idx];
+      Optional |= TypeParam->isVariadic();
+      DeclParamIsOptional.push_back(Optional);
+    }
+  }
+
   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.getValue(), FuncTy,
+                     ArgIdx, ParamIdx, std::move(ClaimedParams),
+                     IsNoninitialVariadic, Info.BaseTy, HasLabel, IsAsync,
+                     DeclParamIsOptional, SolutionSpecificVarTypes});
 }
 
 void ArgumentTypeCheckCompletionCallback::computeShadowedDecls(
@@ -243,7 +260,8 @@ 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.FuncD || !ResultB.FuncD || !ResultA.FuncTy ||
+          !ResultB.FuncTy) {
         continue;
       }
       if (ResultA.FuncD->getName() != ResultB.FuncD->getName()) {

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
+}