[PATCH] D90116: [clangd] Escape Unicode characters to fix Windows builds

2020-10-25 Thread Daniel Martín via Phabricator via cfe-commits
danielmartin added a comment.

Using explicit UTF-8 string literals is a possible solution, but it makes the 
code a bit less readable. Another possible solution is to save the source file 
using UTF-8 with BOM, but this is confusing outside the Microsoft world (and 
it's very easy to remove the BOM by mistake).

I think passing `/utf-8` to MSVC is the best solution for good interoperability 
with Clang.

> I am not sure if there's a way to change that for clang/gcc. I I believe they 
> both require plain ascii or utf-8 anyways.

Clang enforces UTF-8 everywhere so there's no need for additional 
configuration. Clang can also accept source files encoded in UTF-8 with BOM. 
I'm not sure about GCC, I think you need to enforce the encoding manually like 
in MSVC (see the `-finput-charset` and `-fexec-charset` options).

I recommend reading this article about how MSVC interprets the encoding of 
source files, the casuistic is a bit complex: 
https://devblogs.microsoft.com/cppblog/new-options-for-managing-character-sets-in-the-microsoft-cc-compiler/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90116

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


[PATCH] D88881: [clangd] Add a NewName optional parameter to clangdServer::prepareRename.

2020-10-07 Thread Daniel Martín via Phabricator via cfe-commits
danielmartin added inline comments.



Comment at: clang-tools-extra/clangd/ClangdServer.h:277
+  ///
+  /// If NewName is provided, it peforms a name validation.
   void prepareRename(PathRef File, Position Pos,




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D1

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


[PATCH] D80472: [clangd] Add access specifier information to hover contents

2020-05-27 Thread Daniel Martín via Phabricator via cfe-commits
danielmartin updated this revision to Diff 266540.
danielmartin added a comment.

Rebase and squash


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80472

Files:
  clang-tools-extra/clang-doc/Generators.cpp
  clang-tools-extra/clang-doc/Generators.h
  clang-tools-extra/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/clang-doc/MDGenerator.cpp
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/Hover.h
  clang-tools-extra/clangd/unittests/HoverTests.cpp
  clang/include/clang/Basic/Specifiers.h
  clang/lib/AST/DeclPrinter.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp

Index: clang/lib/AST/TextNodeDumper.cpp
===
--- clang/lib/AST/TextNodeDumper.cpp
+++ clang/lib/AST/TextNodeDumper.cpp
@@ -17,6 +17,7 @@
 #include "clang/AST/LocInfoType.h"
 #include "clang/Basic/Module.h"
 #include "clang/Basic/SourceManager.h"
+#include "clang/Basic/Specifiers.h"
 
 using namespace clang;
 
@@ -436,19 +437,10 @@
 }
 
 void TextNodeDumper::dumpAccessSpecifier(AccessSpecifier AS) {
-  switch (AS) {
-  case AS_none:
-break;
-  case AS_public:
-OS << "public";
-break;
-  case AS_protected:
-OS << "protected";
-break;
-  case AS_private:
-OS << "private";
-break;
-  }
+  const auto AccessSpelling = getAccessSpelling(AS);
+  if (AccessSpelling.empty())
+return;
+  OS << AccessSpelling;
 }
 
 void TextNodeDumper::dumpCleanupObject(
Index: clang/lib/AST/JSONNodeDumper.cpp
===
--- clang/lib/AST/JSONNodeDumper.cpp
+++ clang/lib/AST/JSONNodeDumper.cpp
@@ -1,5 +1,6 @@
 #include "clang/AST/JSONNodeDumper.h"
 #include "clang/Basic/SourceManager.h"
+#include "clang/Basic/Specifiers.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/ADT/StringSwitch.h"
 
@@ -465,13 +466,10 @@
 #undef FIELD2
 
 std::string JSONNodeDumper::createAccessSpecifier(AccessSpecifier AS) {
-  switch (AS) {
-  case AS_none: return "none";
-  case AS_private: return "private";
-  case AS_protected: return "protected";
-  case AS_public: return "public";
-  }
-  llvm_unreachable("Unknown access specifier");
+  const auto AccessSpelling = getAccessSpelling(AS);
+  if (AccessSpelling.empty())
+return "none";
+  return AccessSpelling.str();
 }
 
 llvm::json::Object
Index: clang/lib/AST/DeclPrinter.cpp
===
--- clang/lib/AST/DeclPrinter.cpp
+++ clang/lib/AST/DeclPrinter.cpp
@@ -289,12 +289,10 @@
 }
 
 void DeclPrinter::Print(AccessSpecifier AS) {
-  switch(AS) {
-  case AS_none:  llvm_unreachable("No access specifier!");
-  case AS_public:Out << "public"; break;
-  case AS_protected: Out << "protected"; break;
-  case AS_private:   Out << "private"; break;
-  }
+  const auto AccessSpelling = getAccessSpelling(AS);
+  if (AccessSpelling.empty())
+llvm_unreachable("No access specifier!");
+  Out << AccessSpelling;
 }
 
 void DeclPrinter::PrintConstructorInitializers(CXXConstructorDecl *CDecl,
Index: clang/include/clang/Basic/Specifiers.h
===
--- clang/include/clang/Basic/Specifiers.h
+++ clang/include/clang/Basic/Specifiers.h
@@ -365,6 +365,20 @@
   };
 
   llvm::StringRef getParameterABISpelling(ParameterABI kind);
+
+  inline llvm::StringRef getAccessSpelling(AccessSpecifier AS) {
+switch (AS) {
+case AccessSpecifier::AS_public:
+  return "public";
+case AccessSpecifier::AS_protected:
+  return "protected";
+case AccessSpecifier::AS_private:
+  return "private";
+case AccessSpecifier::AS_none:
+  return {};
+}
+llvm_unreachable("Unknown AccessSpecifier");
+  }
 } // end namespace clang
 
 #endif // LLVM_CLANG_BASIC_SPECIFIERS_H
Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -12,6 +12,7 @@
 #include "TestIndex.h"
 #include "TestTU.h"
 #include "index/MemIndex.h"
