[Sema] Add visited contexts to CodeCompleteContext

[hokein]

Summary

This would allow code completion clients to know which context is visited during Sema code completion.

Also some changes:

• add EnteredContext callback in VisibleDeclConsumer.
• add a simple unittest for sema code completion (only for visited contexts at the moment).

[ilya-biryukov]

include/clang/Sema/Lookup.h

791

Maybe rename it to VisitedContext ? Seems more in-line with FoundDecl.
BeginVisitContext also suggest there should be EndVisitContext

[bkramer]

We should have a unit test for this, otherwise it's dead code.

include/clang/Sema/Lookup.h

791

Semicolon is not needed here.

[hokein]

Refine the patch:

• expose visited contexts to client by adding getVisitedContexts in CodeCompleteContext.
• add unittest for sema code completion.

[ilya-biryukov]

LGTM.
Look at the comments for a few NITs.

include/clang/Sema/CodeCompleteConsumer.h

271

Given that lookup does not visit the same DeclContext twice, we're probably fine with vector here, rather than the set.
On the other hand, using set gives a better understanding that the order of items is not specified.
I don't think we should necessarily changes the code here, just something to think about.

290

s/breif/brief

290

I'd mention lookup explicitly in the comment, e.g.

/// \brief A set of decl contexts visited when doing lookup for code completion.

unittests/Sema/CodeCompleteTest.cpp

54

Why onyl namespaces and not all DeclContexts?

71

NIT: local var starts with a lower-case letter.

[hokein]

Address review comments.

[hokein]

include/clang/Sema/CodeCompleteConsumer.h

271

Good point. Yeah, technically, there should be no duplicated DeclContext.

include/clang/Sema/Lookup.h

791

Renamed it to EnteredContext based on offline discussion.

unittests/Sema/CodeCompleteTest.cpp

54

Yeah, this is intended (the method name has indicated it), we only verify the results for namespaces in the test (for simplicity).

include/clang/Sema/CodeCompleteConsumer.h

enum Kind {
     CCC_ObjCInterfaceName,
     /// \brief Code completion where an Objective-C category name is expected.
     CCC_ObjCCategoryName,
     /// \brief An unknown context, in which we are recovering from a parsing
     /// error and don't know which completions we should give.
     CCC_Recovery
   };
 
+  using VisitedContextSet = llvm::SmallPtrSet<DeclContext*, 8>;
+
 private:
   enum Kind Kind;
 
   /// \brief The type that would prefer to see at this point (e.g., the type
   /// of an initializer or function parameter).
   QualType PreferredType;
 
   /// \brief The type of the base object in a member access expression.
   QualType BaseType;
 
   /// \brief The identifiers for Objective-C selector parts.
   ArrayRef<IdentifierInfo *> SelIdents;
 
   /// \brief The scope specifier that comes before the completion token e.g.
   /// "a::b::"
   llvm::Optional<CXXScopeSpec> ScopeSpecifier;
 
+  /// \brief A set of declaration contexts visited by Sema when doing lookup for
+  /// code completion.
+  VisitedContextSet VisitedContexts;
+
 public:
   /// \brief Construct a new code-completion context of the given kind.
   CodeCompletionContext(enum Kind Kind) : Kind(Kind), SelIdents(None) { }
 
   /// \brief Construct a new code-completion context of the given kind.
   CodeCompletionContext(enum Kind Kind, QualType T,
                         ArrayRef<IdentifierInfo *> SelIdents = None)
                         : Kind(Kind),
public:
 
   /// \brief Sets the scope specifier that comes before the completion token.
   /// This is expected to be set in code completions on qualfied specifiers
   /// (e.g. "a::b::").
   void setCXXScopeSpecifier(CXXScopeSpec SS) {
     this->ScopeSpecifier = std::move(SS);
   }
 
+  /// \brief Adds a visited context.
+  void addVisitedContext(DeclContext* Ctx) {
+    VisitedContexts.insert(Ctx);
+  }
+
+  /// \brief Retrieves all visited contexts.
+  const VisitedContextSet &getVisitedContexts() const {
+    return VisitedContexts;
+  }
+
   llvm::Optional<const CXXScopeSpec *> getCXXScopeSpecifier() {
     if (ScopeSpecifier)
       return ScopeSpecifier.getPointer();
     return llvm::None;
   }
 };
 
 /// \brief A "string" used to describe how code completion can

include/clang/Sema/Lookup.h

public:
   /// or NULL if no such declaration exists.
   ///
   /// \param Ctx the original context from which the lookup started.
   ///
   /// \param InBaseClass whether this declaration was found in base
   /// class of the context we searched.
   virtual void FoundDecl(NamedDecl *ND, NamedDecl *Hiding, DeclContext *Ctx,
                          bool InBaseClass) = 0;
