[clang-tools-extra] r365899 - Revert "[clangd] Implement typeHierarchy/resolve for subtypes"

Russell Gallop via cfe-commits cfe-commits at lists.llvm.org
Fri Jul 12 06:35:58 PDT 2019


Author: russell_gallop
Date: Fri Jul 12 06:35:58 2019
New Revision: 365899

URL: http://llvm.org/viewvc/llvm-project?rev=365899&view=rev
Log:
Revert "[clangd] Implement typeHierarchy/resolve for subtypes"

Causing test failure on Windows bot

This reverts commit 5b9484e559d44bd923fc290e335891b1dd2e17c4.

Modified:
    clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
    clang-tools-extra/trunk/clangd/ClangdLSPServer.h
    clang-tools-extra/trunk/clangd/ClangdServer.cpp
    clang-tools-extra/trunk/clangd/ClangdServer.h
    clang-tools-extra/trunk/clangd/Protocol.cpp
    clang-tools-extra/trunk/clangd/Protocol.h
    clang-tools-extra/trunk/clangd/XRefs.cpp
    clang-tools-extra/trunk/clangd/XRefs.h
    clang-tools-extra/trunk/clangd/test/type-hierarchy.test
    clang-tools-extra/trunk/clangd/unittests/TypeHierarchyTests.cpp

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=365899&r1=365898&r2=365899&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Fri Jul 12 06:35:58 2019
@@ -926,13 +926,6 @@ void ClangdLSPServer::onTypeHierarchy(
                         Params.resolve, Params.direction, std::move(Reply));
 }
 
-void ClangdLSPServer::onResolveTypeHierarchy(
-    const ResolveTypeHierarchyItemParams &Params,
-    Callback<Optional<TypeHierarchyItem>> Reply) {
-  Server->resolveTypeHierarchy(Params.item, Params.resolve, Params.direction,
-                               std::move(Reply));
-}
-
 void ClangdLSPServer::applyConfiguration(
     const ConfigurationSettings &Settings) {
   // Per-file update to the compilation database.
@@ -1028,7 +1021,6 @@ ClangdLSPServer::ClangdLSPServer(
   MsgHandler->bind("workspace/didChangeConfiguration", &ClangdLSPServer::onChangeConfiguration);
   MsgHandler->bind("textDocument/symbolInfo", &ClangdLSPServer::onSymbolInfo);
   MsgHandler->bind("textDocument/typeHierarchy", &ClangdLSPServer::onTypeHierarchy);
-  MsgHandler->bind("typeHierarchy/resolve", &ClangdLSPServer::onResolveTypeHierarchy);
   // clang-format on
 }
 

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.h?rev=365899&r1=365898&r2=365899&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.h Fri Jul 12 06:35:58 2019
@@ -100,8 +100,6 @@ private:
                Callback<llvm::Optional<Hover>>);
   void onTypeHierarchy(const TypeHierarchyParams &,
                        Callback<llvm::Optional<TypeHierarchyItem>>);
-  void onResolveTypeHierarchy(const ResolveTypeHierarchyItemParams &,
-                              Callback<llvm::Optional<TypeHierarchyItem>>);
   void onChangeConfiguration(const DidChangeConfigurationParams &);
   void onSymbolInfo(const TextDocumentPositionParams &,
                     Callback<std::vector<SymbolDetails>>);

Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=365899&r1=365898&r2=365899&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Fri Jul 12 06:35:58 2019
@@ -528,13 +528,6 @@ void ClangdServer::typeHierarchy(PathRef
   WorkScheduler.runWithAST("Type Hierarchy", File, Bind(Action, std::move(CB)));
 }
 
-void ClangdServer::resolveTypeHierarchy(
-    TypeHierarchyItem Item, int Resolve, TypeHierarchyDirection Direction,
-    Callback<llvm::Optional<TypeHierarchyItem>> CB) {
-  clangd::resolveTypeHierarchy(Item, Resolve, Direction, Index);
-  CB(Item);
-}
-
 void ClangdServer::onFileEvent(const DidChangeWatchedFilesParams &Params) {
   // FIXME: Do nothing for now. This will be used for indexing and potentially
   // invalidating other caches.

Modified: clang-tools-extra/trunk/clangd/ClangdServer.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=365899&r1=365898&r2=365899&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.h Fri Jul 12 06:35:58 2019
@@ -210,11 +210,6 @@ public:
                      TypeHierarchyDirection Direction,
                      Callback<llvm::Optional<TypeHierarchyItem>> CB);
 