+#include "clang/Basic/Specifiers.h"
 #include "clang/Index/IndexSymbol.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/StringRef.h"
@@ -79,6 +80,7 @@
  HI.Type = "char";
  HI.Offset = 0;
  HI.Size = 1;
+ HI.AccessSpecifier = "public";
}},
   // Local to class method.
   {R"cpp(
@@ -115,6 +117,7 @@
  HI.Type = "char";
  HI.Offset = 0;
  HI.Size = 1;
+ HI.AccessSpecifier = "public";
}},
   // Struct definition shows size.
   {R"cpp(
@@ -344,6 +347,7 @@
  HI.Kind = index::SymbolKind::Constructor;
  HI.Definition = "X()";
  HI.Parameters.emplace();
+ HI.AccessSpecifier = "public";
}},
   {"class X { [[^~]]X(); };", // FIXME: Should be 

[PATCH] D80472: [clangd] Add access specifier information to hover contents

2020-05-26 Thread Daniel Martín via Phabricator via cfe-commits
danielmartin added a comment.

Thanks for the review! I don't have commit access so I'd need someone to land 
this patch for me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80472



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


[PATCH] D80472: [clangd] Add access specifier information to hover contents

2020-05-26 Thread Daniel Martín via Phabricator via cfe-commits
danielmartin updated this revision to Diff 266322.
danielmartin marked 2 inline comments as done.
danielmartin added a comment.

Address feedback

Rename getAccess to getAccessSpelling.
Replace more parts of the codebase that were using their own version of 
getAccessSpelling.
Use StringRef's str() method instead of explicit std::string constructor.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80472

Files:
  clang-tools-extra/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/clang-doc/MDGenerator.cpp
  clang-tools-extra/clangd/Hover.cpp
  clang/include/clang/Basic/Specifiers.h
  clang/lib/AST/DeclPrinter.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp

Index: clang/lib/AST/TextNodeDumper.cpp
===
--- clang/lib/AST/TextNodeDumper.cpp
+++ clang/lib/AST/TextNodeDumper.cpp
@@ -17,6 +17,7 @@
 #include "clang/AST/LocInfoType.h"
 #include "clang/Basic/Module.h"
 #include "clang/Basic/SourceManager.h"
+#include "clang/Basic/Specifiers.h"
 
 using namespace clang;
 
@@ -436,19 +437,10 @@
 }
 
 void TextNodeDumper::dumpAccessSpecifier(AccessSpecifier AS) {
-  switch (AS) {
-  case AS_none:
-break;
-  case AS_public:
-OS << "public";
-break;
-  case AS_protected:
-OS << "protected";
-break;
-  case AS_private:
-OS << "private";
-break;
-  }
+  const auto AccessSpelling = getAccessSpelling(AS);
+  if (AccessSpelling.empty())
+return;
+  OS << AccessSpelling;
 }
 
 void TextNodeDumper::dumpCleanupObject(
Index: clang/lib/AST/JSONNodeDumper.cpp
===
--- clang/lib/AST/JSONNodeDumper.cpp
+++ clang/lib/AST/JSONNodeDumper.cpp
@@ -1,5 +1,6 @@
 #include "clang/AST/JSONNodeDumper.h"
 #include "clang/Basic/SourceManager.h"
+#include "clang/Basic/Specifiers.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/ADT/StringSwitch.h"
 
@@ -465,13 +466,10 @@
 #undef FIELD2
 
 std::string JSONNodeDumper::createAccessSpecifier(AccessSpecifier AS) {
-  switch (AS) {
-  case AS_none: return "none";
-  case AS_private: return "private";
-  case AS_protected: return "protected";
-  case AS_public: return "public";
-  }
-  llvm_unreachable("Unknown access specifier");
+  const auto AccessSpelling = getAccessSpelling(AS);
+  if (AccessSpelling.empty())
+return "none";
+  return AccessSpelling.str();
 }
 
 llvm::json::Object
Index: clang/lib/AST/DeclPrinter.cpp
===
--- clang/lib/AST/DeclPrinter.cpp
+++ clang/lib/AST/DeclPrinter.cpp
@@ -289,12 +289,10 @@
 }
 
 void DeclPrinter::Print(AccessSpecifier AS) {
-  switch(AS) {
-  case AS_none:  llvm_unreachable("No access specifier!");
-  case AS_public:Out << "public"; break;
-  case AS_protected: Out << "protected"; break;
-  case AS_private:   Out << "private"; break;
-  }
+  const auto AccessSpelling = getAccessSpelling(AS);
+  if (AccessSpelling.empty())
+llvm_unreachable("No access specifier!");
+  Out << AccessSpelling;
 }
 
 void DeclPrinter::PrintConstructorInitializers(CXXConstructorDecl *CDecl,
Index: clang/include/clang/Basic/Specifiers.h
===
--- clang/include/clang/Basic/Specifiers.h
+++ clang/include/clang/Basic/Specifiers.h
@@ -366,7 +366,7 @@
 
   llvm::StringRef getParameterABISpelling(ParameterABI kind);
 
-  inline llvm::StringRef getAccess(AccessSpecifier AS) {
+  inline llvm::StringRef getAccessSpelling(AccessSpecifier AS) {
 switch (AS) {
 case AccessSpecifier::AS_public:
   return "public";
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -468,7 +468,7 @@
   HoverInfo HI;
   const ASTContext  = D->getASTContext();
 
-  HI.AccessSpecifier = std::string(getAccess(D->getAccess()));
+  HI.AccessSpecifier = getAccessSpelling(D->getAccess()).str();
   HI.NamespaceScope = getNamespaceScope(D);
   if (!HI.NamespaceScope->empty())
 HI.NamespaceScope->append("::");
Index: clang-tools-extra/clang-doc/MDGenerator.cpp
===
--- clang-tools-extra/clang-doc/MDGenerator.cpp
+++ clang-tools-extra/clang-doc/MDGenerator.cpp
@@ -157,7 +157,7 @@
 First = false;
   }
   writeHeader(I.Name, 3, OS);
-  std::string Access = std::string(getAccess(I.Access));
+  std::string Access = getAccessSpelling(I.Access).str();
   if (Access != "")
 writeLine(genItalic(Access + " " + I.ReturnType.Type.Name + " " + I.Name +
 "(" + Stream.str() + ")"),
@@ -250,7 +250,7 @@
   if (!I.Members.empty()) {
 writeHeader("Members", 2, OS);
 for (const auto  : I.Members) {
-  std::string Access = std::string(getAccess(Member.Access));
+  std::string Access = 

[PATCH] D80472: [clangd] Add access specifier information to hover contents

2020-05-25 Thread Daniel Martín via Phabricator via cfe-commits
danielmartin marked 6 inline comments as done.
danielmartin added inline comments.



Comment at: clang-tools-extra/clangd/Hover.cpp:680
 
+StringRef getAccessString(AccessSpecifier AS) {
+  switch (AS) {

kadircet wrote:
> it is annoying to have this function duplicated in each component (there are 
> duplicates at least in text and json node dumpers too) :/
> 
> Feel free to provide a common implementation in 
> `clang/include/clang/Basic/Specifiers.h` and migrate all other usages to it, 
> or just put a FIXME in here saying we should converge those.
> 
> nit: prefer `llvm::StringRef` as return type
I've only found usages in clang-doc, I don't know if there's more. 

I've changed them to use the new common logic in `Specifiers.h`.



Comment at: clang-tools-extra/clangd/Hover.cpp:793
+  if (AccessSpecifier != AccessSpecifier::AS_none)
+Header.appendText(getAccessString(AccessSpecifier)).appendSpace();
   if (Kind != index::SymbolKind::Unknown)

kadircet wrote:
> I wonder if it would be more natural to put this at bottom, where we list the 
> containing class/struct/union. e.g.
> 
> ```
> struct X { int fo^o; }
> ```
> 
> would result in
> 
> ```
> field foo
> ---
> 
> // In X
> public: int foo
> ```
> 
> I find current one useful too, just listing options to see what you (and 
> possibly others interested in) think.
I think it's a bit less visible, but one good thing your suggestion has is that 
in the Emacs client we use that part of the hover content to show a one liner 
with type information. So I followed your suggestion to also have access 
specifier information in our one-liner.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80472



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


[PATCH] D80472: [clangd] Add access specifier information to hover contents

2020-05-25 Thread Daniel Martín via Phabricator via cfe-commits
danielmartin updated this revision to Diff 265942.
danielmartin added a comment.

Move clang::getAccess to Specifiers.h and refactor logic in clang-doc
to use that function instead of its own.

Also changes where "public", "private" etc. is shown in the hover
contents. Now it's shown at the bottom.

Strengthens an existing unit test to also check for access specifiers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80472

Files:
  clang-tools-extra/clang-doc/Generators.cpp
  clang-tools-extra/clang-doc/Generators.h
  clang-tools-extra/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/clang-doc/MDGenerator.cpp
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/Hover.h
  clang-tools-extra/clangd/unittests/HoverTests.cpp
  clang/include/clang/Basic/Specifiers.h

Index: clang/include/clang/Basic/Specifiers.h
===
--- clang/include/clang/Basic/Specifiers.h
+++ clang/include/clang/Basic/Specifiers.h
@@ -365,6 +365,20 @@
   };
 
   llvm::StringRef getParameterABISpelling(ParameterABI kind);
+
+  inline llvm::StringRef getAccess(AccessSpecifier AS) {
+switch (AS) {
+case AccessSpecifier::AS_public:
+  return "public";
+case AccessSpecifier::AS_protected:
+  return "protected";
+case AccessSpecifier::AS_private:
+  return "private";
+case AccessSpecifier::AS_none:
+  return {};
+}
+llvm_unreachable("Unknown AccessSpecifier");
+  }
 } // end namespace clang
 
 #endif // LLVM_CLANG_BASIC_SPECIFIERS_H
Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -80,6 +80,7 @@
  HI.Type = "char";
  HI.Offset = 0;
  HI.Size = 1;
+ HI.AccessSpecifier = "public";
}},
   // Local to class method.
   {R"cpp(
@@ -116,6 +117,7 @@
  HI.Type = "char";
  HI.Offset = 0;
  HI.Size = 1;
+ HI.AccessSpecifier = "public";
}},
   // Struct definition shows size.
   {R"cpp(
@@ -345,6 +347,7 @@
  HI.Kind = index::SymbolKind::Constructor;
  HI.Definition = "X()";
  HI.Parameters.emplace();
+ HI.AccessSpecifier = "public";
}},
   {"class X { [[^~]]X(); };", // FIXME: Should be [[~X]]()
[](HoverInfo ) {
@@ -354,6 +357,7 @@
  HI.Kind = index::SymbolKind::Destructor;
  HI.Definition = "~X()";
  HI.Parameters.emplace();
+ HI.AccessSpecifier = "private";
}},
   {"class X { [[op^erator]] int(); };",
[](HoverInfo ) {
@@ -363,6 +367,7 @@
  HI.Kind = index::SymbolKind::ConversionFunction;
  HI.Definition = "operator int()";
  HI.Parameters.emplace();
+ HI.AccessSpecifier = "private";
}},
   {"class X { operator [[^X]](); };",
[](HoverInfo ) {
@@ -495,6 +500,7 @@
  HI.NamespaceScope = "";
  HI.LocalScope = "Add<1, 2>::";
  HI.Value = "3";
+ HI.AccessSpecifier = "public";
}},
   {R"cpp(
 constexpr int answer() { return 40 + 2; }
@@ -607,6 +613,7 @@
  HI.Definition = "typename T = int";
  HI.LocalScope = "foo::";
  HI.Type = "typename";
+ HI.AccessSpecifier = "public";
}},
   {// TemplateTemplate Type Parameter
R"cpp(
@@ -619,6 +626,7 @@
  HI.Definition = "template  class T";
  HI.LocalScope = "foo::";
  HI.Type = "template  class";
+ HI.AccessSpecifier = "public";
}},
   {// NonType Template Parameter
R"cpp(
@@ -631,6 +639,7 @@
  HI.Definition = "int T = 5";
  HI.LocalScope = "foo::";
  HI.Type = "int";
+ HI.AccessSpecifier = "public";
}},
 
   {// Getter
@@ -647,6 +656,7 @@
  HI.Type = "float ()";
  HI.ReturnType = "float";
  HI.Parameters.emplace();
+ HI.AccessSpecifier = "public";
}},
   {// Setter
R"cpp(
@@ -665,6 +675,7 @@
  HI.Parameters->emplace_back();
  HI.Parameters->back().Type = "float";
  HI.Parameters->back().Name = "v";
+ HI.AccessSpecifier = "public";
}},
   {// Setter (builder)
R"cpp(
@@ -683,6 +694,7 @@
  HI.Parameters->emplace_back();
  HI.Parameters->back().Type = "float";
  HI.Parameters->back().Name = "v";
+ HI.AccessSpecifier = "public";
}},
   };
   for (const auto  : Cases) {
@@ -716,6 +728,7 @@
 EXPECT_EQ(H->Value, Expected.Value);
 EXPECT_EQ(H->Size, Expected.Size);
 EXPECT_EQ(H->Offset, Expected.Offset);
+EXPECT_EQ(H->AccessSpecifier, Expected.AccessSpecifier);
   }
 }
 
