[PATCH] D51724: [clangd] Add "Deprecated" field to Symbol and CodeCompletion.

2018-09-06 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments.



Comment at: unittests/clangd/SymbolCollectorTests.cpp:1020
+  const std::string Header = R"(
+void TestClangc() __attribute__((deprecated("", "")));
+void TestClangd();

No need for the optional parameters,  
```
void TestClangc() __attribute__((deprecated));
```
should work as well


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51724



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


[PATCH] D51724: [clangd] Add "Deprecated" field to Symbol and CodeCompletion.

2018-09-06 Thread Eric Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE341576: [clangd] Add Deprecated field to 
Symbol and CodeCompletion. (authored by ioeric, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D51724?vs=164259=164260#toc

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51724

Files:
  clangd/CodeComplete.cpp
  clangd/CodeComplete.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/Quality.cpp
  clangd/index/Index.cpp
  clangd/index/Index.h
  clangd/index/MemIndex.cpp
  clangd/index/Merge.cpp
  clangd/index/Serialization.cpp
  clangd/index/SymbolCollector.cpp
  clangd/index/SymbolYAML.cpp
  unittests/clangd/CodeCompleteTests.cpp
  unittests/clangd/QualityTests.cpp
  unittests/clangd/SerializationTests.cpp
  unittests/clangd/SymbolCollectorTests.cpp

Index: unittests/clangd/QualityTests.cpp
===
--- unittests/clangd/QualityTests.cpp
+++ unittests/clangd/QualityTests.cpp
@@ -59,7 +59,7 @@
   F.References = 24; // TestTU doesn't count references, so fake it.
   Quality = {};
   Quality.merge(F);
-  EXPECT_FALSE(Quality.Deprecated); // FIXME: Include deprecated bit in index.
+  EXPECT_TRUE(Quality.Deprecated);
   EXPECT_FALSE(Quality.ReservedName);
   EXPECT_EQ(Quality.References, 24u);
   EXPECT_EQ(Quality.Category, SymbolQualitySignals::Function);
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -77,6 +77,7 @@
   return Contains(AllOf(Named(std::move(Name)), Kind(K)));
 }
 MATCHER(IsDocumented, "") { return !arg.Documentation.empty(); }
+MATCHER(Deprecated, "") { return arg.Deprecated; }
 
 std::unique_ptr memIndex(std::vector Symbols) {
   SymbolSlab::Builder Slab;
@@ -161,7 +162,7 @@
   USR += Regex("^.*$").sub(USRFormat, Sym.Name); // e.g. func -> @F@func#
   Sym.ID = SymbolID(USR);
   Sym.SymInfo.Kind = Kind;
-  Sym.IsIndexedForCodeCompletion = true;
+  Sym.Flags |= Symbol::IndexedForCodeCompletion;
   Sym.Origin = SymbolOrigin::Static;
   return Sym;
 }
