[clangd] Have visibleNamespaces() and getEligiblePoints() take a LangOptions rather than a FormatStyle

HighCommander4 · HighCommander4 · commit 445195ba6cee · 2020-03-10T16:45:41.000-04:00
Summary: These functions only use the FormatStyle to obtain a LangOptions via format::getFormattingLangOpts(), and some callers can more easily obtain a LangOptions more directly. Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, usaxena95, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D75716
diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp
@@ -1373,8 +1373,8 @@ class CodeCompleteFlow {
     //  - accessible scopes are determined heuristically.
     //  - all-scopes query if no qualifier was typed (and it's allowed).
     SpecifiedScope Scopes;
-    Scopes.AccessibleScopes =
-        visibleNamespaces(Content.take_front(Offset), Style);
+    Scopes.AccessibleScopes = visibleNamespaces(
+        Content.take_front(Offset), format::getFormattingLangOpts(Style));
     for (std::string &S : Scopes.AccessibleScopes)
       if (!S.empty())
         S.append("::"); // visibleNamespaces doesn't include trailing ::.
diff --git a/clang-tools-extra/clangd/SourceCode.cpp b/clang-tools-extra/clangd/SourceCode.cpp
@@ -654,8 +654,7 @@ struct NamespaceEvent {
   Position Pos;
 };
 // Scans C++ source code for constructs that change the visible namespaces.
-void parseNamespaceEvents(llvm::StringRef Code,
-                          const format::FormatStyle &Style,
+void parseNamespaceEvents(llvm::StringRef Code, const LangOptions &LangOpts,
                           llvm::function_ref<void(NamespaceEvent)> Callback) {
 
   // Stack of enclosing namespaces, e.g. {"clang", "clangd"}
@@ -674,114 +673,113 @@ void parseNamespaceEvents(llvm::StringRef Code,
   std::string NSName;
 
   NamespaceEvent Event;
-  lex(Code, format::getFormattingLangOpts(Style),
-      [&](const syntax::Token &Tok, const SourceManager &SM) {
-        Event.Pos = sourceLocToPosition(SM, Tok.location());
-        switch (Tok.kind()) {
-        case tok::kw_using:
-          State = State == Default ? Using : Default;
-          break;
-        case tok::kw_namespace:
-          switch (State) {
-          case Using:
-            State = UsingNamespace;
-            break;
-          case Default:
-            State = Namespace;
-            break;
-          default:
-            State = Default;
-            break;
-          }
-          break;
-        case tok::identifier:
-          switch (State) {
-          case UsingNamespace:
-            NSName.clear();
-            LLVM_FALLTHROUGH;
-          case UsingNamespaceName:
-            NSName.append(Tok.text(SM).str());
-            State = UsingNamespaceName;
-            break;
-          case Namespace:
-            NSName.clear();
-            LLVM_FALLTHROUGH;
-          case NamespaceName:
-            NSName.append(Tok.text(SM).str());
-            State = NamespaceName;
-            break;
-          case Using:
-          case Default:
-            State = Default;
-            break;
-          }
-          break;
-        case tok::coloncolon:
-          // This can come at the beginning or in the middle of a namespace
-          // name.
-          switch (State) {
-          case UsingNamespace:
-            NSName.clear();
-            LLVM_FALLTHROUGH;
-          case UsingNamespaceName:
-            NSName.append("::");
-            State = UsingNamespaceName;
-            break;
-          case NamespaceName:
-            NSName.append("::");
-            State = NamespaceName;
-            break;
-          case Namespace: // Not legal here.
-          case Using:
-          case Default:
-            State = Default;
-            break;
-          }
-          break;
-        case tok::l_brace:
-          // Record which { started a namespace, so we know when } ends one.
-          if (State == NamespaceName) {
-            // Parsed: namespace <name> {
-            BraceStack.push_back(true);
-            Enclosing.push_back(NSName);
-            Event.Trigger = NamespaceEvent::BeginNamespace;
-            Event.Payload = llvm::join(Enclosing, "::");
-            Callback(Event);
-          } else {
-            // This case includes anonymous namespaces (State = Namespace).
-            // For our purposes, they're not namespaces and we ignore them.
-            BraceStack.push_back(false);
-          }
-          State = Default;
-          break;
-        case tok::r_brace:
-          // If braces are unmatched, we're going to be confused, but don't
-          // crash.
-          if (!BraceStack.empty()) {
-            if (BraceStack.back()) {
-              // Parsed: } // namespace
-              Enclosing.pop_back();
-              Event.Trigger = NamespaceEvent::EndNamespace;
-              Event.Payload = llvm::join(Enclosing, "::");
-              Callback(Event);
-            }
-            BraceStack.pop_back();
-          }
-          break;
-        case tok::semi:
-          if (State == UsingNamespaceName) {
-            // Parsed: using namespace <name> ;
-            Event.Trigger = NamespaceEvent::UsingDirective;
-            Event.Payload = std::move(NSName);
-            Callback(Event);
-          }
-          State = Default;
-          break;
-        default:
-          State = Default;
-          break;
+  lex(Code, LangOpts, [&](const syntax::Token &Tok, const SourceManager &SM) {
+    Event.Pos = sourceLocToPosition(SM, Tok.location());
+    switch (Tok.kind()) {
+    case tok::kw_using:
+      State = State == Default ? Using : Default;
+      break;
+    case tok::kw_namespace:
+      switch (State) {
+      case Using:
+        State = UsingNamespace;
+        break;
+      case Default:
+        State = Namespace;
+        break;
+      default:
+        State = Default;
+        break;
+      }
+      break;
+    case tok::identifier:
+      switch (State) {
+      case UsingNamespace:
+        NSName.clear();
+        LLVM_FALLTHROUGH;
+      case UsingNamespaceName:
+        NSName.append(Tok.text(SM).str());
+        State = UsingNamespaceName;
+        break;
+      case Namespace:
+        NSName.clear();
+        LLVM_FALLTHROUGH;
+      case NamespaceName:
+        NSName.append(Tok.text(SM).str());
+        State = NamespaceName;
+        break;
+      case Using:
+      case Default:
+        State = Default;
+        break;
+      }
+      break;
+    case tok::coloncolon:
+      // This can come at the beginning or in the middle of a namespace
+      // name.
+      switch (State) {
+      case UsingNamespace:
+        NSName.clear();
+        LLVM_FALLTHROUGH;
+      case UsingNamespaceName:
+        NSName.append("::");
+        State = UsingNamespaceName;
+        break;
+      case NamespaceName:
+        NSName.append("::");
+        State = NamespaceName;
+        break;
+      case Namespace: // Not legal here.
+      case Using:
+      case Default:
+        State = Default;
+        break;
+      }
+      break;
+    case tok::l_brace:
+      // Record which { started a namespace, so we know when } ends one.
+      if (State == NamespaceName) {
+        // Parsed: namespace <name> {
+        BraceStack.push_back(true);
+        Enclosing.push_back(NSName);
+        Event.Trigger = NamespaceEvent::BeginNamespace;
+        Event.Payload = llvm::join(Enclosing, "::");
+        Callback(Event);
+      } else {
+        // This case includes anonymous namespaces (State = Namespace).
+        // For our purposes, they're not namespaces and we ignore them.
+        BraceStack.push_back(false);
+      }
+      State = Default;
+      break;
+    case tok::r_brace:
+      // If braces are unmatched, we're going to be confused, but don't
+      // crash.
+      if (!BraceStack.empty()) {
+        if (BraceStack.back()) {
+          // Parsed: } // namespace
+          Enclosing.pop_back();
+          Event.Trigger = NamespaceEvent::EndNamespace;
+          Event.Payload = llvm::join(Enclosing, "::");
+          Callback(Event);
         }
-      });
+        BraceStack.pop_back();
+      }
+      break;
+    case tok::semi:
+      if (State == UsingNamespaceName) {
+        // Parsed: using namespace <name> ;
+        Event.Trigger = NamespaceEvent::UsingDirective;
+        Event.Payload = std::move(NSName);
+        Callback(Event);
+      }
+      State = Default;
+      break;
+    default:
+      State = Default;
+      break;
+    }
+  });
 }
 
 // Returns the prefix namespaces of NS: {"" ... NS}.
@@ -797,12 +795,12 @@ llvm::SmallVector<llvm::StringRef, 8> ancestorNamespaces(llvm::StringRef NS) {
 } // namespace
 
 std::vector<std::string> visibleNamespaces(llvm::StringRef Code,
-                                           const format::FormatStyle &Style) {
+                                           const LangOptions &LangOpts) {
   std::string Current;
   // Map from namespace to (resolved) namespaces introduced via using directive.
   llvm::StringMap<llvm::StringSet<>> UsingDirectives;
 
-  parseNamespaceEvents(Code, Style, [&](NamespaceEvent Event) {
+  parseNamespaceEvents(Code, LangOpts, [&](NamespaceEvent Event) {
     llvm::StringRef NS = Event.Payload;
     switch (Event.Trigger) {
     case NamespaceEvent::BeginNamespace:
@@ -956,14 +954,14 @@ llvm::Error reformatEdit(Edit &E, const format::FormatStyle &Style) {
 
 EligibleRegion getEligiblePoints(llvm::StringRef Code,
                                  llvm::StringRef FullyQualifiedName,
-                                 const format::FormatStyle &Style) {
+                                 const LangOptions &LangOpts) {
   EligibleRegion ER;
   // Start with global namespace.
   std::vector<std::string> Enclosing = {""};
   // FIXME: In addition to namespaces try to generate events for function
   // definitions as well. One might use a closing parantheses(")" followed by an
   // opening brace "{" to trigger the start.
-  parseNamespaceEvents(Code, Style, [&](NamespaceEvent Event) {
+  parseNamespaceEvents(Code, LangOpts, [&](NamespaceEvent Event) {
     // Using Directives only introduces declarations to current scope, they do
     // not change the current namespace, so skip them.
     if (Event.Trigger == NamespaceEvent::UsingDirective)
diff --git a/clang-tools-extra/clangd/SourceCode.h b/clang-tools-extra/clangd/SourceCode.h
@@ -250,7 +250,7 @@ llvm::StringSet<> collectWords(llvm::StringRef Content);
 ///
 /// visibleNamespaces are {"foo::", "", "a::", "b::", "foo::b::"}, not "a::b::".
 std::vector<std::string> visibleNamespaces(llvm::StringRef Code,
-                                           const format::FormatStyle &Style);
+                                           const LangOptions &LangOpts);
 
 /// Represents locations that can accept a definition.
 struct EligibleRegion {
@@ -271,7 +271,7 @@ struct EligibleRegion {
 /// \p FullyQualifiedName should not contain anonymous namespaces.
 EligibleRegion getEligiblePoints(llvm::StringRef Code,
                                  llvm::StringRef FullyQualifiedName,
-                                 const format::FormatStyle &Style);
+                                 const LangOptions &LangOpts);
 
 struct DefinedMacro {
   llvm::StringRef Name;
diff --git a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -284,10 +284,10 @@ struct InsertionPoint {
 // should also try to follow ordering of declarations. For example, if decls
 // come in order `foo, bar, baz` then this function should return some point
 // between foo and baz for inserting bar.
-llvm::Expected<InsertionPoint>
-getInsertionPoint(llvm::StringRef Contents, llvm::StringRef QualifiedName,
-                  const format::FormatStyle &Style) {
-  auto Region = getEligiblePoints(Contents, QualifiedName, Style);
+llvm::Expected<InsertionPoint> getInsertionPoint(llvm::StringRef Contents,
+                                                 llvm::StringRef QualifiedName,
+                                                 const LangOptions &LangOpts) {
+  auto Region = getEligiblePoints(Contents, QualifiedName, LangOpts);
 
   assert(!Region.EligiblePoints.empty());
   // FIXME: This selection can be made smarter by looking at the definition
@@ -416,9 +416,10 @@ class DefineOutline : public Tweak {
       return llvm::createStringError(Buffer.getError(),
                                      Buffer.getError().message());
     auto Contents = Buffer->get()->getBuffer();
-    auto InsertionPoint =
-        getInsertionPoint(Contents, Source->getQualifiedNameAsString(),
-                          getFormatStyleForFile(*CCFile, Contents, &FS));
+    auto LangOpts = format::getFormattingLangOpts(
+        getFormatStyleForFile(*CCFile, Contents, &FS));
+    auto InsertionPoint = getInsertionPoint(
+        Contents, Source->getQualifiedNameAsString(), LangOpts);
     if (!InsertionPoint)
       return InsertionPoint.takeError();
 
diff --git a/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp b/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
@@ -416,7 +416,8 @@ TEST(SourceCodeTests, VisibleNamespaces) {
   };
   for (const auto &Case : Cases) {
     EXPECT_EQ(Case.second,
-              visibleNamespaces(Case.first, format::getLLVMStyle()))
+              visibleNamespaces(Case.first, format::getFormattingLangOpts(
+                                                format::getLLVMStyle())))
         << Case.first;
   }
 }
@@ -639,8 +640,9 @@ TEST(SourceCodeTests, GetEligiblePoints) {
   for (auto Case : Cases) {
     Annotations Test(Case.Code);
 
-    auto Res = getEligiblePoints(Test.code(), Case.FullyQualifiedName,
-                                 format::getLLVMStyle());
+    auto Res = getEligiblePoints(
+        Test.code(), Case.FullyQualifiedName,
+        format::getFormattingLangOpts(format::getLLVMStyle()));
     EXPECT_THAT(Res.EligiblePoints, testing::ElementsAreArray(Test.points()))
         << Test.code();
     EXPECT_EQ(Res.EnclosingNamespace, Case.EnclosingNamespace) << Test.code();
