[CodeCompletion][NFC] Add doc comments and rename symbols for clarity in the new member completion implementation.

Author: Nathan Hawes

include/swift/Sema/IDETypeChecking.h

@@ -64,34 +64,42 @@ namespace swift {
 
   class DotExprLookup: public CompletionCollector {
     struct SolutionInfo {
-      Type Ty;
-      ValueDecl* ReferencedDecl;
+      Type BaseTy;
+      ValueDecl* BaseDecl;
       SmallVector<Type, 4> ExpectedTypes;
       bool ExpectsNonVoid;
       bool BaseIsStaticMetaType;
-      bool IsSingleExpressionClosure;
+      bool IsSingleExpressionBody;
     };
 
     SourceLoc DotLoc;
     DeclContext *DC;
     CodeCompletionExpr *CompletionExpr;
-
-    llvm::DenseMap<std::pair<Type, Decl*>, size_t> ResultToIndex;
     SmallVector<SolutionInfo, 4> Solutions;
+    llvm::DenseMap<std::pair<Type, Decl*>, size_t> BaseToSolutionIdx;
     bool GotCallback = false;
 
   public:
     DotExprLookup(SourceLoc DotLoc, DeclContext *DC,
                   CodeCompletionExpr *CompletionExpr)
       : DotLoc(DotLoc), DC(DC), CompletionExpr(CompletionExpr) {}
 
+    /// Lookup the completion members on the base expressions extracted from the
+    /// solutions seen so far and pass them to the given \c Consumer.
     void performLookup(ide::CodeCompletionContext &CompletionCtx,
                        ide::CodeCompletionConsumer &Consumer,
                        bool isInSelector) const;
+
+    /// True if a 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();
 
   private:
+    /// Called for each solution produced while  type-checking an expression containing a code
+    /// completion expression.
     void sawSolution(const constraints::Solution &solution) override;
   };
 

lib/IDE/CodeCompletion.cpp

@@ -1716,7 +1716,6 @@ class CodeCompletionCallbacksImpl : public CodeCompletionCallbacks {
 private:
   void addKeywords(CodeCompletionResultSink &Sink, bool MaybeFuncBody);
   bool trySolverCompletion();
-  void deliverCompletionResults();
 };
 } // end anonymous namespace
 
@@ -5812,10 +5811,10 @@ static void addConditionalCompilationFlags(ASTContext &Ctx,
   }
 }
 