-  /// Resolve type hierarchy item in the given direction.
-  void resolveTypeHierarchy(TypeHierarchyItem Item, int Resolve,
-                            TypeHierarchyDirection Direction,
-                            Callback<llvm::Optional<TypeHierarchyItem>> CB);
-
   /// Retrieve the top symbols from the workspace matching a query.
   void workspaceSymbols(StringRef Query, int Limit,
                         Callback<std::vector<SymbolInformation>> CB);

Modified: clang-tools-extra/trunk/clangd/Protocol.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.cpp?rev=365899&r1=365898&r2=365899&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/Protocol.cpp (original)
+++ clang-tools-extra/trunk/clangd/Protocol.cpp Fri Jul 12 06:35:58 2019
@@ -422,7 +422,8 @@ bool fromJSON(const llvm::json::Value &P
 bool fromJSON(const llvm::json::Value &Params,
               DocumentRangeFormattingParams &R) {
   llvm::json::ObjectMapper O(Params);
-  return O && O.map("textDocument", R.textDocument) && O.map("range", R.range);
+  return O && O.map("textDocument", R.textDocument) &&
+         O.map("range", R.range);
 }
 
 bool fromJSON(const llvm::json::Value &Params,
@@ -444,8 +445,8 @@ bool fromJSON(const llvm::json::Value &P
 
 llvm::json::Value toJSON(const DiagnosticRelatedInformation &DRI) {
   return llvm::json::Object{
-      {"location", DRI.location},
-      {"message", DRI.message},
+    {"location", DRI.location},
+    {"message", DRI.message},
   };
 }
 
@@ -977,8 +978,6 @@ llvm::json::Value toJSON(const TypeHiera
     Result["parents"] = I.parents;
   if (I.children)
     Result["children"] = I.children;
-  if (I.data)
-    Result["data"] = I.data;
   return std::move(Result);
 }
 
@@ -997,18 +996,10 @@ bool fromJSON(const llvm::json::Value &P
   O.map("deprecated", I.deprecated);
   O.map("parents", I.parents);
   O.map("children", I.children);
-  O.map("data", I.data);
 
   return true;
 }
 
-bool fromJSON(const llvm::json::Value &Params,
-              ResolveTypeHierarchyItemParams &P) {
-  llvm::json::ObjectMapper O(Params);
-  return O && O.map("item", P.item) && O.map("resolve", P.resolve) &&
-         O.map("direction", P.direction);
-}
-
 bool fromJSON(const llvm::json::Value &Params, ReferenceParams &R) {
   TextDocumentPositionParams &Base = R;
   return fromJSON(Params, Base);

Modified: clang-tools-extra/trunk/clangd/Protocol.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.h?rev=365899&r1=365898&r2=365899&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/Protocol.h (original)
+++ clang-tools-extra/trunk/clangd/Protocol.h Fri Jul 12 06:35:58 2019
@@ -1127,7 +1127,7 @@ struct TypeHierarchyItem {
   SymbolKind kind;
 
   /// `true` if the hierarchy item is deprecated. Otherwise, `false`.
-  bool deprecated = false;
+  bool deprecated;
 
   /// The URI of the text document where this type hierarchy item belongs to.
   URIForFile uri;
@@ -1153,26 +1153,13 @@ struct TypeHierarchyItem {
   /// descendants. If not defined, the children have not been resolved.
   llvm::Optional<std::vector<TypeHierarchyItem>> children;
 
-  /// An optional 'data' filed, which can be used to identify a type hierarchy
-  /// item in a resolve request.
-  llvm::Optional<std::string> data;
+  /// The protocol has a slot here for an optional 'data' filed, which can
+  /// be used to identify a type hierarchy item in a resolve request. We don't
+  /// need this (the item itself is sufficient to identify what to resolve)
+  /// so don't declare it.
 };
 llvm::json::Value toJSON(const TypeHierarchyItem &);
 llvm::raw_ostream &operator<<(llvm::raw_ostream &, const TypeHierarchyItem &);
-bool fromJSON(const llvm::json::Value &, TypeHierarchyItem &);
-
-/// Parameters for the `typeHierarchy/resolve` request.
-struct ResolveTypeHierarchyItemParams {
-  /// The item to resolve.
-  TypeHierarchyItem item;
-
-  /// The hierarchy levels to resolve. `0` indicates no level.
-  int resolve;
-
-  /// The direction of the hierarchy levels to resolve.
-  TypeHierarchyDirection direction;
-};
-bool fromJSON(const llvm::json::Value &, ResolveTypeHierarchyItemParams &);
 
 struct ReferenceParams : public TextDocumentPositionParams {
   // For now, no options like context.includeDeclaration are supported.

Modified: clang-tools-extra/trunk/clangd/XRefs.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/XRefs.cpp?rev=365899&r1=365898&r2=365899&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/XRefs.cpp (original)
+++ clang-tools-extra/trunk/clangd/XRefs.cpp Fri Jul 12 06:35:58 2019
@@ -893,7 +893,7 @@ llvm::Optional<QualType> getDeducedType(
 
 /// Retrieves the deduced type at a given location (auto, decltype).
 bool hasDeducedType(ParsedAST &AST, SourceLocation SourceLocationBeg) {
-  return (bool)getDeducedType(AST, SourceLocationBeg);
+  return (bool) getDeducedType(AST, SourceLocationBeg);
 }
 
 llvm::Optional<HoverInfo> getHover(ParsedAST &AST, Position Pos,
@@ -1104,10 +1104,6 @@ symbolToTypeHierarchyItem(const Symbol &
   // (https://github.com/clangd/clangd/issues/59).
   THI.range = THI.selectionRange;
   THI.uri = Loc->uri;
-  // Store the SymbolID in the 'data' field. The client will
-  // send this back in typeHierarchy/resolve, allowing us to
-  // continue resolving additional levels of the type hierarchy.
-  THI.data = S.ID.str();
 
   return std::move(THI);
 }
@@ -1251,25 +1247,6 @@ getTypeHierarchy(ParsedAST &AST, Positio
   return Result;
 }
 
-void resolveTypeHierarchy(TypeHierarchyItem &Item, int ResolveLevels,
-                          TypeHierarchyDirection Direction,
-                          const SymbolIndex *Index) {
-  // We only support typeHierarchy/resolve for children, because for parents
-  // we ignore ResolveLevels and return all levels of parents eagerly.
-  if (Direction == TypeHierarchyDirection::Parents || ResolveLevels == 0)
-    return;
-
-  Item.children.emplace();
-
-  if (Index && Item.data) {
-    // We store the item's SymbolID in the 'data' field, and the client
-    // passes it back to us in typeHierarchy/resolve.
-    if (Expected<SymbolID> ID = SymbolID::fromStr(*Item.data)) {
-      fillSubTypes(*ID, *Item.children, Index, ResolveLevels, Item.uri.file());
-    }
-  }
-}
-
 FormattedString HoverInfo::present() const {
   FormattedString Output;
   if (NamespaceScope) {

Modified: clang-tools-extra/trunk/clangd/XRefs.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/XRefs.h?rev=365899&r1=365898&r2=365899&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/XRefs.h (original)
+++ clang-tools-extra/trunk/clangd/XRefs.h Fri Jul 12 06:35:58 2019
@@ -141,10 +141,6 @@ llvm::Optional<TypeHierarchyItem> getTyp
     ParsedAST &AST, Position Pos, int Resolve, TypeHierarchyDirection Direction,
     const SymbolIndex *Index = nullptr, PathRef TUPath = PathRef{});
 
-void resolveTypeHierarchy(TypeHierarchyItem &Item, int ResolveLevels,
-                          TypeHierarchyDirection Direction,
-                          const SymbolIndex *Index);
-
 /// Retrieves the deduced type at a given location (auto, decltype).
 /// Retuns None unless SourceLocationBeg starts an auto/decltype token.
 /// It will return the underlying type.

Modified: clang-tools-extra/trunk/clangd/test/type-hierarchy.test
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/test/type-hierarchy.test?rev=365899&r1=365898&r2=365899&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/test/type-hierarchy.test (original)
+++ clang-tools-extra/trunk/clangd/test/type-hierarchy.test Fri Jul 12 06:35:58 2019
@@ -1,7 +1,7 @@
 # RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
 {"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
 ---
-{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"struct Parent {};\nstruct Child1 : Parent {};\nstruct Child2 : Child1 {};\nstruct Child3 : Child2 {};\nstruct Child4 : Child3 {};"}}}
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"struct Parent {};\nstruct Child1 : Parent {};\nstruct Child2 : Child1 {};\nstruct Child3 : Child2 {};"}}}
 ---
 {"jsonrpc":"2.0","id":1,"method":"textDocument/typeHierarchy","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":2,"character":11},"direction":2,"resolve":1}}
 #      CHECK:  "id": 1
@@ -9,7 +9,6 @@
 # CHECK-NEXT:  "result": {
 # CHECK-NEXT:    "children": [
 # CHECK-NEXT:      {
-# CHECK-NEXT:        "data": "A6576FE083F2949A",
 # CHECK-NEXT:        "kind": 23,
 # CHECK-NEXT:        "name": "Child3",
 # CHECK-NEXT:        "range": {
@@ -115,64 +114,6 @@
 # CHECK-NEXT:    "uri": "file:///clangd-test/main.cpp"
 # CHECK-NEXT:  }
 ---
-{"jsonrpc":"2.0","id":2,"method":"typeHierarchy/resolve","params":{"item":{"uri":"test:///main.cpp","data":"A6576FE083F2949A","name":"Child3","kind":23,"range":{"end":{"character":13,"line":3},"start":{"character":7,"line":3}},"selectionRange":{"end":{"character":13,"line":3},"start":{"character":7,"line":3}}},"direction":0,"resolve":1}}
-#      CHECK:  "id": 2
-# CHECK-NEXT:  "jsonrpc": "2.0",
-# CHECK-NEXT:  "result": {
-# CHECK-NEXT:    "children": [
-# CHECK-NEXT:      {
-# CHECK-NEXT:        "data": "5705B382DFC77CBC",
-# CHECK-NEXT:        "kind": 23,
-# CHECK-NEXT:        "name": "Child4",
-# CHECK-NEXT:        "range": {
-# CHECK-NEXT:          "end": {
-# CHECK-NEXT:            "character": 13,
-# CHECK-NEXT:            "line": 4
-# CHECK-NEXT:          },
-# CHECK-NEXT:          "start": {
-# CHECK-NEXT:            "character": 7,
-# CHECK-NEXT:            "line": 4
-# CHECK-NEXT:          }
-# CHECK-NEXT:        },
-# CHECK-NEXT:        "selectionRange": {
-# CHECK-NEXT:          "end": {
-# CHECK-NEXT:            "character": 13,
-# CHECK-NEXT:            "line": 4
-# CHECK-NEXT:          },
-# CHECK-NEXT:          "start": {
-# CHECK-NEXT:            "character": 7,
-# CHECK-NEXT:            "line": 4
-# CHECK-NEXT:          }
-# CHECK-NEXT:        },
-# CHECK-NEXT:        "uri": "file:///clangd-test/main.cpp"
-# CHECK-NEXT:      }
-# CHECK-NEXT:    ],
-# CHECK-NEXT:    "data": "A6576FE083F2949A",
-# CHECK-NEXT:    "kind": 23,
-# CHECK-NEXT:    "name": "Child3",
-# CHECK-NEXT:    "range": {
-# CHECK-NEXT:      "end": {
-# CHECK-NEXT:        "character": 13,
-# CHECK-NEXT:        "line": 3
-# CHECK-NEXT:      },
-# CHECK-NEXT:      "start": {
-# CHECK-NEXT:        "character": 7,
-# CHECK-NEXT:        "line": 3
-# CHECK-NEXT:      }
-# CHECK-NEXT:    },
-# CHECK-NEXT:    "selectionRange": {
-# CHECK-NEXT:      "end": {
-# CHECK-NEXT:        "character": 13,
-# CHECK-NEXT:        "line": 3
-# CHECK-NEXT:      },
-# CHECK-NEXT:      "start": {
-# CHECK-NEXT:        "character": 7,
-# CHECK-NEXT:        "line": 3
-# CHECK-NEXT:      }
-# CHECK-NEXT:    },
-# CHECK-NEXT:    "uri": "file:///clangd-test/main.cpp"
-# CHECK-NEXT:  }
----
-{"jsonrpc":"2.0","id":3,"method":"shutdown"}
+{"jsonrpc":"2.0","id":2,"method":"shutdown"}
 ---
 {"jsonrpc":"2.0","method":"exit"}

Modified: clang-tools-extra/trunk/clangd/unittests/TypeHierarchyTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/TypeHierarchyTests.cpp?rev=365899&r1=365898&r2=365899&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/unittests/TypeHierarchyTests.cpp (original)
+++ clang-tools-extra/trunk/clangd/unittests/TypeHierarchyTests.cpp Fri Jul 12 06:35:58 2019
@@ -42,17 +42,8 @@ MATCHER_P(WithKind, Kind, "") { return a
 MATCHER_P(SelectionRangeIs, R, "") { return arg.selectionRange == R; }
 template <class... ParentMatchers>
 ::testing::Matcher<TypeHierarchyItem> Parents(ParentMatchers... ParentsM) {
-  return Field(&TypeHierarchyItem::parents,
-               HasValue(UnorderedElementsAre(ParentsM...)));
+  return Field(&TypeHierarchyItem::parents, HasValue(ElementsAre(ParentsM...)));
 }
-template <class... ChildMatchers>
-::testing::Matcher<TypeHierarchyItem> Children(ChildMatchers... ChildrenM) {
-  return Field(&TypeHierarchyItem::children,
-               HasValue(UnorderedElementsAre(ChildrenM...)));
-}
-// Note: "not resolved" is differnt from "resolved but empty"!
-MATCHER(ParentsNotResolved, "") { return !arg.parents; }
-MATCHER(ChildrenNotResolved, "") { return !arg.children; }
 
 TEST(FindRecordTypeAt, TypeOrVariable) {
   Annotations Source(R"cpp(
@@ -612,41 +603,6 @@ struct Child : Parent<T> {};
   EXPECT_THAT(collectSubtypes(Parent, Index.get()), ElementsAre(Child));
 }
 
-TEST(Subtypes, LazyResolution) {
-  Annotations Source(R"cpp(
-struct P^arent {};
-struct Child1 : Parent {};
-struct Child2a : Child1 {};
-struct Child2b : Child1 {};
-)cpp");
-
-  TestTU TU = TestTU::withCode(Source.code());
-  auto AST = TU.build();
-  auto Index = TU.index();
-
-  llvm::Optional<TypeHierarchyItem> Result = getTypeHierarchy(
-      AST, Source.point(), /*ResolveLevels=*/1,
-      TypeHierarchyDirection::Children, Index.get(), "/clangd-test/TestTU.cpp");
-  ASSERT_TRUE(bool(Result));
-  EXPECT_THAT(
-      *Result,
-      AllOf(WithName("Parent"), WithKind(SymbolKind::Struct), Parents(),
-            Children(AllOf(WithName("Child1"), WithKind(SymbolKind::Struct),
-                           ParentsNotResolved(), ChildrenNotResolved()))));
-
-  resolveTypeHierarchy((*Result->children)[0], /*ResolveLevels=*/1,
-                       TypeHierarchyDirection::Children, Index.get());
-
-  EXPECT_THAT(
-      (*Result->children)[0],
-      AllOf(WithName("Child1"), WithKind(SymbolKind::Struct),
-            ParentsNotResolved(),
-            Children(AllOf(WithName("Child2a"), WithKind(SymbolKind::Struct),
-                           ParentsNotResolved(), ChildrenNotResolved()),
-                     AllOf(WithName("Child2b"), WithKind(SymbolKind::Struct),
-                           ParentsNotResolved(), ChildrenNotResolved()))));
-}
-
 } // namespace
 } // namespace clangd
 } // namespace clang




More information about the cfe-commits mailing list