@@ -1968,20 +1981,20 @@
   {
   [](HoverInfo ) {
 

[PATCH] D80472: [clangd] Add access specifier information to hover contents

2020-05-25 Thread Daniel Martín via Phabricator via cfe-commits
danielmartin created this revision.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous, 
MaskRay, ilya-biryukov.
Herald added a project: clang.
danielmartin edited the summary of this revision.

For https://github.com/clangd/clangd/issues/382

This commit adds access specifier information to the hover
contents. For example, the hover information of a class field or
member function will now indicate if the field or member is private,
public, or protected. This can be particularly useful when a developer
is in the implementation file and wants to know if a particular member
definition is public or private.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80472

Files:
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/Hover.h
  clang-tools-extra/clangd/unittests/HoverTests.cpp

Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -12,6 +12,7 @@
 #include "TestIndex.h"
 #include "TestTU.h"
 #include "index/MemIndex.h"
+#include "clang/Basic/Specifiers.h"
 #include "clang/Index/IndexSymbol.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/StringRef.h"
@@ -1964,7 +1965,51 @@
 // In test::Bar
 def)",
   },
-  };
+  {
+  [](HoverInfo ) {
+HI.Kind = index::SymbolKind::Field;
+HI.AccessSpecifier = AccessSpecifier::AS_public;
+HI.Name = "foo";
+HI.LocalScope = "test::Bar::";
+HI.Definition = "def";
+  },
+  R"(public field foo
+
+// In test::Bar
+def)",
+  },
+  {
+  [](HoverInfo ) {
+HI.Definition = "int method()";
+HI.AccessSpecifier = AccessSpecifier::AS_protected;
+HI.Kind = index::SymbolKind::InstanceMethod;
+HI.NamespaceScope = "";
+HI.LocalScope = "cls::";
+HI.Name = "method";
+HI.Parameters.emplace();
+HI.ReturnType = "int";
+HI.Type = "int ()";
+  },
+  R"(protected instance-method method
+
+→ int
+
+// In cls
+int method())",
+  },
+  {
+  [](HoverInfo ) {
+HI.Kind = index::SymbolKind::Union;
+HI.AccessSpecifier = AccessSpecifier::AS_private;
+HI.Name = "foo";
+HI.NamespaceScope = "ns1::";
+HI.Definition = "union foo {}";
+  },
+  R"(private union foo
+
+// In namespace ns1
+union foo {})",
+  }};
 
   for (const auto  : Cases) {
 HoverInfo HI;
Index: clang-tools-extra/clangd/Hover.h
===
--- clang-tools-extra/clangd/Hover.h
+++ clang-tools-extra/clangd/Hover.h
@@ -59,6 +59,8 @@
   /// Source code containing the definition of the symbol.
   std::string Definition;
 
+  /// Access specifier. Applies to members of class/structs or unions.
+  AccessSpecifier AccessSpecifier = AccessSpecifier::AS_none;
   /// Pretty-printed variable type.
   /// Set only for variables.
   llvm::Optional Type;
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -468,6 +468,7 @@
   HoverInfo HI;
   const ASTContext  = D->getASTContext();
 
+  HI.AccessSpecifier = D->getAccess();
   HI.NamespaceScope = getNamespaceScope(D);
   if (!HI.NamespaceScope->empty())
 HI.NamespaceScope->append("::");
@@ -676,6 +677,20 @@
   }
 }
 