@@ -720,9 +721,11 @@
 TEST(CompletionTest, GlobalCompletionFiltering) {
 
   Symbol Class = cls("XYZ");
-  Class.IsIndexedForCodeCompletion = false;
+  Class.Flags = static_cast(
+  Class.Flags & ~(Symbol::IndexedForCodeCompletion));
   Symbol Func = func("XYZ::f");
-  Func.IsIndexedForCodeCompletion = false;
+  Func.Flags = static_cast(
+  Func.Flags & ~(Symbol::IndexedForCodeCompletion));
 
   auto Results = completions(R"(//  void f() {
   XYZ::f^
@@ -1374,6 +1377,7 @@
   EXPECT_EQ(R.documentation, "This is x().");
   EXPECT_THAT(R.additionalTextEdits, IsEmpty());
   EXPECT_EQ(R.sortText, sortText(1.0, "x"));
+  EXPECT_FALSE(R.deprecated);
 
   Opts.EnableSnippets = true;
   R = C.render(Opts);
@@ -1392,6 +1396,10 @@
   C.BundleSize = 2;
   R = C.render(Opts);
   EXPECT_EQ(R.detail, "[2 overloads]\n\"foo.h\"");
+
+  C.Deprecated = true;
+  R = C.render(Opts);
+  EXPECT_TRUE(R.deprecated);
 }
 
 TEST(CompletionTest, IgnoreRecoveryResults) {
@@ -1891,12 +1899,24 @@
   Sym.Name = "Clangd_Macro_Test";
   Sym.ID = SymbolID("c:foo.cpp@8@macro@Clangd_Macro_Test");
   Sym.SymInfo.Kind = index::SymbolKind::Macro;
-  Sym.IsIndexedForCodeCompletion = true;
+  Sym.Flags |= Symbol::IndexedForCodeCompletion;
   EXPECT_THAT(completions("#define Clangd_Macro_Test\nClangd_Macro_T^", {Sym})
   .Completions,
   UnorderedElementsAre(Named("Clangd_Macro_Test")));
 }
 
+TEST(CompletionTest, DeprecatedResults) {
+  std::string Body = R"cpp(
+void TestClangd();
+void TestClangc() __attribute__((deprecated("", "")));
+  )cpp";
+
+  EXPECT_THAT(
+  completions(Body + "int main() { TestClang^ }").Completions,
+  UnorderedElementsAre(AllOf(Named("TestClangd"), Not(Deprecated())),
+   AllOf(Named("TestClangc"), Deprecated(;
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: unittests/clangd/SerializationTests.cpp
===
--- unittests/clangd/SerializationTests.cpp
+++ unittests/clangd/SerializationTests.cpp
@@ -35,7 +35,7 @@
   End:
 Line: 1
 Column: 1
-IsIndexedForCodeCompletion:true
+Flags:1
 Documentation:'Foo doc'
 ReturnType:'int'
 IncludeHeaders:
@@ -62,7 +62,7 @@
   End:
 Line: 1
 Column: 1
-IsIndexedForCodeCompletion:false
+Flags:2
 Signature:'-sig'
 CompletionSnippetSuffix:'-snippet'
 ...
@@ -82,7 +82,8 @@
   EXPECT_EQ(Sym1.Documentation, "Foo doc");
   EXPECT_EQ(Sym1.ReturnType, "int");
   EXPECT_EQ(Sym1.CanonicalDeclaration.FileURI, "file:///path/foo.h");
-  EXPECT_TRUE(Sym1.IsIndexedForCodeCompletion);
+  EXPECT_TRUE(Sym1.Flags & Symbol::IndexedForCodeCompletion);
+  EXPECT_FALSE(Sym1.Flags & Symbol::Deprecated);
   EXPECT_THAT(Sym1.IncludeHeaders,
 

[PATCH] D51724: [clangd] Add "Deprecated" field to Symbol and CodeCompletion.

2018-09-06 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 164259.
ioeric added a comment.

- merged with origin/master


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51724

Files:
  clangd/CodeComplete.cpp
  clangd/CodeComplete.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/Quality.cpp
  clangd/index/Index.cpp
  clangd/index/Index.h
  clangd/index/MemIndex.cpp
  clangd/index/Merge.cpp
  clangd/index/Serialization.cpp
  clangd/index/SymbolCollector.cpp
  clangd/index/SymbolYAML.cpp
  unittests/clangd/CodeCompleteTests.cpp
  unittests/clangd/QualityTests.cpp
  unittests/clangd/SerializationTests.cpp
  unittests/clangd/SymbolCollectorTests.cpp

Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -80,8 +80,10 @@
 }
 MATCHER_P(RefCount, R, "") { return int(arg.References) == R; }
 MATCHER_P(ForCodeCompletion, IsIndexedForCodeCompletion, "") {
-  return arg.IsIndexedForCodeCompletion == IsIndexedForCodeCompletion;
+  return static_cast(arg.Flags & Symbol::IndexedForCodeCompletion) ==
+ IsIndexedForCodeCompletion;
 }
+MATCHER(Deprecated, "") { return arg.Flags & Symbol::Deprecated; }
 MATCHER(RefRange, "") {
   const Ref  = testing::get<0>(arg);
   const Range  = testing::get<1>(arg);
@@ -1014,6 +1016,17 @@
  DeclRange(Header.range("used");
 }
 
+TEST_F(SymbolCollectorTest, DeprecatedSymbols) {
+  const std::string Header = R"(
+void TestClangc() __attribute__((deprecated("", "")));
+void TestClangd();
+  )";
+  runSymbolCollector(Header, /**/ "");
+  EXPECT_THAT(Symbols, UnorderedElementsAre(
+   AllOf(QName("TestClangc"), Deprecated()),
+   AllOf(QName("TestClangd"), Not(Deprecated();
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: unittests/clangd/SerializationTests.cpp
===
--- unittests/clangd/SerializationTests.cpp
+++ unittests/clangd/SerializationTests.cpp
@@ -35,7 +35,7 @@
   End:
 Line: 1
 Column: 1
-IsIndexedForCodeCompletion:true
+Flags:1
 Documentation:'Foo doc'
 ReturnType:'int'
 IncludeHeaders:
@@ -62,7 +62,7 @@
   End:
 Line: 1
 Column: 1
-IsIndexedForCodeCompletion:false
+Flags:2
 Signature:'-sig'
 CompletionSnippetSuffix:'-snippet'
 ...
@@ -82,7 +82,8 @@
   EXPECT_EQ(Sym1.Documentation, "Foo doc");
   EXPECT_EQ(Sym1.ReturnType, "int");
   EXPECT_EQ(Sym1.CanonicalDeclaration.FileURI, "file:///path/foo.h");
-  EXPECT_TRUE(Sym1.IsIndexedForCodeCompletion);
+  EXPECT_TRUE(Sym1.Flags & Symbol::IndexedForCodeCompletion);
+  EXPECT_FALSE(Sym1.Flags & Symbol::Deprecated);
   EXPECT_THAT(Sym1.IncludeHeaders,
   UnorderedElementsAre(IncludeHeaderWithRef("include1", 7u),
IncludeHeaderWithRef("include2", 3u)));
@@ -94,7 +95,8 @@
   EXPECT_EQ(Sym2.Signature, "-sig");
   EXPECT_EQ(Sym2.ReturnType, "");
   EXPECT_EQ(Sym2.CanonicalDeclaration.FileURI, "file:///path/bar.h");
-  EXPECT_FALSE(Sym2.IsIndexedForCodeCompletion);
+  EXPECT_FALSE(Sym2.Flags & Symbol::IndexedForCodeCompletion);
+  EXPECT_TRUE(Sym2.Flags & Symbol::Deprecated);
 
   std::string ConcatenatedYAML;
   {
Index: unittests/clangd/QualityTests.cpp
===
--- unittests/clangd/QualityTests.cpp
+++ unittests/clangd/QualityTests.cpp
@@ -59,7 +59,7 @@
   F.References = 24; // TestTU doesn't count references, so fake it.
   Quality = {};
   Quality.merge(F);
-  EXPECT_FALSE(Quality.Deprecated); // FIXME: Include deprecated bit in index.
+  EXPECT_TRUE(Quality.Deprecated);
   EXPECT_FALSE(Quality.ReservedName);
   EXPECT_EQ(Quality.References, 24u);
   EXPECT_EQ(Quality.Category, SymbolQualitySignals::Function);
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -77,6 +77,7 @@
   return Contains(AllOf(Named(std::move(Name)), Kind(K)));
 }
 MATCHER(IsDocumented, "") { return !arg.Documentation.empty(); }
+MATCHER(Deprecated, "") { return arg.Deprecated; }
 
 std::unique_ptr memIndex(std::vector Symbols) {
   SymbolSlab::Builder Slab;
@@ -161,7 +162,7 @@
   USR += Regex("^.*$").sub(USRFormat, Sym.Name); // e.g. func -> @F@func#
   Sym.ID = SymbolID(USR);
   Sym.SymInfo.Kind = Kind;
-  Sym.IsIndexedForCodeCompletion = true;
+  Sym.Flags |= Symbol::IndexedForCodeCompletion;
   Sym.Origin = SymbolOrigin::Static;
   return Sym;
 }
@@ -720,9 +721,11 @@
 TEST(CompletionTest, GlobalCompletionFiltering) {
 
   Symbol Class = cls("XYZ");
-  Class.IsIndexedForCodeCompletion = false;
+  Class.Flags = static_cast(
+  Class.Flags & ~(Symbol::IndexedForCodeCompletion));
   Symbol Func = 

[PATCH] D51724: [clangd] Add "Deprecated" field to Symbol and CodeCompletion.

2018-09-06 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 164258.
ioeric marked 5 inline comments as done.
ioeric added a comment.

- addressed review comments


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51724

Files:
  clangd/CodeComplete.cpp
  clangd/CodeComplete.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/Quality.cpp
  clangd/index/Index.cpp
  clangd/index/Index.h
  clangd/index/MemIndex.cpp
  clangd/index/Merge.cpp
  clangd/index/Serialization.cpp
  clangd/index/SymbolCollector.cpp
  clangd/index/SymbolYAML.cpp
  unittests/clangd/CodeCompleteTests.cpp
  unittests/clangd/QualityTests.cpp
  unittests/clangd/SerializationTests.cpp
  unittests/clangd/SymbolCollectorTests.cpp

Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -80,8 +80,10 @@
 }
 MATCHER_P(RefCount, R, "") { return int(arg.References) == R; }
 MATCHER_P(ForCodeCompletion, IsIndexedForCodeCompletion, "") {
-  return arg.IsIndexedForCodeCompletion == IsIndexedForCodeCompletion;
+  return static_cast(arg.Flags & Symbol::IndexedForCodeCompletion) ==
+ IsIndexedForCodeCompletion;
 }
+MATCHER(Deprecated, "") { return arg.Flags & Symbol::Deprecated; }
 MATCHER(RefRange, "") {
   const Ref  = testing::get<0>(arg);
   const Range  = testing::get<1>(arg);
@@ -1014,6 +1016,17 @@
  DeclRange(Header.range("used");
 }
 
+TEST_F(SymbolCollectorTest, DeprecatedSymbols) {
+  const std::string Header = R"(
+void TestClangc() __attribute__((deprecated("", "")));
+void TestClangd();
+  )";
+  runSymbolCollector(Header, /**/ "");
+  EXPECT_THAT(Symbols, UnorderedElementsAre(
+   AllOf(QName("TestClangc"), Deprecated()),
+   AllOf(QName("TestClangd"), Not(Deprecated();
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: unittests/clangd/SerializationTests.cpp
===
--- unittests/clangd/SerializationTests.cpp
+++ unittests/clangd/SerializationTests.cpp
@@ -35,7 +35,7 @@
   End:
 Line: 1
 Column: 1
-IsIndexedForCodeCompletion:true
+Flags:1
 Documentation:'Foo doc'
 ReturnType:'int'
 IncludeHeaders:
@@ -62,7 +62,7 @@
   End:
 Line: 1
 Column: 1
-IsIndexedForCodeCompletion:false
+Flags:2
 Signature:'-sig'
 CompletionSnippetSuffix:'-snippet'
 ...
@@ -82,7 +82,8 @@
   EXPECT_EQ(Sym1.Documentation, "Foo doc");
   EXPECT_EQ(Sym1.ReturnType, "int");
   EXPECT_EQ(Sym1.CanonicalDeclaration.FileURI, "file:///path/foo.h");
-  EXPECT_TRUE(Sym1.IsIndexedForCodeCompletion);
+  EXPECT_TRUE(Sym1.Flags & Symbol::IndexedForCodeCompletion);
+  EXPECT_FALSE(Sym1.Flags & Symbol::Deprecated);
   EXPECT_THAT(Sym1.IncludeHeaders,
   UnorderedElementsAre(IncludeHeaderWithRef("include1", 7u),
IncludeHeaderWithRef("include2", 3u)));
@@ -94,7 +95,8 @@
   EXPECT_EQ(Sym2.Signature, "-sig");
   EXPECT_EQ(Sym2.ReturnType, "");
   EXPECT_EQ(Sym2.CanonicalDeclaration.FileURI, "file:///path/bar.h");
-  EXPECT_FALSE(Sym2.IsIndexedForCodeCompletion);
+  EXPECT_FALSE(Sym2.Flags & Symbol::IndexedForCodeCompletion);
+  EXPECT_TRUE(Sym2.Flags & Symbol::Deprecated);
 
   std::string ConcatenatedYAML;
   {
Index: unittests/clangd/QualityTests.cpp
===
--- unittests/clangd/QualityTests.cpp
+++ unittests/clangd/QualityTests.cpp
@@ -59,7 +59,7 @@
   F.References = 24; // TestTU doesn't count references, so fake it.
   Quality = {};
   Quality.merge(F);
-  EXPECT_FALSE(Quality.Deprecated); // FIXME: Include deprecated bit in index.
+  EXPECT_TRUE(Quality.Deprecated);
   EXPECT_FALSE(Quality.ReservedName);
   EXPECT_EQ(Quality.References, 24u);
   EXPECT_EQ(Quality.Category, SymbolQualitySignals::Function);
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -77,6 +77,7 @@
   return Contains(AllOf(Named(std::move(Name)), Kind(K)));
 }
 MATCHER(IsDocumented, "") { return !arg.Documentation.empty(); }
+MATCHER(Deprecated, "") { return arg.Deprecated; }
 
 std::unique_ptr memIndex(std::vector Symbols) {
   SymbolSlab::Builder Slab;
@@ -161,7 +162,7 @@
   USR += Regex("^.*$").sub(USRFormat, Sym.Name); // e.g. func -> @F@func#
   Sym.ID = SymbolID(USR);
   Sym.SymInfo.Kind = Kind;
-  Sym.IsIndexedForCodeCompletion = true;
+  Sym.Flags |= Symbol::IndexedForCodeCompletion;
   Sym.Origin = SymbolOrigin::Static;
   return Sym;
 }
@@ -720,9 +721,11 @@
 TEST(CompletionTest, GlobalCompletionFiltering) {
 
   Symbol Class = cls("XYZ");
-  Class.IsIndexedForCodeCompletion = false;
+  Class.Flags = static_cast(
+  Class.Flags & 

[PATCH] D51724: [clangd] Add "Deprecated" field to Symbol and CodeCompletion.

2018-09-06 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.

Agree with Kadir's comments.
I'd just suggest reducing boilerplate a bit by taking some shortcuts.




Comment at: clangd/index/Index.h:167
 
+enum class SymbolFlag : uint8_t {
+  None = 0,

enum class is a pain for bitfields.
I'd just make this a plain enum nested inside symbol, then you don't need to 
define the operators and don't need to cast as often.



Comment at: clangd/index/Index.h:268
+  /// FIXME: also add deprecation message and fixit?
+  bool Deprecated() const {
+return static_cast(Flags & SymbolFlag::Deprecated);

kadircet wrote:
> nit: rename to isDeprecated ?
FWIW I don't think these accessors pull their weight: in calller code `if 
(Sym.Flags & Symbol::IsDeprecated)` is clear enough



Comment at: clangd/index/Serialization.cpp:309
 // data. Later we may want to support some backward compatibility.
 constexpr static uint32_t Version = 2;
 

3


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51724



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


[PATCH] D51724: [clangd] Add "Deprecated" field to Symbol and CodeCompletion.

2018-09-06 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments.



Comment at: clangd/index/Index.h:171
+  /// See also isIndexedForCodeCompletion().
+  IsIndexedForCodeCompletion = 1 << 0,
+  /// Indicates if the symbol is deprecated.

nit: rename to IndexedForCodeCompletion, since it is more of an attribute 
having is in the name doesn't seem so cool. Instead of the attributes 
themselves maybe the checkers below should have "is" prefix.



Comment at: clangd/index/Index.h:172
+  IsIndexedForCodeCompletion = 1 << 0,
+  /// Indicates if the symbol is deprecated.
+  Deprecated = 1 << 1,

nit: Add a comment similar to above one leading to Deprecated()?



Comment at: clangd/index/Index.h:268
+  /// FIXME: also add deprecation message and fixit?
+  bool Deprecated() const {
+return static_cast(Flags & SymbolFlag::Deprecated);

nit: rename to isDeprecated ?



Comment at: unittests/clangd/CodeCompleteTests.cpp:1359
   C.Origin = SymbolOrigin::AST | SymbolOrigin::Static;
+  C.Deprecated = true;
 

Maybe do this on its own render so that we can have both code paths covered. 
Just before rendering with `Opts.EnableSnippets = true` below. We can simply 
set this one render again and check deprecated is set to true.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51724



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


[PATCH] D51724: [clangd] Add "Deprecated" field to Symbol and CodeCompletion.

2018-09-06 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 164205.
ioeric marked an inline comment as done.
ioeric added a comment.

- Pack flags in Symbol.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51724

Files:
  clangd/CodeComplete.cpp
  clangd/CodeComplete.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/Quality.cpp
  clangd/index/Index.cpp
  clangd/index/Index.h
  clangd/index/MemIndex.cpp
  clangd/index/Serialization.cpp
  clangd/index/SymbolCollector.cpp
  clangd/index/SymbolYAML.cpp
  unittests/clangd/CodeCompleteTests.cpp
  unittests/clangd/QualityTests.cpp
  unittests/clangd/SerializationTests.cpp
  unittests/clangd/SymbolCollectorTests.cpp

Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -80,8 +80,9 @@
 }
 MATCHER_P(RefCount, R, "") { return int(arg.References) == R; }
 MATCHER_P(ForCodeCompletion, IsIndexedForCodeCompletion, "") {
-  return arg.IsIndexedForCodeCompletion == IsIndexedForCodeCompletion;
+  return arg.IsIndexedForCodeCompletion() == IsIndexedForCodeCompletion;
 }
+MATCHER(Deprecated, "") { return arg.Deprecated(); }
 MATCHER(RefRange, "") {
   const Ref  = testing::get<0>(arg);
   const Range  = testing::get<1>(arg);
@@ -1014,6 +1015,17 @@
  DeclRange(Header.range("used");
 }
 
+TEST_F(SymbolCollectorTest, DeprecatedSymbols) {
+  const std::string Header = R"(
+void TestClangc() __attribute__((deprecated("", "")));
+void TestClangd();
+  )";
+  runSymbolCollector(Header, /**/ "");
+  EXPECT_THAT(Symbols, UnorderedElementsAre(
+   AllOf(QName("TestClangc"), Deprecated()),
+   AllOf(QName("TestClangd"), Not(Deprecated();
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: unittests/clangd/SerializationTests.cpp
===
--- unittests/clangd/SerializationTests.cpp
+++ unittests/clangd/SerializationTests.cpp
@@ -35,7 +35,7 @@
   End:
 Line: 1
 Column: 1
-IsIndexedForCodeCompletion:true
+Flags:1
 Documentation:'Foo doc'
 ReturnType:'int'
 IncludeHeaders:
@@ -62,7 +62,7 @@
   End:
 Line: 1
 Column: 1
-IsIndexedForCodeCompletion:false
+Flags:2
 Signature:'-sig'
 CompletionSnippetSuffix:'-snippet'
 ...
@@ -82,7 +82,8 @@
   EXPECT_EQ(Sym1.Documentation, "Foo doc");
   EXPECT_EQ(Sym1.ReturnType, "int");
   EXPECT_EQ(Sym1.CanonicalDeclaration.FileURI, "file:///path/foo.h");
-  EXPECT_TRUE(Sym1.IsIndexedForCodeCompletion);
+  EXPECT_TRUE(Sym1.IsIndexedForCodeCompletion());
+  EXPECT_FALSE(Sym1.Deprecated());
   EXPECT_THAT(Sym1.IncludeHeaders,
   UnorderedElementsAre(IncludeHeaderWithRef("include1", 7u),
IncludeHeaderWithRef("include2", 3u)));
@@ -94,7 +95,8 @@
   EXPECT_EQ(Sym2.Signature, "-sig");
   EXPECT_EQ(Sym2.ReturnType, "");
   EXPECT_EQ(Sym2.CanonicalDeclaration.FileURI, "file:///path/bar.h");
-  EXPECT_FALSE(Sym2.IsIndexedForCodeCompletion);
+  EXPECT_FALSE(Sym2.IsIndexedForCodeCompletion());
+  EXPECT_TRUE(Sym2.Deprecated());
 
   std::string ConcatenatedYAML;
   {
Index: unittests/clangd/QualityTests.cpp
===
--- unittests/clangd/QualityTests.cpp
+++ unittests/clangd/QualityTests.cpp
@@ -59,7 +59,7 @@
   F.References = 24; // TestTU doesn't count references, so fake it.
   Quality = {};
   Quality.merge(F);
-  EXPECT_FALSE(Quality.Deprecated); // FIXME: Include deprecated bit in index.
+  EXPECT_TRUE(Quality.Deprecated);
   EXPECT_FALSE(Quality.ReservedName);
   EXPECT_EQ(Quality.References, 24u);
   EXPECT_EQ(Quality.Category, SymbolQualitySignals::Function);
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -77,6 +77,7 @@
   return Contains(AllOf(Named(std::move(Name)), Kind(K)));
 }
 MATCHER(IsDocumented, "") { return !arg.Documentation.empty(); }
+MATCHER(Deprecated, "") { return arg.Deprecated; }
 
 std::unique_ptr memIndex(std::vector Symbols) {
   SymbolSlab::Builder Slab;
@@ -161,7 +162,7 @@
   USR += Regex("^.*$").sub(USRFormat, Sym.Name); // e.g. func -> @F@func#
   Sym.ID = SymbolID(USR);
   Sym.SymInfo.Kind = Kind;
-  Sym.IsIndexedForCodeCompletion = true;
+  Sym.Flags |= SymbolFlag::IsIndexedForCodeCompletion;
   Sym.Origin = SymbolOrigin::Static;
   return Sym;
 }
@@ -720,9 +721,9 @@
 TEST(CompletionTest, GlobalCompletionFiltering) {
 
   Symbol Class = cls("XYZ");
-  Class.IsIndexedForCodeCompletion = false;
+  Class.Flags &= ~(SymbolFlag::IsIndexedForCodeCompletion);
   Symbol Func = func("XYZ::f");
-  Func.IsIndexedForCodeCompletion = false;
+  Func.Flags &= ~(SymbolFlag::IsIndexedForCodeCompletion);
 
   

[PATCH] D51724: [clangd] Add "Deprecated" field to Symbol and CodeCompletion.

2018-09-06 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments.



Comment at: clangd/Protocol.cpp:520
 Result["additionalTextEdits"] = json::Array(CI.additionalTextEdits);
+  if (CI.deprecated)
+Result["deprecated"] = CI.deprecated;

sammccall wrote:
> do we actually want this in JSON?
> (genuinely unsure - any clients aware of this extension?)
This is actually defined in LSP: 
https://github.com/Microsoft/language-server-protocol/blob/gh-pages/specification.md#completion-request-leftwards_arrow_with_hook



Comment at: clangd/index/Index.h:249
+  /// FIXME: also add deprecation message and fixit?
+  bool Deprecated = false;
 };

sammccall wrote:
> would you mind packing this together with IsIndexedForCompletion, for memory 
> size?
> either as an actual bitfield `bool Deprecated : 1 = false` or as enum flags 
> `enum Flags : uint8_t { IndexedForCompletion, Deprecated, }; Flags flags`
> 
> The latter will simplify life for serialization, but up to you.
Done. Pack them into flags.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51724



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


[PATCH] D51724: [clangd] Add "Deprecated" field to Symbol and CodeCompletion.

2018-09-06 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

I think you also need to update SymbolsYAML and Serialization.




Comment at: clangd/Protocol.cpp:520
 Result["additionalTextEdits"] = json::Array(CI.additionalTextEdits);
+  if (CI.deprecated)
+Result["deprecated"] = CI.deprecated;

do we actually want this in JSON?
(genuinely unsure - any clients aware of this extension?)



Comment at: clangd/Protocol.h:771
 
+  /// Indicates if this item is deprecated.
+  bool deprecated = false;

this is a clangd extension.
(right?)



Comment at: clangd/index/Index.h:249
+  /// FIXME: also add deprecation message and fixit?
+  bool Deprecated = false;
 };

would you mind packing this together with IsIndexedForCompletion, for memory 
size?
either as an actual bitfield `bool Deprecated : 1 = false` or as enum flags 
`enum Flags : uint8_t { IndexedForCompletion, Deprecated, }; Flags flags`

The latter will simplify life for serialization, but up to you.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51724



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


[PATCH] D51724: [clangd] Add "Deprecated" field to Symbol and CodeCompletion.

2018-09-06 Thread Eric Liu via Phabricator via cfe-commits
ioeric created this revision.
ioeric added reviewers: sammccall, kadircet.
Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay, 
ilya-biryukov.

Also set "deprecated" field in LSP CompletionItem.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51724

Files:
  clangd/CodeComplete.cpp
  clangd/CodeComplete.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/Quality.cpp
  clangd/index/Index.h
  clangd/index/SymbolCollector.cpp
  unittests/clangd/CodeCompleteTests.cpp
  unittests/clangd/QualityTests.cpp

Index: unittests/clangd/QualityTests.cpp
===
--- unittests/clangd/QualityTests.cpp
+++ unittests/clangd/QualityTests.cpp
@@ -59,7 +59,7 @@
   F.References = 24; // TestTU doesn't count references, so fake it.
   Quality = {};
   Quality.merge(F);
-  EXPECT_FALSE(Quality.Deprecated); // FIXME: Include deprecated bit in index.
+  EXPECT_TRUE(Quality.Deprecated);
   EXPECT_FALSE(Quality.ReservedName);
   EXPECT_EQ(Quality.References, 24u);
   EXPECT_EQ(Quality.Category, SymbolQualitySignals::Function);
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -77,6 +77,7 @@
   return Contains(AllOf(Named(std::move(Name)), Kind(K)));
 }
 MATCHER(IsDocumented, "") { return !arg.Documentation.empty(); }
+MATCHER(Deprecated, "") { return arg.Deprecated; }
 
 std::unique_ptr memIndex(std::vector Symbols) {
   SymbolSlab::Builder Slab;
@@ -1355,6 +1356,7 @@
   C.Kind = CompletionItemKind::Method;
   C.Score.Total = 1.0;
   C.Origin = SymbolOrigin::AST | SymbolOrigin::Static;
+  C.Deprecated = true;
 
   CodeCompleteOptions Opts;
   Opts.IncludeIndicator.Insert = "^";
@@ -1370,6 +1372,7 @@
   EXPECT_EQ(R.documentation, "This is x().");
   EXPECT_THAT(R.additionalTextEdits, IsEmpty());
   EXPECT_EQ(R.sortText, sortText(1.0, "x"));
+  EXPECT_TRUE(R.deprecated);
 
   Opts.EnableSnippets = true;
   R = C.render(Opts);
@@ -1882,6 +1885,18 @@
   AllOf(Named("Func"), HasInclude("\"foo.h\""), Not(InsertInclude();
 }
 
+TEST(CompletionTest, DeprecatedResults) {
+  std::string Body = R"cpp(
+void TestClangd();
+void TestClangc() __attribute__((deprecated("", "")));
+  )cpp";
+
+  EXPECT_THAT(
+  completions(Body + "int main() { TestClang^ }").Completions,
+  UnorderedElementsAre(AllOf(Named("TestClangd"), Not(Deprecated())),
+   AllOf(Named("TestClangc"), Deprecated(;
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -535,6 +535,7 @@
 S.IncludeHeaders.emplace_back(Include, 1);
 
   S.Origin = Opts.Origin;
+  S.Deprecated = ND.getAvailability() == AR_Deprecated;
   Symbols.insert(S);
   return Symbols.find(S.ID);
 }
Index: clangd/index/Index.h
===
--- clangd/index/Index.h
+++ clangd/index/Index.h
@@ -244,7 +244,9 @@
   ///   any definition.
   llvm::SmallVector IncludeHeaders;
 
-  // FIXME: add extra fields for index scoring signals.
+  /// Indicates if the symbol is deprecated.
+  /// FIXME: also add deprecation message and fixit?
+  bool Deprecated = false;
 };
 llvm::raw_ostream <<(llvm::raw_ostream , const Symbol );
 
Index: clangd/Quality.cpp
===
--- clangd/Quality.cpp
+++ clangd/Quality.cpp
@@ -167,9 +167,7 @@
 }
 
 void SymbolQualitySignals::merge(const CodeCompletionResult ) {
-  if (SemaCCResult.Availability == CXAvailability_Deprecated)
-Deprecated = true;
-
+  Deprecated |= (SemaCCResult.Availability == CXAvailability_Deprecated);
   Category = categorize(SemaCCResult);
 
   if (SemaCCResult.Declaration) {
@@ -180,6 +178,7 @@
 }
 
 void SymbolQualitySignals::merge(const Symbol ) {
+  Deprecated |= IndexResult.Deprecated;
   References = std::max(IndexResult.References, References);
   Category = categorize(IndexResult.SymInfo);
   ReservedName = ReservedName || isReserved(IndexResult.Name);
Index: clangd/Protocol.h
===
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -768,6 +768,9 @@
   /// themselves.
   std::vector additionalTextEdits;
 
+  /// Indicates if this item is deprecated.
+  bool deprecated = false;
+
   // TODO(krasimir): The following optional fields defined by the language
   // server protocol are unsupported:
   //
Index: clangd/Protocol.cpp
===
--- clangd/Protocol.cpp
+++ clangd/Protocol.cpp
@@ -517,6 +517,8 @@
 Result["textEdit"] = *CI.textEdit;
   if (!CI.additionalTextEdits.empty())
 Result["additionalTextEdits"] =