[PATCH] D82436: [clangd] Implement textDocument/foldingRange

2020-07-14 Thread Kirill Bobyrev via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7a514c9bf8f2: [clangd] Implement textDocument/foldingRange 
(authored by kbobyrev).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82436/new/

https://reviews.llvm.org/D82436

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/SemanticSelection.cpp
  clang-tools-extra/clangd/SemanticSelection.h
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp
@@ -17,15 +17,19 @@
 #include "TestTU.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/Support/Error.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
+
 namespace clang {
 namespace clangd {
 namespace {
+
 using ::testing::ElementsAre;
 using ::testing::ElementsAreArray;
+using ::testing::UnorderedElementsAreArray;
 
 // front() is SR.range, back() is outermost range.
 std::vector gatherRanges(const SelectionRange ) {
@@ -35,6 +39,20 @@
   return Ranges;
 }
 
+std::vector
+gatherFoldingRanges(llvm::ArrayRef FoldingRanges) {
+  std::vector Ranges;
+  Range NextRange;
+  for (const auto  : FoldingRanges) {
+NextRange.start.line = R.startLine;
+NextRange.start.character = R.startCharacter;
+NextRange.end.line = R.endLine;
+NextRange.end.character = R.endCharacter;
+Ranges.push_back(NextRange);
+  }
+  return Ranges;
+}
+
 TEST(SemanticSelection, All) {
   const char *Tests[] = {
   R"cpp( // Single statement in a function body.
@@ -118,16 +136,16 @@
   )cpp",
   R"cpp( // Inside struct.
 struct A { static int a(); };
-[[struct B { 
+[[struct B {
   [[static int b() [[{
 [[return 1^1]] + 2;
   }
 }]];
   )cpp",
   // Namespaces.
-  R"cpp( 
-[[namespace nsa { 
-  [[namespace nsb { 
+  R"cpp(
+[[namespace nsa {
+  [[namespace nsb {
 static int ccc();
 [[void func() [[{
   // int x = nsa::nsb::ccc();
@@ -181,6 +199,41 @@
   EXPECT_THAT(gatherRanges(Ranges->back()),
   ElementsAre(SourceAnnotations.range("empty")));
 }
+
+TEST(FoldingRanges, All) {
+  const char *Tests[] = {
+  R"cpp(
+[[int global_variable]];
+
+[[void func() {
+  int v = 100;
+}]]
+  )cpp",
+  R"cpp(
+[[class Foo {
+public:
+  [[Foo() {
+int X = 1;
+  }]]
+
+private:
+  [[int getBar() {
+return 42;
+  }]]
+
+  [[void getFooBar() { }]]
+}]];
+  )cpp",
+  };
+  for (const char *Test : Tests) {
+auto T = Annotations(Test);
+auto AST = TestTU::withCode(T.code()).build();
+EXPECT_THAT(gatherFoldingRanges(llvm::cantFail(getFoldingRanges(AST))),
+UnorderedElementsAreArray(T.ranges()))
+<< Test;
+  }
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -296,6 +296,14 @@
 Hidden,
 };
 
+opt FoldingRanges{
+"folding-ranges",
+cat(Features),
+desc("Enable preview of FoldingRanges feature"),
+init(false),
+Hidden,
+};
+
 opt WorkerThreadsCount{
 "j",
 cat(Misc),
@@ -676,6 +684,7 @@
   Opts.AsyncThreadsCount = WorkerThreadsCount;
   Opts.BuildRecoveryAST = RecoveryAST;
   Opts.PreserveRecoveryASTType = RecoveryASTType;
+  Opts.FoldingRanges = FoldingRanges;
 
   clangd::CodeCompleteOptions CCOpts;
   CCOpts.IncludeIneligibleResults = IncludeIneligibleResults;
Index: clang-tools-extra/clangd/SemanticSelection.h
===
--- clang-tools-extra/clangd/SemanticSelection.h
+++ clang-tools-extra/clangd/SemanticSelection.h
@@ -25,6 +25,10 @@
 /// If pos is not in any interesting range, return [Pos, Pos).
 llvm::Expected getSemanticRanges(ParsedAST , Position Pos);
 
+/// Returns a list of ranges whose contents might be collapsible in an editor.
+/// This should include large scopes, preprocessor blocks etc.
+llvm::Expected> getFoldingRanges(ParsedAST );
+
 } // namespace clangd
 } // namespace clang
 
Index: 

[PATCH] D82436: [clangd] Implement textDocument/foldingRange

2020-07-14 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev updated this revision to Diff 277683.
kbobyrev marked an inline comment as done.
kbobyrev added a comment.

Remove llvm::Optional from character fields.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82436/new/

https://reviews.llvm.org/D82436

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/SemanticSelection.cpp
  clang-tools-extra/clangd/SemanticSelection.h
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp
@@ -17,15 +17,19 @@
 #include "TestTU.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/Support/Error.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
+
 namespace clang {
 namespace clangd {
 namespace {
+
 using ::testing::ElementsAre;
 using ::testing::ElementsAreArray;
+using ::testing::UnorderedElementsAreArray;
 
 // front() is SR.range, back() is outermost range.
 std::vector gatherRanges(const SelectionRange ) {
@@ -35,6 +39,20 @@
   return Ranges;
 }
 
+std::vector
+gatherFoldingRanges(llvm::ArrayRef FoldingRanges) {
+  std::vector Ranges;
+  Range NextRange;
+  for (const auto  : FoldingRanges) {
+NextRange.start.line = R.startLine;
+NextRange.start.character = R.startCharacter;
+NextRange.end.line = R.endLine;
+NextRange.end.character = R.endCharacter;
+Ranges.push_back(NextRange);
+  }
+  return Ranges;
+}
+
 TEST(SemanticSelection, All) {
   const char *Tests[] = {
   R"cpp( // Single statement in a function body.
@@ -118,16 +136,16 @@
   )cpp",
   R"cpp( // Inside struct.
 struct A { static int a(); };
-[[struct B { 
+[[struct B {
   [[static int b() [[{
 [[return 1^1]] + 2;
   }
 }]];
   )cpp",
   // Namespaces.
-  R"cpp( 
-[[namespace nsa { 
-  [[namespace nsb { 
+  R"cpp(
+[[namespace nsa {
+  [[namespace nsb {
 static int ccc();
 [[void func() [[{
   // int x = nsa::nsb::ccc();
@@ -181,6 +199,41 @@
   EXPECT_THAT(gatherRanges(Ranges->back()),
   ElementsAre(SourceAnnotations.range("empty")));
 }
+
+TEST(FoldingRanges, All) {
+  const char *Tests[] = {
+  R"cpp(
+[[int global_variable]];
+
+[[void func() {
+  int v = 100;
+}]]
+  )cpp",
+  R"cpp(
+[[class Foo {
+public:
+  [[Foo() {
+int X = 1;
+  }]]
+
+private:
+  [[int getBar() {
+return 42;
+  }]]
+
+  [[void getFooBar() { }]]
+}]];
+  )cpp",
+  };
+  for (const char *Test : Tests) {
+auto T = Annotations(Test);
+auto AST = TestTU::withCode(T.code()).build();
+EXPECT_THAT(gatherFoldingRanges(llvm::cantFail(getFoldingRanges(AST))),
+UnorderedElementsAreArray(T.ranges()))
+<< Test;
+  }
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -296,6 +296,14 @@
 Hidden,
 };
 
+opt FoldingRanges{
+"folding-ranges",
+cat(Features),
+desc("Enable preview of FoldingRanges feature"),
+init(false),
+Hidden,
+};
+
 opt WorkerThreadsCount{
 "j",
 cat(Misc),
@@ -676,6 +684,7 @@
   Opts.AsyncThreadsCount = WorkerThreadsCount;
   Opts.BuildRecoveryAST = RecoveryAST;
   Opts.PreserveRecoveryASTType = RecoveryASTType;
+  Opts.FoldingRanges = FoldingRanges;
 
   clangd::CodeCompleteOptions CCOpts;
   CCOpts.IncludeIneligibleResults = IncludeIneligibleResults;
Index: clang-tools-extra/clangd/SemanticSelection.h
===
--- clang-tools-extra/clangd/SemanticSelection.h
+++ clang-tools-extra/clangd/SemanticSelection.h
@@ -25,6 +25,10 @@
 /// If pos is not in any interesting range, return [Pos, Pos).
 llvm::Expected getSemanticRanges(ParsedAST , Position Pos);
 
+/// Returns a list of ranges whose contents might be collapsible in an editor.
+/// This should include large scopes, preprocessor blocks etc.
+llvm::Expected> getFoldingRanges(ParsedAST );
+
 } // namespace clangd
 } // namespace clang
 
Index: 

[PATCH] D82436: [clangd] Implement textDocument/foldingRange

2020-07-13 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

Nice, thanks!




Comment at: clang-tools-extra/clangd/Protocol.h:1523
+  unsigned startLine = 0;
+  llvm::Optional startCharacter;
+  unsigned endLine = 0;

for outgoing fields that are optional in the protocol but we always set, we 
tend to omit `Optional`. The protocol is worded in a way that suggests servers 
may continue to set the character fields even for line-oriented editors, and I 
think we should do this, dropping `Optional` here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82436/new/

https://reviews.llvm.org/D82436



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D82436: [clangd] Implement textDocument/foldingRange

2020-07-13 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev updated this revision to Diff 277382.
kbobyrev added a comment.

Rebase on top of master.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82436/new/

https://reviews.llvm.org/D82436

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/SemanticSelection.cpp
  clang-tools-extra/clangd/SemanticSelection.h
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp
@@ -17,15 +17,19 @@
 #include "TestTU.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/Support/Error.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
+
 namespace clang {
 namespace clangd {
 namespace {
+
 using ::testing::ElementsAre;
 using ::testing::ElementsAreArray;
+using ::testing::UnorderedElementsAreArray;
 
 // front() is SR.range, back() is outermost range.
 std::vector gatherRanges(const SelectionRange ) {
@@ -35,6 +39,22 @@
   return Ranges;
 }
 
+std::vector
+gatherFoldingRanges(llvm::ArrayRef FoldingRanges) {
+  std::vector Ranges;
+  Range NextRange;
+  for (const auto  : FoldingRanges) {
+NextRange.start.line = R.startLine;
+EXPECT_TRUE(R.startCharacter);
+NextRange.start.character = *R.startCharacter;
+NextRange.end.line = R.endLine;
+EXPECT_TRUE(R.endCharacter);
+NextRange.end.character = *R.endCharacter;
+Ranges.push_back(NextRange);
+  }
+  return Ranges;
+}
+
 TEST(SemanticSelection, All) {
   const char *Tests[] = {
   R"cpp( // Single statement in a function body.
@@ -118,16 +138,16 @@
   )cpp",
   R"cpp( // Inside struct.
 struct A { static int a(); };
-[[struct B { 
+[[struct B {
   [[static int b() [[{
 [[return 1^1]] + 2;
   }
 }]];
   )cpp",
   // Namespaces.
-  R"cpp( 
-[[namespace nsa { 
-  [[namespace nsb { 
+  R"cpp(
+[[namespace nsa {
+  [[namespace nsb {
 static int ccc();
 [[void func() [[{
   // int x = nsa::nsb::ccc();
@@ -181,6 +201,41 @@
   EXPECT_THAT(gatherRanges(Ranges->back()),
   ElementsAre(SourceAnnotations.range("empty")));
 }
+
+TEST(FoldingRanges, All) {
+  const char *Tests[] = {
+  R"cpp(
+int [[global_variable]];
+
+[[void func() {
+  int v = 100;
+}]]
+  )cpp",
+  R"cpp(
+[[class Foo {
+public:
+  [[Foo() {
+int X = 1;
+  }]]
+
+private:
+  [[int getBar() {
+return 42;
+  }]]
+
+  [[void getFooBar() { }]]
+}]];
+  )cpp",
+  };
+  for (const char *Test : Tests) {
+auto T = Annotations(Test);
+auto AST = TestTU::withCode(T.code()).build();
+EXPECT_THAT(gatherFoldingRanges(llvm::cantFail(getFoldingRanges(AST))),
+UnorderedElementsAreArray(T.ranges()))
+<< Test;
+  }
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -296,6 +296,14 @@
 Hidden,
 };
 
+opt FoldingRanges{
+"folding-ranges",
+cat(Features),
+desc("Enable preview of FoldingRanges feature"),
+init(false),
+Hidden,
+};
+
 opt WorkerThreadsCount{
 "j",
 cat(Misc),
@@ -676,6 +684,7 @@
   Opts.AsyncThreadsCount = WorkerThreadsCount;
   Opts.BuildRecoveryAST = RecoveryAST;
   Opts.PreserveRecoveryASTType = RecoveryASTType;
+  Opts.FoldingRanges = FoldingRanges;
 
   clangd::CodeCompleteOptions CCOpts;
   CCOpts.IncludeIneligibleResults = IncludeIneligibleResults;
Index: clang-tools-extra/clangd/SemanticSelection.h
===
--- clang-tools-extra/clangd/SemanticSelection.h
+++ clang-tools-extra/clangd/SemanticSelection.h
@@ -25,6 +25,10 @@
 /// If pos is not in any interesting range, return [Pos, Pos).
 llvm::Expected getSemanticRanges(ParsedAST , Position Pos);
 
+/// Returns a list of ranges whose contents might be collapsible in an editor.
+/// This should include large scopes, preprocessor blocks etc.
+llvm::Expected> getFoldingRanges(ParsedAST );
+
 } // namespace clangd
 } // namespace clang
 
Index: 

[PATCH] D82436: [clangd] Implement textDocument/foldingRange

2020-07-13 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev updated this revision to Diff 277381.
kbobyrev marked 5 inline comments as done.
kbobyrev added a comment.

Isolate DocumentSymbol range changes into D83668 
.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82436/new/

https://reviews.llvm.org/D82436

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/SemanticSelection.cpp
  clang-tools-extra/clangd/SemanticSelection.h
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp
@@ -17,15 +17,19 @@
 #include "TestTU.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/Support/Error.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
+
 namespace clang {
 namespace clangd {
 namespace {
+
 using ::testing::ElementsAre;
 using ::testing::ElementsAreArray;
+using ::testing::UnorderedElementsAreArray;
 
 // front() is SR.range, back() is outermost range.
 std::vector gatherRanges(const SelectionRange ) {
@@ -35,6 +39,22 @@
   return Ranges;
 }
 
+std::vector
+gatherFoldingRanges(llvm::ArrayRef FoldingRanges) {
+  std::vector Ranges;
+  Range NextRange;
+  for (const auto  : FoldingRanges) {
+NextRange.start.line = R.startLine;
+EXPECT_TRUE(R.startCharacter);
+NextRange.start.character = *R.startCharacter;
+NextRange.end.line = R.endLine;
+EXPECT_TRUE(R.endCharacter);
+NextRange.end.character = *R.endCharacter;
+Ranges.push_back(NextRange);
+  }
+  return Ranges;
+}
+
 TEST(SemanticSelection, All) {
   const char *Tests[] = {
   R"cpp( // Single statement in a function body.
@@ -118,16 +138,16 @@
   )cpp",
   R"cpp( // Inside struct.
 struct A { static int a(); };
-[[struct B { 
+[[struct B {
   [[static int b() [[{
 [[return 1^1]] + 2;
   }
 }]];
   )cpp",
   // Namespaces.
-  R"cpp( 
-[[namespace nsa { 
-  [[namespace nsb { 
+  R"cpp(
+[[namespace nsa {
+  [[namespace nsb {
 static int ccc();
 [[void func() [[{
   // int x = nsa::nsb::ccc();
@@ -181,6 +201,41 @@
   EXPECT_THAT(gatherRanges(Ranges->back()),
   ElementsAre(SourceAnnotations.range("empty")));
 }
+
+TEST(FoldingRanges, All) {
+  const char *Tests[] = {
+  R"cpp(
+int [[global_variable]];
+
+[[void func() {
+  int v = 100;
+}]]
+  )cpp",
+  R"cpp(
+[[class Foo {
+public:
+  [[Foo() {
+int X = 1;
+  }]]
+
+private:
+  [[int getBar() {
+return 42;
+  }]]
+
+  [[void getFooBar() { }]]
+}]];
+  )cpp",
+  };
+  for (const char *Test : Tests) {
+auto T = Annotations(Test);
+auto AST = TestTU::withCode(T.code()).build();
+EXPECT_THAT(gatherFoldingRanges(llvm::cantFail(getFoldingRanges(AST))),
+UnorderedElementsAreArray(T.ranges()))
+<< Test;
+  }
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -296,6 +296,14 @@
 Hidden,
 };
 
+opt FoldingRanges{
+"folding-ranges",
+cat(Features),
+desc("Enable preview of FoldingRanges feature"),
+init(false),
+Hidden,
+};
+
 opt WorkerThreadsCount{
 "j",
 cat(Misc),
@@ -659,6 +667,7 @@
   Opts.AsyncThreadsCount = WorkerThreadsCount;
   Opts.BuildRecoveryAST = RecoveryAST;
   Opts.PreserveRecoveryASTType = RecoveryASTType;
+  Opts.FoldingRanges = FoldingRanges;
 
   clangd::CodeCompleteOptions CCOpts;
   CCOpts.IncludeIneligibleResults = IncludeIneligibleResults;
Index: clang-tools-extra/clangd/SemanticSelection.h
===
--- clang-tools-extra/clangd/SemanticSelection.h
+++ clang-tools-extra/clangd/SemanticSelection.h
@@ -25,6 +25,10 @@
 /// If pos is not in any interesting range, return [Pos, Pos).
 llvm::Expected getSemanticRanges(ParsedAST , Position Pos);
 
+/// Returns a list of ranges whose contents might be collapsible in an editor.
+/// This should include large scopes, preprocessor blocks etc.
+llvm::Expected> 

[PATCH] D82436: [clangd] Implement textDocument/foldingRange

2020-07-13 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments.



Comment at: clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp:205
+
+TEST(SemanticSelection, FoldingRanges) {
+  const char *Tests[] = {

nit, the test is usually named after the API/feature, so this should be 
TEST(FoldingRanges, All) or so


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82436/new/

https://reviews.llvm.org/D82436



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D82436: [clangd] Implement textDocument/foldingRange

2020-07-13 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

I'd suggest splitting out the DocumentSymbol changes into a separate patch from 
the FoldingRanges patches.




Comment at: clang-tools-extra/clangd/FindSymbols.cpp:140
   SourceLocation NameLoc = nameLocation(ND, SM);
   // getFileLoc is a good choice for us, but we also need to make sure
   // sourceLocToPosition won't switch files, so we call getSpellingLoc on top 
of

this whole block seems likely to be obsolete if you switch to 
toHalfOpenFileRange



Comment at: clang-tools-extra/clangd/FindSymbols.cpp:167
   SI.range =
   Range{sourceLocToPosition(SM, BeginLoc), sourceLocToPosition(SM, 
EndLoc)};
+  // Include trailing '}' token for most symbol kinds.

As far as I can tell, this line is just always wrong: EndLoc points at the 
*start* of the last token in the range, so the last token will be omitted from 
the range in all cases.



Comment at: clang-tools-extra/clangd/FindSymbols.cpp:168
   Range{sourceLocToPosition(SM, BeginLoc), sourceLocToPosition(SM, 
EndLoc)};
+  // Include trailing '}' token for most symbol kinds.
+  if (SK == SymbolKind::Class || SK == SymbolKind::Struct ||

I can't see any conceptual reason for this to be conditional.



Comment at: clang-tools-extra/clangd/SemanticSelection.h:28
 
+llvm::Expected> getFoldingRanges(ParsedAST );
+

Can we still have a high-level description of what this function does?

```
Returns a list of ranges whose contents might be collapsible in an editor.
This should include large scopes, preprocessor blocks etc.
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82436/new/

https://reviews.llvm.org/D82436



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D82436: [clangd] Implement textDocument/foldingRange

2020-07-13 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev updated this revision to Diff 277359.
kbobyrev added a comment.

Updated a couple of comments.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82436/new/

https://reviews.llvm.org/D82436

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/FindSymbols.cpp
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/SemanticSelection.cpp
  clang-tools-extra/clangd/SemanticSelection.h
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
  clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp
@@ -17,15 +17,19 @@
 #include "TestTU.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/Support/Error.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
+
 namespace clang {
 namespace clangd {
 namespace {
+
 using ::testing::ElementsAre;
 using ::testing::ElementsAreArray;
+using ::testing::UnorderedElementsAreArray;
 
 // front() is SR.range, back() is outermost range.
 std::vector gatherRanges(const SelectionRange ) {
@@ -35,6 +39,22 @@
   return Ranges;
 }
 
+std::vector
+gatherFoldingRanges(llvm::ArrayRef FoldingRanges) {
+  std::vector Ranges;
+  Range NextRange;
+  for (const auto  : FoldingRanges) {
+NextRange.start.line = R.startLine;
+EXPECT_TRUE(R.startCharacter);
+NextRange.start.character = *R.startCharacter;
+NextRange.end.line = R.endLine;
+EXPECT_TRUE(R.endCharacter);
+NextRange.end.character = *R.endCharacter;
+Ranges.push_back(NextRange);
+  }
+  return Ranges;
+}
+
 TEST(SemanticSelection, All) {
   const char *Tests[] = {
   R"cpp( // Single statement in a function body.
@@ -118,16 +138,16 @@
   )cpp",
   R"cpp( // Inside struct.
 struct A { static int a(); };
-[[struct B { 
+[[struct B {
   [[static int b() [[{
 [[return 1^1]] + 2;
   }
 }]];
   )cpp",
   // Namespaces.
-  R"cpp( 
-[[namespace nsa { 
-  [[namespace nsb { 
+  R"cpp(
+[[namespace nsa {
+  [[namespace nsb {
 static int ccc();
 [[void func() [[{
   // int x = nsa::nsb::ccc();
@@ -181,6 +201,41 @@
   EXPECT_THAT(gatherRanges(Ranges->back()),
   ElementsAre(SourceAnnotations.range("empty")));
 }
+
+TEST(SemanticSelection, FoldingRanges) {
+  const char *Tests[] = {
+  R"cpp(
+int [[global_variable]];
+
+[[void func() {
+  int v = 100;
+}]]
+  )cpp",
+  R"cpp(
+[[class Foo {
+public:
+  [[Foo() {
+int X = 1;
+  }]]
+
+private:
+  [[int getBar() {
+return 42;
+  }]]
+
+  [[void getFooBar() { }]]
+}]];
+  )cpp",
+  };
+  for (const char *Test : Tests) {
+auto T = Annotations(Test);
+auto AST = TestTU::withCode(T.code()).build();
+EXPECT_THAT(gatherFoldingRanges(llvm::cantFail(getFoldingRanges(AST))),
+UnorderedElementsAreArray(T.ranges()))
+<< Test;
+  }
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
+++ clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
@@ -8,6 +8,7 @@
 #include "Annotations.h"
 #include "ClangdServer.h"
 #include "FindSymbols.h"
+#include "Protocol.h"
 #include "SyncAPI.h"
 #include "TestFS.h"
 #include "gmock/gmock.h"
@@ -33,7 +34,7 @@
 }
 MATCHER_P(WithName, N, "") { return arg.name == N; }
 MATCHER_P(WithKind, Kind, "") { return arg.kind == Kind; }
-MATCHER_P(SymRange, Range, "") { return arg.location.range == Range; }
+MATCHER_P(SymRange, Range, "") { return arg.range == Range; }
 
 // GMock helpers for matching DocumentSymbol.
 MATCHER_P(SymNameRange, Range, "") { return arg.selectionRange == Range; }
@@ -746,5 +747,71 @@
WithName("Foo_type::method3")));
 }
 
+TEST_F(DocumentSymbolsTest, Ranges) {
+  std::string FilePath = testPath("foo.cpp");
+  Annotations Main(R"(
+  $foo[[int foo(bool Argument) {
+return 42;
+  }]]
+
+  char $variable[[GLOBAL_VARIABLE]];
+
+  $ns[[namespace ns {
+  $bar[[class Bar {
+  public:
+$ctor[[Bar() {}]]
+$dtor[[~Bar()]];
+
+  private:
+

[PATCH] D82436: [clangd] Implement textDocument/foldingRange

2020-07-13 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev updated this revision to Diff 277357.
kbobyrev marked 4 inline comments as done.
kbobyrev added a comment.

Add tests and fix DocumentSymbol ranges.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82436/new/

https://reviews.llvm.org/D82436

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/FindSymbols.cpp
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/SemanticSelection.cpp
  clang-tools-extra/clangd/SemanticSelection.h
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
  clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp
@@ -17,15 +17,19 @@
 #include "TestTU.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/Support/Error.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
+
 namespace clang {
 namespace clangd {
 namespace {
+
 using ::testing::ElementsAre;
 using ::testing::ElementsAreArray;
+using ::testing::UnorderedElementsAreArray;
 
 // front() is SR.range, back() is outermost range.
 std::vector gatherRanges(const SelectionRange ) {
@@ -35,6 +39,23 @@
   return Ranges;
 }
 
+// front() is SR.range, back() is outermost range.
+std::vector
+gatherFoldingRanges(llvm::ArrayRef FoldingRanges) {
+  std::vector Ranges;
+  Range NextRange;
+  for (const auto  : FoldingRanges) {
+NextRange.start.line = R.startLine;
+EXPECT_TRUE(R.startCharacter);
+NextRange.start.character = *R.startCharacter;
+NextRange.end.line = R.endLine;
+EXPECT_TRUE(R.endCharacter);
+NextRange.end.character = *R.endCharacter;
+Ranges.push_back(NextRange);
+  }
+  return Ranges;
+}
+
 TEST(SemanticSelection, All) {
   const char *Tests[] = {
   R"cpp( // Single statement in a function body.
@@ -118,16 +139,16 @@
   )cpp",
   R"cpp( // Inside struct.
 struct A { static int a(); };
-[[struct B { 
+[[struct B {
   [[static int b() [[{
 [[return 1^1]] + 2;
   }
 }]];
   )cpp",
   // Namespaces.
-  R"cpp( 
-[[namespace nsa { 
-  [[namespace nsb { 
+  R"cpp(
+[[namespace nsa {
+  [[namespace nsb {
 static int ccc();
 [[void func() [[{
   // int x = nsa::nsb::ccc();
@@ -181,6 +202,41 @@
   EXPECT_THAT(gatherRanges(Ranges->back()),
   ElementsAre(SourceAnnotations.range("empty")));
 }
+
+TEST(SemanticSelection, FoldingRanges) {
+  const char *Tests[] = {
+  R"cpp(
+int [[global_variable]];
+
+[[void func() {
+  int v = 100;
+}]]
+  )cpp",
+  R"cpp(
+[[class Foo {
+public:
+  [[Foo() {
+int X = 1;
+  }]]
+
+private:
+  [[int getBar() {
+return 42;
+  }]]
+
+  [[void getFooBar() { }]]
+}]];
+  )cpp",
+  };
+  for (const char *Test : Tests) {
+auto T = Annotations(Test);
+auto AST = TestTU::withCode(T.code()).build();
+EXPECT_THAT(gatherFoldingRanges(llvm::cantFail(getFoldingRanges(AST))),
+UnorderedElementsAreArray(T.ranges()))
+<< Test;
+  }
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
+++ clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
@@ -8,6 +8,7 @@
 #include "Annotations.h"
 #include "ClangdServer.h"
 #include "FindSymbols.h"
+#include "Protocol.h"
 #include "SyncAPI.h"
 #include "TestFS.h"
 #include "gmock/gmock.h"
@@ -33,7 +34,7 @@
 }
 MATCHER_P(WithName, N, "") { return arg.name == N; }
 MATCHER_P(WithKind, Kind, "") { return arg.kind == Kind; }
-MATCHER_P(SymRange, Range, "") { return arg.location.range == Range; }
+MATCHER_P(SymRange, Range, "") { return arg.range == Range; }
 
 // GMock helpers for matching DocumentSymbol.
 MATCHER_P(SymNameRange, Range, "") { return arg.selectionRange == Range; }
@@ -746,5 +747,71 @@
WithName("Foo_type::method3")));
 }
 
+TEST_F(DocumentSymbolsTest, Ranges) {
+  std::string FilePath = testPath("foo.cpp");
+  Annotations Main(R"(
+  $foo[[int foo(bool Argument) {
+return 42;
+  }]]
+
+  char $variable[[GLOBAL_VARIABLE]];
+
+  $ns[[namespace ns {
+  

[PATCH] D82436: [clangd] Implement textDocument/foldingRange

2020-07-09 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

In D82436#2142631 , @kbobyrev wrote:

> In D82436#2141953 , @sammccall wrote:
>
> > Tests :-)
>
>
> I was hoping glorious DocumentSymbols super tested API would shield me from 
> that :P


While the implementation is shared, a very small set of tests is fine. We 
should at least include some of the ranges that are "incorrect" that we want to 
fix.

> Didn't think of reasonable testing strategy but fair enough, will do!

I think you can literally just use an `Annotations` marked with all the ranges 
you expect.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82436/new/

https://reviews.llvm.org/D82436



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D82436: [clangd] Implement textDocument/foldingRange

2020-07-09 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev marked an inline comment as done.
kbobyrev added a comment.

In D82436#2141953 , @sammccall wrote:

> Tests :-)


I was hoping glorious DocumentSymbols super tested API would shield me from 
that :P Didn't think of reasonable testing strategy but fair enough, will do!




Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:300
+opt FoldingRanges{
+"folding-rangees",
+cat(Features),

sammccall wrote:
> rangees -> ranges
My broken keyboard puts multiple symbols all over the place :( Catching them 
all is hard, sorry, the other patch also had a couple of those.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82436/new/

https://reviews.llvm.org/D82436



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D82436: [clangd] Implement textDocument/foldingRange

2020-07-09 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

Tests :-)




Comment at: clang-tools-extra/clangd/Protocol.h:1522
+struct FoldingRange {
+  unsigned startLine;
+  llvm::Optional startCharacter;

nit: =0 (and on endLine)



Comment at: clang-tools-extra/clangd/SemanticSelection.cpp:107
+  // FIXME(kirillbobyrev): getDocumentSymbols() is conveniently available but
+  // limited (e.g. doesn't yield blocks inside functions). Replace this with a
+  // more general RecursiveASTVisitor implementation instead.

I think this should mention the contents-of-blocks vs whole-nodes issue too.



Comment at: clang-tools-extra/clangd/SemanticSelection.h:28
 
+/// Retrieves folding ranges in the "main file" section of given AST.
+llvm::Expected> getFoldingRanges(ParsedAST );

I think "main file" section is just a for-now implementation detail, I'd leave 
this out.



Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:300
+opt FoldingRanges{
+"folding-rangees",
+cat(Features),

rangees -> ranges


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82436/new/

https://reviews.llvm.org/D82436



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D82436: [clangd] Implement textDocument/foldingRange

2020-07-07 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev added inline comments.



Comment at: clang-tools-extra/clangd/FindSymbols.cpp:281
+  Range.startCharacter = Symbol.range.start.character;
+  Range.endLine = Symbol.range.end.line;
+  Range.endCharacter = Symbol.range.end.character;

sammccall wrote:
> kbobyrev wrote:
> > sammccall wrote:
> > > How useful are folding ranges when symbols start/end on the same line? Do 
> > > we really want to emit them?
> > I think they are useful from the LSP perspective, if characters didn't 
> > matter in the ranges they wouldn't be included in the protocol at all and 
> > there wouldn't be any way for clients to specify their preferences.
> > 
> > I don't think it's a common use case, but I do think it's a totally valid 
> > one. Maybe those should be the first candidates for excluding when there is 
> > a limit and maybe we could completely cut them for performance. But from 
> > the LSP perspective it seems logical to have those.
> > 
> > What do you think?
> Well, I don't think "the protocol allows regions within a line, therefore we 
> must emit them" is a great argument. You mention a valid use case, but you 
> haven't *described* any use cases - what specific C++ intra-line region is 
> the user likely to fold in a way that's useful?
> 
> I think that it's hard to point to concrete benefits, but the costs are clear 
> enough:
>  - responses will be (much) bigger, which we know some clients don't deal 
> well with
>  - clients are not likely to do any smart filtering, so I think this will 
> lead to performance/ux problems in practice (especially if most 
> implementations only support folding blocks etc)
>  - it makes decisions on what folding regions to emit harder, e.g. should we 
> fold function calls, not fold them, or fold them only if start/end are on 
> different lines?
>  - we need two different modes, to handle clients that support line-folds vs 
> clients that support char-folds. (If we only emit multi-line folds we could 
> also only emit the inner (or the outer) fold for a pair of lines, and the 
> result would be suitable for both types of editors)
> 
> > when there is a limit
> It's pretty important that this feature behaves consistently, for it to be 
> useful. If we're going to only-sometimes enable folding of certain 
> constructs, it better be for a reason that's pretty obvious to the user (I 
> believe single vs multiline qualifies, but "the document complexity exceeds 
> X" certainly doesn't.) For this reason I don't think we should implement that 
> capabilities feature (or should do it in some totally naive and unobtrusive 
> way, like truncation)
As discussed online, we should emit single-line ranges.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82436/new/

https://reviews.llvm.org/D82436



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D82436: [clangd] Implement textDocument/foldingRange

2020-07-07 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev updated this revision to Diff 275929.
kbobyrev marked 7 inline comments as done.
kbobyrev added a comment.

Hide FoldingRanges feature behind the flag.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82436/new/

https://reviews.llvm.org/D82436

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/SemanticSelection.cpp
  clang-tools-extra/clangd/SemanticSelection.h
  clang-tools-extra/clangd/tool/ClangdMain.cpp

Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -296,6 +296,14 @@
 Hidden,
 };
 
+opt FoldingRanges{
+"folding-rangees",
+cat(Features),
+desc("Enable preview of FoldingRanges feature"),
+init(false),
+Hidden,
+};
+
 opt WorkerThreadsCount{
 "j",
 cat(Misc),
@@ -659,6 +667,7 @@
   Opts.AsyncThreadsCount = WorkerThreadsCount;
   Opts.BuildRecoveryAST = RecoveryAST;
   Opts.PreserveRecoveryASTType = RecoveryASTType;
+  Opts.FoldingRanges = FoldingRanges;
 
   clangd::CodeCompleteOptions CCOpts;
   CCOpts.IncludeIneligibleResults = IncludeIneligibleResults;
Index: clang-tools-extra/clangd/SemanticSelection.h
===
--- clang-tools-extra/clangd/SemanticSelection.h
+++ clang-tools-extra/clangd/SemanticSelection.h
@@ -25,6 +25,9 @@
 /// If pos is not in any interesting range, return [Pos, Pos).
 llvm::Expected getSemanticRanges(ParsedAST , Position Pos);
 
+/// Retrieves folding ranges in the "main file" section of given AST.
+llvm::Expected> getFoldingRanges(ParsedAST );
+
 } // namespace clangd
 } // namespace clang
 
Index: clang-tools-extra/clangd/SemanticSelection.cpp
===
--- clang-tools-extra/clangd/SemanticSelection.cpp
+++ clang-tools-extra/clangd/SemanticSelection.cpp
@@ -6,6 +6,7 @@
 //
 //===--===//
 #include "SemanticSelection.h"
+#include "FindSymbols.h"
 #include "ParsedAST.h"
 #include "Protocol.h"
 #include "Selection.h"
@@ -18,6 +19,7 @@
 namespace clang {
 namespace clangd {
 namespace {
+
 // Adds Range \p R to the Result if it is distinct from the last added Range.
 // Assumes that only consecutive ranges can coincide.
 void addIfDistinct(const Range , std::vector ) {
@@ -25,6 +27,20 @@
 Result.push_back(R);
   }
 }
+
+// Recursively collects FoldingRange from a symbol and its children.
+void collectFoldingRanges(DocumentSymbol Symbol,
+  std::vector ) {
+  FoldingRange Range;
+  Range.startLine = Symbol.range.start.line;
+  Range.startCharacter = Symbol.range.start.character;
+  Range.endLine = Symbol.range.end.line;
+  Range.endCharacter = Symbol.range.end.character;
+  Result.push_back(Range);
+  for (const auto  : Symbol.children)
+collectFoldingRanges(Child, Result);
+}
+
 } // namespace
 
 llvm::Expected getSemanticRanges(ParsedAST , Position Pos) {
@@ -81,5 +97,23 @@
   return std::move(Head);
 }
 
+// FIXME(kirillbobyrev): Collect comments, PP conditional regions, includes and
+// other code regions (e.g. public/private/protected sections of classes,
+// control flow statement bodies).
+// Related issue:
+// https://github.com/clangd/clangd/issues/310
+llvm::Expected> getFoldingRanges(ParsedAST ) {
+  // FIXME(kirillbobyrev): getDocumentSymbols() is conveniently available but
+  // limited (e.g. doesn't yield blocks inside functions). Replace this with a
+  // more general RecursiveASTVisitor implementation instead.
+  auto DocumentSymbols = getDocumentSymbols(AST);
+  if (!DocumentSymbols)
+return DocumentSymbols.takeError();
+  std::vector Result;
+  for (const auto  : *DocumentSymbols)
+collectFoldingRanges(Symbol, Result);
+  return Result;
+}
+
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/Protocol.h
===
--- clang-tools-extra/clangd/Protocol.h
+++ clang-tools-extra/clangd/Protocol.h
@@ -1510,6 +1510,23 @@
 };
 llvm::json::Value toJSON(const DocumentLink );
 
+// FIXME(kirillbobyrev): Add FoldingRangeClientCapabilities so we can support
+// per-line-folding editors.
+struct FoldingRangeParams {
+  TextDocumentIdentifier textDocument;
+};
+bool fromJSON(const llvm::json::Value &, FoldingRangeParams &);
+
+/// Stores information about a region of code that can be folded.
+struct FoldingRange {
+  unsigned startLine;
+  llvm::Optional startCharacter;
+  unsigned endLine;
+  llvm::Optional endCharacter;
+  llvm::Optional kind;
+};
+llvm::json::Value toJSON(const 

[PATCH] D82436: [clangd] Implement textDocument/foldingRange

2020-07-02 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

I think this looks reasonable, but I'd like to make sure we have a plan going 
forward otherwise the behavior/assumptions tend to calcify.
I think RAV and adding other region types are clear, but maybe we should 
discuss lines/filtering behavior offline a bit.




Comment at: clang-tools-extra/clangd/FindSymbols.cpp:281
+  Range.startCharacter = Symbol.range.start.character;
+  Range.endLine = Symbol.range.end.line;
+  Range.endCharacter = Symbol.range.end.character;

kbobyrev wrote:
> sammccall wrote:
> > How useful are folding ranges when symbols start/end on the same line? Do 
> > we really want to emit them?
> I think they are useful from the LSP perspective, if characters didn't matter 
> in the ranges they wouldn't be included in the protocol at all and there 
> wouldn't be any way for clients to specify their preferences.
> 
> I don't think it's a common use case, but I do think it's a totally valid 
> one. Maybe those should be the first candidates for excluding when there is a 
> limit and maybe we could completely cut them for performance. But from the 
> LSP perspective it seems logical to have those.
> 
> What do you think?
Well, I don't think "the protocol allows regions within a line, therefore we 
must emit them" is a great argument. You mention a valid use case, but you 
haven't *described* any use cases - what specific C++ intra-line region is the 
user likely to fold in a way that's useful?

I think that it's hard to point to concrete benefits, but the costs are clear 
enough:
 - responses will be (much) bigger, which we know some clients don't deal well 
with
 - clients are not likely to do any smart filtering, so I think this will lead 
to performance/ux problems in practice (especially if most implementations only 
support folding blocks etc)
 - it makes decisions on what folding regions to emit harder, e.g. should we 
fold function calls, not fold them, or fold them only if start/end are on 
different lines?
 - we need two different modes, to handle clients that support line-folds vs 
clients that support char-folds. (If we only emit multi-line folds we could 
also only emit the inner (or the outer) fold for a pair of lines, and the 
result would be suitable for both types of editors)

> when there is a limit
It's pretty important that this feature behaves consistently, for it to be 
useful. If we're going to only-sometimes enable folding of certain constructs, 
it better be for a reason that's pretty obvious to the user (I believe single 
vs multiline qualifies, but "the document complexity exceeds X" certainly 
doesn't.) For this reason I don't think we should implement that capabilities 
feature (or should do it in some totally naive and unobtrusive way, like 
truncation)



Comment at: clang-tools-extra/clangd/FindSymbols.cpp:297
+llvm::Expected> getFoldingRanges(ParsedAST ) {
+  auto DocumentSymbols = collectDocSymbols(AST);
+  std::vector Result;

kbobyrev wrote:
> sammccall wrote:
> > I'm not sure whether this is the right foundation, vs RecursiveASTVisitor.
> > How would we generalize to compoundstmts etc without RAV, and if we do use 
> > RAV for those, why would we still use getDocSymbols for anythnig?
> Sure, I agree. The thing is I wanted to do folding ranges in an incremental 
> way while gradually building the right foundations. RAV + TokenBuffers 
> certainly seems right for the final implementation but it'd be much more code 
> and tests and I didn't want to push a big patch since it is harder to review 
> and also harder to feel the real progress. Pushing this patch allows me to 
> parallelize the work, too, since there are several improvements on the LSP 
> side which wouldn't touch implementation.
> 
> I think it'd be better to leave the current implementation and simply replace 
> it in the next patch, do you see any reasons why this wouldn't be a good 
> option?
That sounds fair enough - it is very simple. I'd suggest leaving a slightly 
more specific fixme: like "getDocumentSymbols() is conveniently available but 
limited (e.g. doesn't yield blocks inside functions). Replace this with a more 
general RecursiveASTVisitor implementation instead."



Comment at: clang-tools-extra/clangd/Protocol.h:1519
+/// Stores information about a region of code that can be folded.
+/// FIXME(kirillbobyrev): Implement FoldingRangeClientCapabilities.
+struct FoldingRange {

This doesn't really specify what is to be fixed and why.
Vs FIXME: add FoldingRangeClientCapabilities so we can support per-line-folding 
editors.

(wouldn't this fixme belong on the *request* rather than the response?)



Comment at: clang-tools-extra/clangd/SemanticSelection.cpp:100
 
+// FIXME(kirillbobyrev): Collect commenets, PP definitions and other code
+// regions (e.g. public/private sections of classes, control flow statement

nit: comments
I 

[PATCH] D82436: [clangd] Implement textDocument/foldingRange

2020-07-01 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev updated this revision to Diff 274741.
kbobyrev marked 7 inline comments as done.
kbobyrev added a comment.

Address review comments.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82436/new/

https://reviews.llvm.org/D82436

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/FindSymbols.cpp
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/SemanticSelection.cpp
  clang-tools-extra/clangd/SemanticSelection.h
  clang-tools-extra/clangd/test/initialize-params.test

Index: clang-tools-extra/clangd/test/initialize-params.test
===
--- clang-tools-extra/clangd/test/initialize-params.test
+++ clang-tools-extra/clangd/test/initialize-params.test
@@ -37,6 +37,7 @@
 # CHECK-NEXT:  "clangd.applyTweak"
 # CHECK-NEXT:]
 # CHECK-NEXT:  },
+# CHECK-NEXT:  "foldingRangeProvider": true,
 # CHECK-NEXT:  "hoverProvider": true,
 # CHECK-NEXT:  "referencesProvider": true,
 # CHECK-NEXT:  "renameProvider": true,
Index: clang-tools-extra/clangd/SemanticSelection.h
===
--- clang-tools-extra/clangd/SemanticSelection.h
+++ clang-tools-extra/clangd/SemanticSelection.h
@@ -25,6 +25,9 @@
 /// If pos is not in any interesting range, return [Pos, Pos).
 llvm::Expected getSemanticRanges(ParsedAST , Position Pos);
 
+/// Retrieves folding ranges in the "main file" section of given AST.
+llvm::Expected> getFoldingRanges(ParsedAST );
+
 } // namespace clangd
 } // namespace clang
 
Index: clang-tools-extra/clangd/SemanticSelection.cpp
===
--- clang-tools-extra/clangd/SemanticSelection.cpp
+++ clang-tools-extra/clangd/SemanticSelection.cpp
@@ -6,6 +6,7 @@
 //
 //===--===//
 #include "SemanticSelection.h"
+#include "FindSymbols.h"
 #include "ParsedAST.h"
 #include "Protocol.h"
 #include "Selection.h"
@@ -18,6 +19,7 @@
 namespace clang {
 namespace clangd {
 namespace {
+
 // Adds Range \p R to the Result if it is distinct from the last added Range.
 // Assumes that only consecutive ranges can coincide.
 void addIfDistinct(const Range , std::vector ) {
@@ -25,6 +27,20 @@
 Result.push_back(R);
   }
 }
+
+// Recursively collects FoldingRange from a symbol and its children.
+void collectFoldingRanges(DocumentSymbol Symbol,
+  std::vector ) {
+  FoldingRange Range;
+  Range.startLine = Symbol.range.start.line;
+  Range.startCharacter = Symbol.range.start.character;
+  Range.endLine = Symbol.range.end.line;
+  Range.endCharacter = Symbol.range.end.character;
+  Result.push_back(Range);
+  for (const auto  : Symbol.children)
+collectFoldingRanges(Child, Result);
+}
+
 } // namespace
 
 llvm::Expected getSemanticRanges(ParsedAST , Position Pos) {
@@ -81,5 +97,18 @@
   return std::move(Head);
 }
 
+// FIXME(kirillbobyrev): Collect commenets, PP definitions and other code
+// regions (e.g. public/private sections of classes, control flow statement
+// bodies).
+llvm::Expected> getFoldingRanges(ParsedAST ) {
+  auto DocumentSymbols = getDocumentSymbols(AST);
+  if (!DocumentSymbols)
+return DocumentSymbols.takeError();
+  std::vector Result;
+  for (const auto  : *DocumentSymbols)
+collectFoldingRanges(Symbol, Result);
+  return Result;
+}
+
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/Protocol.h
===
--- clang-tools-extra/clangd/Protocol.h
+++ clang-tools-extra/clangd/Protocol.h
@@ -1510,6 +1510,22 @@
 };
 llvm::json::Value toJSON(const DocumentLink );
 
+struct FoldingRangeParams {
+  TextDocumentIdentifier textDocument;
+};
+bool fromJSON(const llvm::json::Value &, FoldingRangeParams &);
+
+/// Stores information about a region of code that can be folded.
+/// FIXME(kirillbobyrev): Implement FoldingRangeClientCapabilities.
+struct FoldingRange {
+  unsigned startLine;
+  llvm::Optional startCharacter;
+  unsigned endLine;
+  llvm::Optional endCharacter;
+  llvm::Optional kind;
+};
+llvm::json::Value toJSON(const FoldingRange );
+
 } // namespace clangd
 } // namespace clang
 
Index: clang-tools-extra/clangd/Protocol.cpp
===
--- clang-tools-extra/clangd/Protocol.cpp
+++ clang-tools-extra/clangd/Protocol.cpp
@@ -1241,5 +1241,24 @@
   };
 }
 
+bool fromJSON(const llvm::json::Value , FoldingRangeParams ) {
+  llvm::json::ObjectMapper O(Params);
+  return O && O.map("textDocument", R.textDocument);
+}
+
+llvm::json::Value toJSON(const FoldingRange ) {
+  llvm::json::Object Result{
+  {"startLine", 

[PATCH] D82436: [clangd] Implement textDocument/foldingRange

2020-07-01 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev updated this revision to Diff 274743.
kbobyrev added a comment.

Remove unused variable.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82436/new/

https://reviews.llvm.org/D82436

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/FindSymbols.cpp
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/SemanticSelection.cpp
  clang-tools-extra/clangd/SemanticSelection.h
  clang-tools-extra/clangd/test/initialize-params.test

Index: clang-tools-extra/clangd/test/initialize-params.test
===
--- clang-tools-extra/clangd/test/initialize-params.test
+++ clang-tools-extra/clangd/test/initialize-params.test
@@ -37,6 +37,7 @@
 # CHECK-NEXT:  "clangd.applyTweak"
 # CHECK-NEXT:]
 # CHECK-NEXT:  },
+# CHECK-NEXT:  "foldingRangeProvider": true,
 # CHECK-NEXT:  "hoverProvider": true,
 # CHECK-NEXT:  "referencesProvider": true,
 # CHECK-NEXT:  "renameProvider": true,
Index: clang-tools-extra/clangd/SemanticSelection.h
===
--- clang-tools-extra/clangd/SemanticSelection.h
+++ clang-tools-extra/clangd/SemanticSelection.h
@@ -25,6 +25,9 @@
 /// If pos is not in any interesting range, return [Pos, Pos).
 llvm::Expected getSemanticRanges(ParsedAST , Position Pos);
 
+/// Retrieves folding ranges in the "main file" section of given AST.
+llvm::Expected> getFoldingRanges(ParsedAST );
+
 } // namespace clangd
 } // namespace clang
 
Index: clang-tools-extra/clangd/SemanticSelection.cpp
===
--- clang-tools-extra/clangd/SemanticSelection.cpp
+++ clang-tools-extra/clangd/SemanticSelection.cpp
@@ -6,6 +6,7 @@
 //
 //===--===//
 #include "SemanticSelection.h"
+#include "FindSymbols.h"
 #include "ParsedAST.h"
 #include "Protocol.h"
 #include "Selection.h"
@@ -18,6 +19,7 @@
 namespace clang {
 namespace clangd {
 namespace {
+
 // Adds Range \p R to the Result if it is distinct from the last added Range.
 // Assumes that only consecutive ranges can coincide.
 void addIfDistinct(const Range , std::vector ) {
@@ -25,6 +27,20 @@
 Result.push_back(R);
   }
 }
+
+// Recursively collects FoldingRange from a symbol and its children.
+void collectFoldingRanges(DocumentSymbol Symbol,
+  std::vector ) {
+  FoldingRange Range;
+  Range.startLine = Symbol.range.start.line;
+  Range.startCharacter = Symbol.range.start.character;
+  Range.endLine = Symbol.range.end.line;
+  Range.endCharacter = Symbol.range.end.character;
+  Result.push_back(Range);
+  for (const auto  : Symbol.children)
+collectFoldingRanges(Child, Result);
+}
+
 } // namespace
 
 llvm::Expected getSemanticRanges(ParsedAST , Position Pos) {
@@ -81,5 +97,18 @@
   return std::move(Head);
 }
 
+// FIXME(kirillbobyrev): Collect commenets, PP definitions and other code
+// regions (e.g. public/private sections of classes, control flow statement
+// bodies).
+llvm::Expected> getFoldingRanges(ParsedAST ) {
+  auto DocumentSymbols = getDocumentSymbols(AST);
+  if (!DocumentSymbols)
+return DocumentSymbols.takeError();
+  std::vector Result;
+  for (const auto  : *DocumentSymbols)
+collectFoldingRanges(Symbol, Result);
+  return Result;
+}
+
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/Protocol.h
===
--- clang-tools-extra/clangd/Protocol.h
+++ clang-tools-extra/clangd/Protocol.h
@@ -1510,6 +1510,22 @@
 };
 llvm::json::Value toJSON(const DocumentLink );
 
+struct FoldingRangeParams {
+  TextDocumentIdentifier textDocument;
+};
+bool fromJSON(const llvm::json::Value &, FoldingRangeParams &);
+
+/// Stores information about a region of code that can be folded.
+/// FIXME(kirillbobyrev): Implement FoldingRangeClientCapabilities.
+struct FoldingRange {
+  unsigned startLine;
+  llvm::Optional startCharacter;
+  unsigned endLine;
+  llvm::Optional endCharacter;
+  llvm::Optional kind;
+};
+llvm::json::Value toJSON(const FoldingRange );
+
 } // namespace clangd
 } // namespace clang
 
Index: clang-tools-extra/clangd/Protocol.cpp
===
--- clang-tools-extra/clangd/Protocol.cpp
+++ clang-tools-extra/clangd/Protocol.cpp
@@ -1241,5 +1241,24 @@
   };
 }
 
+bool fromJSON(const llvm::json::Value , FoldingRangeParams ) {
+  llvm::json::ObjectMapper O(Params);
+  return O && O.map("textDocument", R.textDocument);
+}
+
+llvm::json::Value toJSON(const FoldingRange ) {
+  llvm::json::Object Result{
+  {"startLine", Range.startLine},
+  {"endLine", 

[PATCH] D82436: [clangd] Implement textDocument/foldingRange

2020-07-01 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev added inline comments.



Comment at: clang-tools-extra/clangd/FindSymbols.cpp:281
+  Range.startCharacter = Symbol.range.start.character;
+  Range.endLine = Symbol.range.end.line;
+  Range.endCharacter = Symbol.range.end.character;

sammccall wrote:
> How useful are folding ranges when symbols start/end on the same line? Do we 
> really want to emit them?
I think they are useful from the LSP perspective, if characters didn't matter 
in the ranges they wouldn't be included in the protocol at all and there 
wouldn't be any way for clients to specify their preferences.

I don't think it's a common use case, but I do think it's a totally valid one. 
Maybe those should be the first candidates for excluding when there is a limit 
and maybe we could completely cut them for performance. But from the LSP 
perspective it seems logical to have those.

What do you think?



Comment at: clang-tools-extra/clangd/FindSymbols.cpp:297
+llvm::Expected> getFoldingRanges(ParsedAST ) {
+  auto DocumentSymbols = collectDocSymbols(AST);
+  std::vector Result;

sammccall wrote:
> I'm not sure whether this is the right foundation, vs RecursiveASTVisitor.
> How would we generalize to compoundstmts etc without RAV, and if we do use 
> RAV for those, why would we still use getDocSymbols for anythnig?
Sure, I agree. The thing is I wanted to do folding ranges in an incremental way 
while gradually building the right foundations. RAV + TokenBuffers certainly 
seems right for the final implementation but it'd be much more code and tests 
and I didn't want to push a big patch since it is harder to review and also 
harder to feel the real progress. Pushing this patch allows me to parallelize 
the work, too, since there are several improvements on the LSP side which 
wouldn't touch implementation.

I think it'd be better to leave the current implementation and simply replace 
it in the next patch, do you see any reasons why this wouldn't be a good option?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82436/new/

https://reviews.llvm.org/D82436



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D82436: [clangd] Implement textDocument/foldingRange

2020-06-30 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments.



Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:967
+  URIForFile FileURI = Params.textDocument.uri;
+  Server->foldingRanges(
+  Params.textDocument.uri.file(),

one we fix the signature, you should just be able to pass the callback through.



Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:969
+  Params.textDocument.uri.file(),
+  [FileURI, Reply = std::move(Reply)](
+  llvm::Expected> Items) mutable {

unused capture (and variable)



Comment at: clang-tools-extra/clangd/ClangdLSPServer.h:90
 Callback);
+  void onFoldingRange(const FoldingRangeParams &, Callback);
   void onCodeAction(const CodeActionParams &, Callback);

this should return (as callback) vector.
You got unlucky: it's sandwiched between onDocumentSymbol and onCodeAction 
which both have to be dynamically typed as they return different structures 
depending on capabilities.



Comment at: clang-tools-extra/clangd/FindSymbols.cpp:281
+  Range.startCharacter = Symbol.range.start.character;
+  Range.endLine = Symbol.range.end.line;
+  Range.endCharacter = Symbol.range.end.character;

How useful are folding ranges when symbols start/end on the same line? Do we 
really want to emit them?



Comment at: clang-tools-extra/clangd/FindSymbols.cpp:297
+llvm::Expected> getFoldingRanges(ParsedAST ) {
+  auto DocumentSymbols = collectDocSymbols(AST);
+  std::vector Result;

I'm not sure whether this is the right foundation, vs RecursiveASTVisitor.
How would we generalize to compoundstmts etc without RAV, and if we do use RAV 
for those, why would we still use getDocSymbols for anythnig?



Comment at: clang-tools-extra/clangd/FindSymbols.h:50
 
+/// Retrieves folding ranges using Document Symbols in the "main file" section
+/// of given AST.

"using Document Symbols" seems like an implementation detail here, I'd keep it 
out of the first line at least.

Maybe separate out explicitly in a second line: Currently this includes the 
ranges of multi-line symbols (functions etc)



Comment at: clang-tools-extra/clangd/FindSymbols.h:52
+/// of given AST.
+llvm::Expected> getFoldingRanges(ParsedAST );
+

This seems like a surprising place to find this function - it only really makes 
sense when considering the current implementation (which will presumably grow).
I think it'd be a more natural fit in SemanticSelection, maybe.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82436/new/

https://reviews.llvm.org/D82436



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D82436: [clangd] Implement textDocument/foldingRange

2020-06-29 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev added a comment.

@sammccall ping


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82436/new/

https://reviews.llvm.org/D82436



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D82436: [clangd] Implement textDocument/foldingRange

2020-06-24 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev created this revision.
kbobyrev added a reviewer: sammccall.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous, 
MaskRay, ilya-biryukov.
Herald added a project: clang.

This patch introduces basic textDocument/foldingRange support. It relies on
textDocument/documentSymbols to collect all symbols and uses takes ranges
to create folds.

The next steps for textDocument/foldingRange support would be:

- Implementing FoldingRangeClientCapabilities and respecting respect client 
preferences
- Specifying folding range kind
- Supporting more folding range types: PP definitions, sequential includes, 
public/private/protected sections of classes and structs

Tested: (Neo)Vim (coc-clangd) and VSCode.

Related issue: https://github.com/clangd/clangd/issues/310


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D82436

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/FindSymbols.cpp
  clang-tools-extra/clangd/FindSymbols.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/test/initialize-params.test

Index: clang-tools-extra/clangd/test/initialize-params.test
===
--- clang-tools-extra/clangd/test/initialize-params.test
+++ clang-tools-extra/clangd/test/initialize-params.test
@@ -37,6 +37,7 @@
 # CHECK-NEXT:  "clangd.applyTweak"
 # CHECK-NEXT:]
 # CHECK-NEXT:  },
+# CHECK-NEXT:  "foldingRangeProvider": true,
 # CHECK-NEXT:  "hoverProvider": true,
 # CHECK-NEXT:  "referencesProvider": true,
 # CHECK-NEXT:  "renameProvider": true,
Index: clang-tools-extra/clangd/Protocol.h
===
--- clang-tools-extra/clangd/Protocol.h
+++ clang-tools-extra/clangd/Protocol.h
@@ -1510,6 +1510,22 @@
 };
 llvm::json::Value toJSON(const DocumentLink );
 
+struct FoldingRangeParams {
+  TextDocumentIdentifier textDocument;
+};
+bool fromJSON(const llvm::json::Value &, FoldingRangeParams &);
+
+/// Stores information about a region of code that can be folded.
+/// FIXME(kirillbobyrev): Implement FoldingRangeClientCapabilities.
+struct FoldingRange {
+  unsigned startLine;
+  llvm::Optional startCharacter;
+  unsigned endLine;
+  llvm::Optional endCharacter;
+  llvm::Optional kind;
+};
+llvm::json::Value toJSON(const FoldingRange );
+
 } // namespace clangd
 } // namespace clang
 
Index: clang-tools-extra/clangd/Protocol.cpp
===
--- clang-tools-extra/clangd/Protocol.cpp
+++ clang-tools-extra/clangd/Protocol.cpp
@@ -1241,5 +1241,24 @@
   };
 }
 
+bool fromJSON(const llvm::json::Value , FoldingRangeParams ) {
+  llvm::json::ObjectMapper O(Params);
+  return O && O.map("textDocument", R.textDocument);
+}
+
+llvm::json::Value toJSON(const FoldingRange ) {
+  llvm::json::Object Result{
+  {"startLine", Range.startLine},
+  {"endLine", Range.endLine},
+  };
+  if (Range.startCharacter)
+Result["startCharacter"] = *Range.startCharacter;
+  if (Range.endCharacter)
+Result["endCharacter"] = *Range.endCharacter;
+  if (Range.kind)
+Result["kind"] = *Range.kind;
+  return Result;
+}
+
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/FindSymbols.h
===
--- clang-tools-extra/clangd/FindSymbols.h
+++ clang-tools-extra/clangd/FindSymbols.h
@@ -47,6 +47,10 @@
 /// same order that they appear.
 llvm::Expected> getDocumentSymbols(ParsedAST );
 
+/// Retrieves folding ranges using Document Symbols in the "main file" section
+/// of given AST.
+llvm::Expected> getFoldingRanges(ParsedAST );
+
 } // namespace clangd
 } // namespace clang
 
Index: clang-tools-extra/clangd/FindSymbols.cpp
===
--- clang-tools-extra/clangd/FindSymbols.cpp
+++ clang-tools-extra/clangd/FindSymbols.cpp
@@ -271,11 +271,35 @@
 std::vector collectDocSymbols(ParsedAST ) {
   return DocumentOutline(AST).build();
 }
+
+// Recursively collects FoldingRange from a symbol and its children.
+void collectFoldingRanges(DocumentSymbol Symbol,
+  std::vector ) {
+  FoldingRange Range;
+  Range.startLine = Symbol.range.start.line;
+  Range.startCharacter = Symbol.range.start.character;
+  Range.endLine = Symbol.range.end.line;
+  Range.endCharacter = Symbol.range.end.character;
+  Result.push_back(Range);
+  for (const auto  : Symbol.children)
+collectFoldingRanges(Child, Result);
+}
 } // namespace
 
 llvm::Expected> getDocumentSymbols(ParsedAST ) {
   return collectDocSymbols(AST);
 }
 
+// FIXME(kirillbobyrev): Collect commenets, PP definitions and other code
+// regions (e.g. public/private sections of classes,