+StringRef getAccessString(AccessSpecifier AS) {
+  switch (AS) {
+  case AccessSpecifier::AS_public:
+return "public";
+  case AccessSpecifier::AS_protected:
+return "protected";
+  case AccessSpecifier::AS_private:
+return "private";
+  case AccessSpecifier::AS_none:
+return {};
+  }
+  llvm_unreachable("Unknown AccessSpecifier");
+}
+
 } // namespace
 
 llvm::Optional getHover(ParsedAST , Position Pos,
@@ -774,6 +789,8 @@
   // level 1 and 2 headers in a huge font, see
   // https://github.com/microsoft/vscode/issues/88417 for details.
   markup::Paragraph  = Output.addHeading(3);
+  if (AccessSpecifier != AccessSpecifier::AS_none)
+Header.appendText(getAccessString(AccessSpecifier)).appendSpace();
   if (Kind != index::SymbolKind::Unknown)
 Header.appendText(index::getSymbolKindString(Kind)).appendSpace();
   assert(!Name.empty() && "hover triggered on a nameless symbol");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D37192: [clang-format] Add support for generic Obj-C categories

2017-08-28 Thread Daniel Martín via Phabricator via cfe-commits
danielmartin updated this revision to Diff 112855.
danielmartin added a comment.

Make comment fit in one line


https://reviews.llvm.org/D37192

Files:
  lib/Format/UnwrappedLineParser.cpp
  unittests/Format/FormatTestObjC.cpp


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -821,6 +821,13 @@
"  NSBundle.mainBundle.infoDictionary[@\"a\"]\n"
"]];");
 }