-static void processModuleRequests(CodeCompletionContext &CompletionContext,
-                                  CompletionLookup &Lookup,
-                                  SourceFile &SF,
-                                  CodeCompletionConsumer &Consumer) {
+static void deliverCompletionResults(CodeCompletionContext &CompletionContext,
+                                     CompletionLookup &Lookup,
+                                     SourceFile &SF,
+                                     CodeCompletionConsumer &Consumer) {
   llvm::SmallPtrSet<Identifier, 8> seenModuleNames;
   std::vector<RequestedCachedModule> RequestedModules;
 
@@ -5890,7 +5889,6 @@ static void processModuleRequests(CodeCompletionContext &CompletionContext,
 
       // Add results for all imported modules.
       SmallVector<ModuleDecl::ImportedModule, 4> Imports;
-      //auto *SF = CurDeclContext->getParentSourceFile();
       SF.getImportedModules(
           Imports, {ModuleDecl::ImportFilterKind::Public,
                     ModuleDecl::ImportFilterKind::Private,
@@ -5903,7 +5901,11 @@ static void processModuleRequests(CodeCompletionContext &CompletionContext,
     }
   }
   Lookup.RequestedCachedResults.clear();
+  CompletionContext.typeContextKind = Lookup.typeContextKind();
 
+  // Use the current SourceFile as the DeclContext so that we can use it to
+  // perform qualified lookup, and to get the correct visibility for
+  // @testable imports.
   DeclContext *DCForModules = &SF;
   Consumer.handleResultsAndModules(CompletionContext, RequestedModules,
                                    DCForModules);
@@ -5935,19 +5937,17 @@ void DotExprLookup::performLookup(ide::CodeCompletionContext &CompletionCtx,
 
   for (auto &Solution: Solutions) {
     Lookup.setIsStaticMetatype(Solution.BaseIsStaticMetaType);
-    Lookup.getPostfixKeywordCompletions(Solution.Ty, BaseExpr);
+    Lookup.getPostfixKeywordCompletions(Solution.BaseTy, BaseExpr);
     Lookup.setExpectedTypes(Solution.ExpectedTypes,
-                            Solution.IsSingleExpressionClosure,
+                            Solution.IsSingleExpressionBody,
                             Solution.ExpectsNonVoid);
-    if (isDynamicLookup(Solution.Ty))
+    if (isDynamicLookup(Solution.BaseTy))
       Lookup.setIsDynamicLookup();
-    Lookup.getValueExprCompletions(Solution.Ty, Solution.ReferencedDecl);
+    Lookup.getValueExprCompletions(Solution.BaseTy, Solution.BaseDecl);
   }
 
   SourceFile *SF = DC->getParentSourceFile();
-  // FIXME: There may be multiple of these now.
-  CompletionCtx.typeContextKind = Lookup.typeContextKind();
-  processModuleRequests(CompletionCtx, Lookup, *SF, Consumer);
+  deliverCompletionResults(CompletionCtx, Lookup, *SF, Consumer);
 }
 
 bool CodeCompletionCallbacksImpl::trySolverCompletion() {
@@ -5985,7 +5985,7 @@ bool CodeCompletionCallbacksImpl::trySolverCompletion() {
       CompletionCollector(Context.CompletionCallback, &Lookup);
     typeCheckContextAt(CurDeclContext, CompletionLoc);
 
-    // This (should) only happens in cases where the expression isn't
+    // This (hopefully) only happens in cases where the expression isn't
     // typechecked during normal compilation either (e.g. member completion in a
     // switch case where there switched value is invalid). Having normal
     // typechecking still resolve even these cases would be beneficial for
@@ -6529,112 +6529,7 @@ void CodeCompletionCallbacksImpl::doneParsing() {
     break;
   }
 
-  llvm::SmallPtrSet<Identifier, 8> seenModuleNames;
-
-  for (auto &Request: Lookup.RequestedCachedResults) {
-    // Use the current SourceFile as the DeclContext so that we can use it to
-    // perform qualified lookup, and to get the correct visibility for
-    // @testable imports.
-    const SourceFile &SF = P.SF;
-
-    llvm::DenseSet<CodeCompletionCache::Key> ImportsSeen;
-    auto handleImport = [&](ModuleDecl::ImportedModule Import) {
-      ModuleDecl *TheModule = Import.importedModule;
-      ModuleDecl::AccessPathTy Path = Import.accessPath;
-      if (TheModule->getFiles().empty())
-        return;
-
-      // Clang submodules are ignored and there's no lookup cost involved,
-      // so just ignore them and don't put the empty results in the cache
-      // because putting a lot of objects in the cache will push out
-      // other lookups.
-      if (isClangSubModule(TheModule))
-        return;
-
-      std::vector<std::string> AccessPath;
-      for (auto Piece : Path) {
-        AccessPath.push_back(std::string(Piece.Item));
-      }
-
-      StringRef ModuleFilename = TheModule->getModuleFilename();
-      // ModuleFilename can be empty if something strange happened during
-      // module loading, for example, the module file is corrupted.
-      if (!ModuleFilename.empty()) {
-        auto &Ctx = TheModule->getASTContext();
-        CodeCompletionCache::Key K{
-            ModuleFilename.str(),
-            std::string(TheModule->getName()),
-            AccessPath,
-            Request.NeedLeadingDot,
-            SF.hasTestableOrPrivateImport(
-                AccessLevel::Internal, TheModule,
-                SourceFile::ImportQueryKind::TestableOnly),
-            SF.hasTestableOrPrivateImport(
-                AccessLevel::Internal, TheModule,
-                SourceFile::ImportQueryKind::PrivateOnly),
-            Ctx.LangOpts.CodeCompleteInitsInPostfixExpr,
-            CompletionContext.getAnnotateResult(),
-        };
-
-        using PairType = llvm::DenseSet<swift::ide::CodeCompletionCache::Key,
-            llvm::DenseMapInfo<CodeCompletionCache::Key>>::iterator;
-        std::pair<PairType, bool> Result = ImportsSeen.insert(K);
-        if (!Result.second)
-          return; // already handled.
-        RequestedModules.push_back({std::move(K), TheModule,
-          Request.OnlyTypes, Request.OnlyPrecedenceGroups});
-
-        if (Request.IncludeModuleQualifier &&
-            seenModuleNames.insert(TheModule->getName()).second)
-          Lookup.addModuleName(TheModule);
-      }
-    };
-
-    if (Request.TheModule) {
-      // FIXME: actually check imports.
-      for (auto Import : namelookup::getAllImports(Request.TheModule)) {
-        handleImport(Import);
-      }
-    } else {
-      // Add results from current module.
-      Lookup.getToplevelCompletions(Request.OnlyTypes);
-
-      // Add the qualifying module name
-      auto curModule = CurDeclContext->getParentModule();
-      if (Request.IncludeModuleQualifier &&
-          seenModuleNames.insert(curModule->getName()).second)
-        Lookup.addModuleName(curModule);
-
-      // Add results for all imported modules.
-      SmallVector<ModuleDecl::ImportedModule, 4> Imports;
-      auto *SF = CurDeclContext->getParentSourceFile();
-      SF->getImportedModules(
-          Imports, {ModuleDecl::ImportFilterKind::Public,
-                    ModuleDecl::ImportFilterKind::Private,
-                    ModuleDecl::ImportFilterKind::ImplementationOnly});
-
-      for (auto Imported : Imports) {
-        for (auto Import : namelookup::getAllImports(Imported.importedModule))
-          handleImport(Import);
-      }
-    }
-  }
-  Lookup.RequestedCachedResults.clear();
-
-  CompletionContext.typeContextKind = Lookup.typeContextKind();
-
-  deliverCompletionResults();
-}
-
-void CodeCompletionCallbacksImpl::deliverCompletionResults() {
-  // Use the current SourceFile as the DeclContext so that we can use it to
-  // perform qualified lookup, and to get the correct visibility for
-  // @testable imports.
-  DeclContext *DCForModules = &P.SF;
-
-  Consumer.handleResultsAndModules(CompletionContext, RequestedModules,
-                                   DCForModules);
-  RequestedModules.clear();
+  deliverCompletionResults(CompletionContext, Lookup, P.SF, Consumer);
 }
 
 void PrintingCodeCompletionConsumer::handleResults(

lib/IDE/ExprContextAnalysis.cpp

@@ -38,7 +38,6 @@
 using namespace swift;
 using namespace ide;
 
-
 //===----------------------------------------------------------------------===//
 // typeCheckContextAt(DeclContext, SourceLoc)
 //===----------------------------------------------------------------------===//

lib/Parse/ParseExpr.cpp

@@ -2879,7 +2879,6 @@ ParserResult<Expr> Parser::parseExprClosure() {
   // may be incomplete and the type mismatch in return statement will just
   // confuse the type checker.
   bool hasSingleExpressionBody = false;
-  // FIXME: make this change separately
   if (!missingRBrace && bodyElements.size() == 1 &&
       (!Status.hasCodeCompletion() || isMemberCompletion(bodyElements[0]))) {
     // If the closure's only body element is a single return statement,

lib/Sema/TypeCheckCodeCompletion.cpp

@@ -1007,31 +1007,31 @@ void DotExprLookup::sawSolution(const constraints::Solution &S) {
     ReferencedDecl = SelectedOverload->choice.getDeclOrNull();
 
   auto Key = std::make_pair(BaseTy, ReferencedDecl);
-  auto Ret = ResultToIndex.insert({Key, Solutions.size()});
+  auto Ret = BaseToSolutionIdx.insert({Key, Solutions.size()});
   if (!Ret.second && ExpectedTy) {
     Solutions[Ret.first->getSecond()].ExpectedTypes.push_back(ExpectedTy);
   } else {
     bool ISDMT = S.isStaticallyDerivedMetatype(ParsedExpr);
-    bool ISEC = false;
+    bool SingleExprBody = false;
     bool DisallowVoid = ExpectedTy
                             ? !ExpectedTy->isVoid()
                             : !ParentExpr && CS.getContextualTypePurpose(
                                                  CompletionExpr) != CTP_Unused;
 
     if (!ParentExpr) {
       if (CS.getContextualTypePurpose(CompletionExpr) == CTP_ReturnSingleExpr)
-        ISEC = true;
+        SingleExprBody = true;
     } else if (auto *ParentCE = dyn_cast<ClosureExpr>(ParentExpr)) {
       if (ParentCE->hasSingleExpressionBody() &&
           ParentCE->getSingleExpressionBody() == CompletionExpr) {
         ASTNode Last = ParentCE->getBody()->getLastElement();
         if (!Last.isStmt(StmtKind::Return) || Last.isImplicit())
-          ISEC = true;
+          SingleExprBody = true;
       }
     }
 
     Solutions.push_back(
-        {BaseTy, ReferencedDecl, {}, DisallowVoid, ISDMT, ISEC});
+        {BaseTy, ReferencedDecl, {}, DisallowVoid, ISDMT, SingleExprBody});
     if (ExpectedTy)
       Solutions.back().ExpectedTypes.push_back(ExpectedTy);
   }