+
+  /// \brief Callback to inform the client that Sema entered into a new context
+  /// to find a visible declaration.
+  //
+  /// \param Ctx the context which Sema entered.
+  virtual void EnteredContext(DeclContext *Ctx) {}
 };
 
 /// \brief A class for storing results from argument-dependent lookup.
 class ADLResult {
 private:
   /// A map from canonical decls to the 'most recent' decl.
   llvm::MapVector<NamedDecl*, NamedDecl*> Decls;
 

lib/Sema/SemaCodeComplete.cpp

public:
     void EnterNewScope();
     
     /// \brief Exit from the current scope.
     void ExitScope();
     
     /// \brief Ignore this declaration, if it is seen again.
     void Ignore(const Decl *D) { AllDeclsFound.insert(D->getCanonicalDecl()); }
 
+    /// \brief Add a visited context.
+    void addVisitedContext(DeclContext *Ctx) {
+      CompletionContext.addVisitedContext(Ctx);
+    }
+
     /// \name Name lookup predicates
     ///
     /// These predicates can be passed to the name lookup functions to filter the
     /// results of name lookup. All of the predicates have the same type, so that
     /// 
     //@{
     bool IsOrdinaryName(const NamedDecl *ND) const;
     bool IsOrdinaryNonTypeName(const NamedDecl *ND) const;

 }
 
 namespace {
   /// \brief Visible declaration consumer that adds a code-completion result
   /// for each visible declaration.
   class CodeCompletionDeclConsumer : public VisibleDeclConsumer {
     ResultBuilder &Results;
     DeclContext *CurContext;
-    
   public:
     CodeCompletionDeclConsumer(ResultBuilder &Results, DeclContext *CurContext)
       : Results(Results), CurContext(CurContext) { }
 
     void FoundDecl(NamedDecl *ND, NamedDecl *Hiding, DeclContext *Ctx,
                    bool InBaseClass) override {
       bool Accessible = true;
       if (Ctx)
         Accessible = Results.getSema().IsSimplyAccessible(ND, Ctx);
 
       ResultBuilder::Result Result(ND, Results.getBasePriority(ND), nullptr,
                                    false, Accessible);
       Results.AddResult(Result, CurContext, Hiding, InBaseClass);
     }
+
+    void EnteredContext(DeclContext* Ctx) override {
+      Results.addVisitedContext(Ctx);
+    }
   };
 }
 
 /// \brief Add type specifiers for the current language as keyword results.
 static void AddTypeSpecifierResults(const LangOptions &LangOpts,
                                     ResultBuilder &Results) {
   typedef CodeCompletionResult Result;
   Results.AddResult(Result("short", CCP_Type));

lib/Sema/SemaLookup.cpp

static void LookupVisibleDecls(DeclContext *Ctx, LookupResult &Result,
                                bool LoadExternal) {
   if (!Ctx)
     return;
 
   // Make sure we don't visit the same context twice.
   if (Visited.visitedContext(Ctx->getPrimaryContext()))
     return;
 
+  Consumer.EnteredContext(Ctx);
+
   // Outside C++, lookup results for the TU live on identifiers.
   if (isa<TranslationUnitDecl>(Ctx) &&
       !Result.getSema().getLangOpts().CPlusPlus) {
     auto &S = Result.getSema();
     auto &Idents = S.Context.Idents;
 
     // Ensure all external identifiers are in the identifier table.
     if (LoadExternal)

unittests/Sema/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   Support
   )
 
 add_clang_unittest(SemaTests
   ExternalSemaSourceTest.cpp
+  CodeCompleteTest.cpp
   )
 
 target_link_libraries(SemaTests
   PRIVATE
   clangAST
   clangBasic
   clangFrontend
   clangParse
   clangSema
   clangTooling
   )

unittests/Sema/CodeCompleteTest.cpp

+//=== unittests/Sema/CodeCompleteTest.cpp - Code Complete tests ==============//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/FrontendActions.h"
+#include "clang/Lex/Preprocessor.h"
+#include "clang/Parse/ParseAST.h"
+#include "clang/Sema/Sema.h"
+#include "clang/Sema/SemaDiagnostic.h"
+#include "clang/Tooling/Tooling.h"
+#include "gtest/gtest.h"
+#include "gmock/gmock.h"
+
+namespace {
+
+using namespace clang;
+using namespace clang::tooling;
+using ::testing::UnorderedElementsAre;
+
+const char TestCCName[] = "test.cc";
+using VisitedContextResults = std::vector<std::string>;
+
+class VisitedContextFinder: public CodeCompleteConsumer {
+public:
+  VisitedContextFinder(VisitedContextResults &Results)
+      : CodeCompleteConsumer(/*CodeCompleteOpts=*/{},
+                             /*CodeCompleteConsumer*/ false),
+        VCResults(Results),
+        CCTUInfo(std::make_shared<GlobalCodeCompletionAllocator>()) {}
+
+  void ProcessCodeCompleteResults(Sema &S, CodeCompletionContext Context,
+                                  CodeCompletionResult *Results,
+                                  unsigned NumResults) override {
+    VisitedContexts = Context.getVisitedContexts();
+    VCResults = getVisitedNamespace();
+  }
+
+  CodeCompletionAllocator &getAllocator() override {
+    return CCTUInfo.getAllocator();
+  }
+
+  CodeCompletionTUInfo &getCodeCompletionTUInfo() override { return CCTUInfo; }
+
+  std::vector<std::string> getVisitedNamespace() const {
+    std::vector<std::string> NSNames;
+    for (const auto *Context : VisitedContexts)
+      if (const auto *NS = llvm::dyn_cast<NamespaceDecl>(Context))
+        NSNames.push_back(NS->getQualifiedNameAsString());
+    return NSNames;
+  }
+
+private:
+  VisitedContextResults& VCResults;
+  CodeCompletionTUInfo CCTUInfo;
+  CodeCompletionContext::VisitedContextSet VisitedContexts;
+};
+
+class CodeCompleteAction : public SyntaxOnlyAction {
+public:
+  CodeCompleteAction(ParsedSourceLocation P, VisitedContextResults &Results)
+      : CompletePosition(std::move(P)), VCResults(Results) {}
+
+  bool BeginInvocation(CompilerInstance &CI) override {
+    CI.getFrontendOpts().CodeCompletionAt = CompletePosition;
+    CI.setCodeCompletionConsumer(new VisitedContextFinder(VCResults));
+    return true;
+  }
+
+private:
+  // 1-based code complete position <Line, Col>;
+  ParsedSourceLocation CompletePosition;
+  VisitedContextResults& VCResults;
+};
+
+ParsedSourceLocation offsetToPosition(llvm::StringRef Code, size_t Offset) {
+  Offset = std::min(Code.size(), Offset);
+  StringRef Before = Code.substr(0, Offset);
+  int Lines = Before.count('\n');
+  size_t PrevNL = Before.rfind('\n');
+  size_t StartOfLine = (PrevNL == StringRef::npos) ? 0 : (PrevNL + 1);
+  return {TestCCName, static_cast<unsigned>(Lines + 1),
+          static_cast<unsigned>(Offset - StartOfLine + 1)};
+}
+
+VisitedContextResults runCodeCompleteOnCode(StringRef Code) {
+  VisitedContextResults Results;
+  auto TokenOffset = Code.find('^');
+  assert(TokenOffset != StringRef::npos &&
+         "Completion token ^ wasn't found in Code.");
+  std::string WithoutToken = Code.take_front(TokenOffset);
+  WithoutToken += Code.drop_front(WithoutToken.size() + 1);
+  assert(StringRef(WithoutToken).find('^') == StringRef::npos &&
+         "expected exactly one completion token ^ inside the code");
+
+  auto Action = llvm::make_unique<CodeCompleteAction>(
+      offsetToPosition(WithoutToken, TokenOffset), Results);
+  clang::tooling::runToolOnCodeWithArgs(Action.release(), Code, {"-std=c++11"},
+                                        TestCCName);
+  return Results;
+}
+
+TEST(SemaCodeCompleteTest, VisitedNSForValidQualifiedId) {
+  auto VisitedNS = runCodeCompleteOnCode(R"cpp(
+     namespace ns1 {}
+     namespace ns2 {}
+     namespace ns3 {}
+     namespace ns3 { namespace nns3 {} }
+
+     namespace foo {
+     using namespace ns1;
+     namespace ns4 {} // not visited
+     namespace { using namespace ns2; }
+     inline namespace bar { using namespace ns3::nns3; }
+     } // foo
+     namespace ns { foo::^ }
+  )cpp");
+  EXPECT_THAT(VisitedNS, UnorderedElementsAre("foo", "ns1", "ns2", "ns3::nns3",
+                                              "foo::(anonymous)"));
+}
+
+TEST(SemaCodeCompleteTest, VisitedNSForInvalideQualifiedId) {
+  auto VisitedNS = runCodeCompleteOnCode(R"cpp(
+     namespace ns { foo::^ }
+  )cpp");
+  EXPECT_TRUE(VisitedNS.empty());
+}
+
+} // namespace