+
+TEST_F(FormatTestObjC, FormatGenericObjCCategory) {
+  verifyFormat(
+  "@interface NSHashTable (MYFoundation)\n"
+  "- (void)xyz_addObjectsFromArray:(nonnull NSArray *)array;\n"
+  "@end");
+}
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -2096,6 +2096,10 @@
   if (FormatTok->Tok.is(tok::less))
 parseObjCProtocolList();
 
+  // After a protocol list, we can have a category (Obj-C generic category).
+  if (FormatTok->Tok.is(tok::l_paren))
+parseParens();
+
   if (FormatTok->Tok.is(tok::l_brace)) {
 if (Style.BraceWrapping.AfterObjCDeclaration)
   addUnwrappedLine();


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -821,6 +821,13 @@
"  NSBundle.mainBundle.infoDictionary[@\"a\"]\n"
"]];");
 }
+
+TEST_F(FormatTestObjC, FormatGenericObjCCategory) {
+  verifyFormat(
+  "@interface NSHashTable (MYFoundation)\n"
+  "- (void)xyz_addObjectsFromArray:(nonnull NSArray *)array;\n"
+  "@end");
+}
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -2096,6 +2096,10 @@
   if (FormatTok->Tok.is(tok::less))
 parseObjCProtocolList();
 
+  // After a protocol list, we can have a category (Obj-C generic category).
+  if (FormatTok->Tok.is(tok::l_paren))
+parseParens();
+
   if (FormatTok->Tok.is(tok::l_brace)) {
 if (Style.BraceWrapping.AfterObjCDeclaration)
   addUnwrappedLine();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D37192: [clang-format] Add support for generic Obj-C categories

2017-08-27 Thread Daniel Martín via Phabricator via cfe-commits
danielmartin created this revision.
Herald added a subscriber: klimek.

Objective C supports lightweight generics in categories. This patch
adds support for that in clang-format.


https://reviews.llvm.org/D37192

Files:
  lib/Format/UnwrappedLineParser.cpp
  unittests/Format/FormatTestObjC.cpp


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -821,6 +821,13 @@
"  NSBundle.mainBundle.infoDictionary[@\"a\"]\n"
"]];");
 }
+
+TEST_F(FormatTestObjC, FormatGenericObjCCategory) {
+  verifyFormat(
+  "@interface NSHashTable (MYFoundation)\n"
+  "- (void)xyz_addObjectsFromArray:(nonnull NSArray *)array;\n"
+  "@end");
+}
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -2096,6 +2096,11 @@
   if (FormatTok->Tok.is(tok::less))
 parseObjCProtocolList();
 
+  // After a protocol list, we can have a category (Obj-C generic
+  // category).
+  if (FormatTok->Tok.is(tok::l_paren))
+parseParens();
+
   if (FormatTok->Tok.is(tok::l_brace)) {
 if (Style.BraceWrapping.AfterObjCDeclaration)
   addUnwrappedLine();


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -821,6 +821,13 @@
"  NSBundle.mainBundle.infoDictionary[@\"a\"]\n"
"]];");
 }
+
+TEST_F(FormatTestObjC, FormatGenericObjCCategory) {
+  verifyFormat(
+  "@interface NSHashTable (MYFoundation)\n"
+  "- (void)xyz_addObjectsFromArray:(nonnull NSArray *)array;\n"
+  "@end");
+}
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -2096,6 +2096,11 @@
   if (FormatTok->Tok.is(tok::less))
 parseObjCProtocolList();
 
+  // After a protocol list, we can have a category (Obj-C generic
+  // category).
+  if (FormatTok->Tok.is(tok::l_paren))
+parseParens();
+
   if (FormatTok->Tok.is(tok::l_brace)) {
 if (Style.BraceWrapping.AfterObjCDeclaration)
   addUnwrappedLine();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits