[PATCH] D137494: [Clangd] Fix the code action `RemoveUsingNamespace`

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry created this revision.
v1nh1shungry added reviewers: thesamesam, tom-anders, junaire.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
v1nh1shungry requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

Avoid adding qualifiers before C++ operators declared in a non-class context


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D137494

Files:
  clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp


Index: clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
@@ -155,6 +155,13 @@
 if (!visibleContext(T->getDeclContext())
  ->Equals(TargetDirective->getNominatedNamespace()))
   return;
+// Avoid adding qualifiers before operators, e.g.
+//   using namespace std;
+//   cout << "foo"; // Must not changed to std::cout std:: << "foo"
+// FIXME: Operators declared in a class context are not handled
+if (T->isInIdentifierNamespace(
+Decl::IdentifierNamespace::IDNS_NonMemberOperator))
+  return;
   }
   SourceLocation Loc = Ref.NameLoc;
   if (Loc.isMacroID()) {


Index: clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
@@ -155,6 +155,13 @@
 if (!visibleContext(T->getDeclContext())
  ->Equals(TargetDirective->getNominatedNamespace()))
   return;
+// Avoid adding qualifiers before operators, e.g.
+//   using namespace std;
+//   cout << "foo"; // Must not changed to std::cout std:: << "foo"
+// FIXME: Operators declared in a class context are not handled
+if (T->isInIdentifierNamespace(
+Decl::IdentifierNamespace::IDNS_NonMemberOperator))
+  return;
   }
   SourceLocation Loc = Ref.NameLoc;
   if (Loc.isMacroID()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D137494: [Clangd] Fix the code action `RemoveUsingNamespace`

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry updated this revision to Diff 473475.
v1nh1shungry added a comment.

Add a test and rewrite the comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137494

Files:
  clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
  clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp


Index: clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
@@ -226,7 +226,30 @@
   int main() {
 std::vector V;
   }
-)cpp"}};
+)cpp"},
+  {// Does not qualify operators
+   R"cpp(
+  namespace ns {
+  struct Foo {};
+  void operator+(const Foo &, int) {}
+  }
+  using namespace ns;
+  int main() {
+Foo foo;
+foo + 10;
+  }
+  )cpp",
+   R"cpp(
+  namespace ns {
+  struct Foo {};
+  void operator+(const Foo &, int) {}
+  }
+
+  int main() {
+ns::Foo foo;
+foo + 10;
+  }
+  )cpp"}};
   for (auto C : Cases)
 EXPECT_EQ(C.second, apply(C.first)) << C.first;
 }
Index: clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
@@ -155,6 +155,13 @@
 if (!visibleContext(T->getDeclContext())
  ->Equals(TargetDirective->getNominatedNamespace()))
   return;
+// Avoid adding qualifiers before operators, e.g.
+//   using namespace std;
+//   cout << "foo"; // Must not changed to std::cout std:: << "foo"
+// FIXME: User-defined literals are not handled
+if (T->isInIdentifierNamespace(
+Decl::IdentifierNamespace::IDNS_NonMemberOperator))
+  return;
   }
   SourceLocation Loc = Ref.NameLoc;
   if (Loc.isMacroID()) {


Index: clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
@@ -226,7 +226,30 @@
   int main() {
 std::vector V;
   }
-)cpp"}};
+)cpp"},
+  {// Does not qualify operators
+   R"cpp(
+  namespace ns {
+  struct Foo {};
+  void operator+(const Foo &, int) {}
+  }
+  using namespace ns;
+  int main() {
+Foo foo;
+foo + 10;
+  }
+  )cpp",
+   R"cpp(
+  namespace ns {
+  struct Foo {};
+  void operator+(const Foo &, int) {}
+  }
+
+  int main() {
+ns::Foo foo;
+foo + 10;
+  }
+  )cpp"}};
   for (auto C : Cases)
 EXPECT_EQ(C.second, apply(C.first)) << C.first;
 }
Index: clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
@@ -155,6 +155,13 @@
 if (!visibleContext(T->getDeclContext())
  ->Equals(TargetDirective->getNominatedNamespace()))
   return;
+// Avoid adding qualifiers before operators, e.g.
+//   using namespace std;
+//   cout << "foo"; // Must not changed to std::cout std:: << "foo"
+// FIXME: User-defined literals are not handled
+if (T->isInIdentifierNamespace(
+Decl::IdentifierNamespace::IDNS_NonMemberOperator))
+  return;
   }
   SourceLocation Loc = Ref.NameLoc;
   if (Loc.isMacroID()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D137494: [Clangd] Fix the code action `RemoveUsingNamespace`

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry updated this revision to Diff 473477.
v1nh1shungry added a comment.

Correct the test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137494

Files:
  clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
  clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp


Index: clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
@@ -226,7 +226,30 @@
   int main() {
 std::vector V;
   }
-)cpp"}};
+)cpp"},
+  {// Does not qualify operators declared in a non-class context
+   R"cpp(
+  namespace ns {
+  struct Foo {};
+  void operator+(const Foo &, int) {}
+  }
+  using namespace n^s;
+  int main() {
+Foo foo;
+foo + 10;
+  }
+  )cpp",
+   R"cpp(
+  namespace ns {
+  struct Foo {};
+  void operator+(const Foo &, int) {}
+  }
+
+  int main() {
+ns::Foo foo;
+foo + 10;
+  }
+  )cpp"}};
   for (auto C : Cases)
 EXPECT_EQ(C.second, apply(C.first)) << C.first;
 }
Index: clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
@@ -155,6 +155,13 @@
 if (!visibleContext(T->getDeclContext())
  ->Equals(TargetDirective->getNominatedNamespace()))
   return;
+// Avoid adding qualifiers before operators, e.g.
+//   using namespace std;
+//   cout << "foo"; // Must not changed to std::cout std:: << "foo"
+// FIXME: User-defined literals are not handled
+if (T->isInIdentifierNamespace(
+Decl::IdentifierNamespace::IDNS_NonMemberOperator))
+  return;
   }
   SourceLocation Loc = Ref.NameLoc;
   if (Loc.isMacroID()) {


Index: clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
@@ -226,7 +226,30 @@
   int main() {
 std::vector V;
   }
-)cpp"}};
+)cpp"},
+  {// Does not qualify operators declared in a non-class context
+   R"cpp(
+  namespace ns {
+  struct Foo {};
+  void operator+(const Foo &, int) {}
+  }
+  using namespace n^s;
+  int main() {
+Foo foo;
+foo + 10;
+  }
+  )cpp",
+   R"cpp(
+  namespace ns {
+  struct Foo {};
+  void operator+(const Foo &, int) {}
+  }
+
+  int main() {
+ns::Foo foo;
+foo + 10;
+  }
+  )cpp"}};
   for (auto C : Cases)
 EXPECT_EQ(C.second, apply(C.first)) << C.first;
 }
Index: clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
@@ -155,6 +155,13 @@
 if (!visibleContext(T->getDeclContext())
  ->Equals(TargetDirective->getNominatedNamespace()))
   return;
+// Avoid adding qualifiers before operators, e.g.
+//   using namespace std;
+//   cout << "foo"; // Must not changed to std::cout std:: << "foo"
+// FIXME: User-defined literals are not handled
+if (T->isInIdentifierNamespace(
+Decl::IdentifierNamespace::IDNS_NonMemberOperator))
+  return;
   }
   SourceLocation Loc = Ref.NameLoc;
   if (Loc.isMacroID()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D137494: [Clangd] Fix the code action `RemoveUsingNamespace`

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry added a comment.

@tom-anders Thank you for reviewing!

I tried adding a test but I don't know how to run the tests.

And I just found that operators declared in a class context are already handled 
in the existing version, but user-defined literals are not, so I rewrote the 
comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137494

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

[PATCH] D137494: [Clangd] Fix the code action `RemoveUsingNamespace`

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry updated this revision to Diff 473478.
v1nh1shungry added a comment.

Correct the test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137494

Files:
  clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
  clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp


Index: clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
@@ -226,7 +226,30 @@
   int main() {
 std::vector V;
   }
-)cpp"}};
+)cpp"},
+  {// Does not qualify operators declared in a non-class context
+   R"cpp(
+  namespace ns {
+  struct Foo {};
+  void operator+(const Foo &, int) {}
+  }
+  using namespace n^s;
+  int main() {
+Foo foo;
+foo + 10;
+  }
+  )cpp",
+   R"cpp(
+  namespace ns {
+  struct Foo {};
+  void operator+(const Foo &, int) {}
+  }
+  
+  int main() {
+ns::Foo foo;
+foo + 10;
+  }
+  )cpp"}};
   for (auto C : Cases)
 EXPECT_EQ(C.second, apply(C.first)) << C.first;
 }
Index: clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
@@ -155,6 +155,13 @@
 if (!visibleContext(T->getDeclContext())
  ->Equals(TargetDirective->getNominatedNamespace()))
   return;
+// Avoid adding qualifiers before operators, e.g.
+//   using namespace std;
+//   cout << "foo"; // Must not changed to std::cout std:: << "foo"
+// FIXME: User-defined literals are not handled
+if (T->isInIdentifierNamespace(
+Decl::IdentifierNamespace::IDNS_NonMemberOperator))
+  return;
   }
   SourceLocation Loc = Ref.NameLoc;
   if (Loc.isMacroID()) {


Index: clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
@@ -226,7 +226,30 @@
   int main() {
 std::vector V;
   }
-)cpp"}};
+)cpp"},
+  {// Does not qualify operators declared in a non-class context
+   R"cpp(
+  namespace ns {
+  struct Foo {};
+  void operator+(const Foo &, int) {}
+  }
+  using namespace n^s;
+  int main() {
+Foo foo;
+foo + 10;
+  }
+  )cpp",
+   R"cpp(
+  namespace ns {
+  struct Foo {};
+  void operator+(const Foo &, int) {}
+  }
+  
+  int main() {
+ns::Foo foo;
+foo + 10;
+  }
+  )cpp"}};
   for (auto C : Cases)
 EXPECT_EQ(C.second, apply(C.first)) << C.first;
 }
Index: clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
@@ -155,6 +155,13 @@
 if (!visibleContext(T->getDeclContext())
  ->Equals(TargetDirective->getNominatedNamespace()))
   return;
+// Avoid adding qualifiers before operators, e.g.
+//   using namespace std;
+//   cout << "foo"; // Must not changed to std::cout std:: << "foo"
+// FIXME: User-defined literals are not handled
+if (T->isInIdentifierNamespace(
+Decl::IdentifierNamespace::IDNS_NonMemberOperator))
+  return;
   }
   SourceLocation Loc = Ref.NameLoc;
   if (Loc.isMacroID()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D137494: [Clangd] Fix the code action `RemoveUsingNamespace`

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry added a comment.

In D137494#3910500 , @v1nh1shungry 
wrote:

> @tom-anders Thank you for reviewing!
>
> I tried adding a test but I don't know how to run the tests.
>
> And I just found that operators declared in a class context are already 
> handled in the existing version, but user-defined literals are not, so I 
> rewrote the comments.

Sorry, I found tips about running tests in the document. Feel like a fool :)

Please check this test is correct and suitable. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137494

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

[PATCH] D137494: [Clangd] Fix the code action `RemoveUsingNamespace`

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry added inline comments.



Comment at: 
clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp:236
+  }
+  using namespace n^s;
+  int main() {

junaire wrote:
> is this a typo?
I guess not. I refer to the tests above and I think this represents the cursor. 
But I'm not sure though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137494

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

[PATCH] D137494: [Clangd] Fix the code action `RemoveUsingNamespace`

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry added a comment.

@junaire Thank you for reviewing!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137494

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

[PATCH] D137494: [Clangd] Fix the code action `RemoveUsingNamespace`

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry updated this revision to Diff 473480.
v1nh1shungry added a comment.

Format codes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137494

Files:
  clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
  clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp


Index: clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
@@ -226,6 +226,29 @@
   int main() {
 std::vector V;
   }
+)cpp"},
+  {// Does not qualify operators declared in a non-class context
+   R"cpp(
+  namespace ns {
+  struct Foo {};
+  void operator+(const Foo &, int) {}
+  }
+  using namespace n^s;
+  int main() {
+Foo foo;
+foo + 10;
+  }
+)cpp",
+   R"cpp(
+  namespace ns {
+  struct Foo {};
+  void operator+(const Foo &, int) {}
+  }
+  
+  int main() {
+ns::Foo foo;
+foo + 10;
+  }
 )cpp"}};
   for (auto C : Cases)
 EXPECT_EQ(C.second, apply(C.first)) << C.first;
Index: clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
@@ -155,6 +155,13 @@
 if (!visibleContext(T->getDeclContext())
  ->Equals(TargetDirective->getNominatedNamespace()))
   return;
+// Avoid adding qualifiers before operators, e.g.
+//   using namespace std;
+//   cout << "foo"; // Must not changed to std::cout std:: << "foo"
+// FIXME: User-defined literals are not handled
+if (T->isInIdentifierNamespace(
+Decl::IdentifierNamespace::IDNS_NonMemberOperator))
+  return;
   }
   SourceLocation Loc = Ref.NameLoc;
   if (Loc.isMacroID()) {


Index: clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
@@ -226,6 +226,29 @@
   int main() {
 std::vector V;
   }
+)cpp"},
+  {// Does not qualify operators declared in a non-class context
+   R"cpp(
+  namespace ns {
+  struct Foo {};
+  void operator+(const Foo &, int) {}
+  }
+  using namespace n^s;
+  int main() {
+Foo foo;
+foo + 10;
+  }
+)cpp",
+   R"cpp(
+  namespace ns {
+  struct Foo {};
+  void operator+(const Foo &, int) {}
+  }
+  
+  int main() {
+ns::Foo foo;
+foo + 10;
+  }
 )cpp"}};
   for (auto C : Cases)
 EXPECT_EQ(C.second, apply(C.first)) << C.first;
Index: clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
@@ -155,6 +155,13 @@
 if (!visibleContext(T->getDeclContext())
  ->Equals(TargetDirective->getNominatedNamespace()))
   return;
+// Avoid adding qualifiers before operators, e.g.
+//   using namespace std;
+//   cout << "foo"; // Must not changed to std::cout std:: << "foo"
+// FIXME: User-defined literals are not handled
+if (T->isInIdentifierNamespace(
+Decl::IdentifierNamespace::IDNS_NonMemberOperator))
+  return;
   }
   SourceLocation Loc = Ref.NameLoc;
   if (Loc.isMacroID()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D137494: [Clangd] Fix the code action `RemoveUsingNamespace`

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry added a comment.

In D137494#3910598 , @tom-anders 
wrote:

> Thanks for the fix, LGTM! Do you want me to commit this for you?

Yes! Thanks a lot!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137494

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

[PATCH] D137550: [clangd] Fix the code action `RemoveUsingNamespace`

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry created this revision.
v1nh1shungry added reviewers: tom-anders, sammccall.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
v1nh1shungry requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

Avoid adding qualifiers before user-defined literals


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D137550

Files:
  clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
  clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp


Index: clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
@@ -249,6 +249,21 @@
 ns::Foo foo;
 foo + 10;
   }
+)cpp"},
+  {// Does qualify user-defined literals
+   R"cpp(
+  namespace ns {
+  long double operator "" _w(long double);
+  }
+  using namespace n^s;
+  int main() { 1.5_w; }
+)cpp",
+   R"cpp(
+  namespace ns {
+  long double operator "" _w(long double);
+  }
+  
+  int main() { 1.5_w; }
 )cpp"}};
   for (auto C : Cases)
 EXPECT_EQ(C.second, apply(C.first)) << C.first;
Index: clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
@@ -155,12 +155,14 @@
 if (!visibleContext(T->getDeclContext())
  ->Equals(TargetDirective->getNominatedNamespace()))
   return;
-// Avoid adding qualifiers before operators, e.g.
+// Avoid adding qualifiers before operators
+// and user-defined literals, e.g.
 //   using namespace std;
+//   auto s = "foo"s; // Must not changed to auto s = "foo" std::s;
 //   cout << "foo"; // Must not changed to std::cout std:: << "foo"
-// FIXME: User-defined literals are not handled
-if (T->isInIdentifierNamespace(
-Decl::IdentifierNamespace::IDNS_NonMemberOperator))
+if (auto kind = T->getDeclName().getNameKind();
+kind == DeclarationName::NameKind::CXXOperatorName ||
+kind == DeclarationName::NameKind::CXXLiteralOperatorName)
   return;
   }
   SourceLocation Loc = Ref.NameLoc;


Index: clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
@@ -249,6 +249,21 @@
 ns::Foo foo;
 foo + 10;
   }
+)cpp"},
+  {// Does qualify user-defined literals
+   R"cpp(
+  namespace ns {
+  long double operator "" _w(long double);
+  }
+  using namespace n^s;
+  int main() { 1.5_w; }
+)cpp",
+   R"cpp(
+  namespace ns {
+  long double operator "" _w(long double);
+  }
+  
+  int main() { 1.5_w; }
 )cpp"}};
   for (auto C : Cases)
 EXPECT_EQ(C.second, apply(C.first)) << C.first;
Index: clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
@@ -155,12 +155,14 @@
 if (!visibleContext(T->getDeclContext())
  ->Equals(TargetDirective->getNominatedNamespace()))
   return;
-// Avoid adding qualifiers before operators, e.g.
+// Avoid adding qualifiers before operators
+// and user-defined literals, e.g.
 //   using namespace std;
+//   auto s = "foo"s; // Must not changed to auto s = "foo" std::s;
 //   cout << "foo"; // Must not changed to std::cout std:: << "foo"
-// FIXME: User-defined literals are not handled
-if (T->isInIdentifierNamespace(
-Decl::IdentifierNamespace::IDNS_NonMemberOperator))
+if (auto kind = T->getDeclName().getNameKind();
+kind == DeclarationName::NameKind::CXXOperatorName ||
+kind == DeclarationName::NameKind::CXXLiteralOperatorName)
   return;
   }
   SourceLocation Loc = Ref.NameLoc;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D137650: [clangd] Implement hover for C++ string literals

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry created this revision.
v1nh1shungry added reviewers: nridge, tom-anders.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
v1nh1shungry requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

Show C++ string-literals' character's type and length in a hover card

Issue related: https://github.com/clangd/clangd/issues/1016


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D137650

Files:
  clang-tools-extra/clangd/Hover.cpp
  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
@@ -1300,7 +1300,6 @@
   "auto x = ^42.0i;",
   "auto x = ^42;",
   "auto x = ^nullptr;",
-  "auto x = ^\"asdf\";",
   };
 
   for (const auto &Test : Tests) {
@@ -1326,6 +1325,11 @@
  HI.Type = "char";
  HI.Value = "65 (0x41)";
}},
+  {"auto s = ^[[\"Hello, world!\"]]; // string literal",
+   [](HoverInfo &HI) {
+ HI.Name = "String Literal";
+ HI.Type = "const char[14]";
+   }},
   {
   R"cpp(// Local variable
 int main() {
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -777,6 +777,52 @@
   return HI;
 }
 
+llvm::Optional
+getStringLiteralHoverContents(const StringLiteral *SL,
+  const LangOptions &LangOpts) {
+  HoverInfo HI;
+
+  // TODO: Show string literal's contents
+  HI.Name = "String Literal";
+
+  // In C++ string literals have `const` qualifier, but in C it don't.
+  std::string Qualifier;
+
+  std::string CharType;
+  // C++
+  // TODO: Support different C++ standards
+  if (LangOpts.CPlusPlus) {
+Qualifier = "const ";
+switch (SL->getKind()) {
+case StringLiteral::Ordinary:
+  CharType = "char";
+  break;
+case StringLiteral::Wide:
+  CharType = "wchar_t";
+  break;
+case StringLiteral::UTF8:
+  CharType = "char8_t";
+  break;
+case StringLiteral::UTF16:
+  CharType = "char16_t";
+  break;
+case StringLiteral::UTF32:
+  CharType = "char32_t";
+  break;
+}
+  } else {
+// TODO: For other languages
+return llvm::None;
+  }
+
+  HoverInfo::PrintedType Type;
+  Type.Type =
+  Qualifier + CharType + "[" + std::to_string(SL->getLength() + 1) + "]";
+  HI.Type = Type;
+
+  return HI;
+}
+
 bool isLiteral(const Expr *E) {
   // Unfortunately there's no common base Literal classes inherits from
   // (apart from Expr), therefore these exclusions.
@@ -806,8 +852,14 @@
const SymbolIndex *Index) {
   // There's not much value in hovering over "42" and getting a hover card
   // saying "42 is an int", similar for other literals.
-  if (isLiteral(E))
+  if (isLiteral(E)) {
+// Generate hover info for string literals showing
+// its character's type and length
+if (const StringLiteral *SL = dyn_cast(E)) {
+  return getStringLiteralHoverContents(SL, AST.getLangOpts());
+}
 return llvm::None;
+  }
 
   HoverInfo HI;
   // For `this` expr we currently generate hover with pointee type.


Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1300,7 +1300,6 @@
   "auto x = ^42.0i;",
   "auto x = ^42;",
   "auto x = ^nullptr;",
-  "auto x = ^\"asdf\";",
   };
 
   for (const auto &Test : Tests) {
@@ -1326,6 +1325,11 @@
  HI.Type = "char";
  HI.Value = "65 (0x41)";
}},
+  {"auto s = ^[[\"Hello, world!\"]]; // string literal",
+   [](HoverInfo &HI) {
+ HI.Name = "String Literal";
+ HI.Type = "const char[14]";
+   }},
   {
   R"cpp(// Local variable
 int main() {
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -777,6 +777,52 @@
   return HI;
 }
 
+llvm::Optional
+getStringLiteralHoverContents(const StringLiteral *SL,
+  const LangOptions &LangOpts) {
+  HoverInfo HI;
+
+  // TODO: Show string literal's contents
+  HI.Name = "String Literal";
+
+  // In C++ string literals have `const` qualifier, but in C it don't.
+  std::string Qualifier;
+
+  std::string CharType;
+  // C++
+  // TODO: Support different C++ standards
+  if (LangOpts.CPlusPlus) {
+Qualifier = "const ";
+switch (SL->getKind()) {
+case StringLiteral::Ordinary

[PATCH] D137650: [clangd] Implement hover for C++ string literals

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry added a comment.

@tom-anders Thanks for reviewing and for your suggestions!




Comment at: clang-tools-extra/clangd/Hover.cpp:785
+
+  // TODO: Show string literal's contents
+  HI.Name = "String Literal";

tom-anders wrote:
> Is it really useful to show the contents inside the Hover? You already see 
> the contents of the string literal anyway, so I don't think this adds any 
> value
I added this comment only because others show the contents. I think you're 
right.



Comment at: clang-tools-extra/clangd/Hover.cpp:814
+  } else {
+// TODO: For other languages
+return llvm::None;

tom-anders wrote:
> Hmm so what's stopping us from adding support for C here?
Because I'm not that familiar with C's data types and standards.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137650

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

[PATCH] D137650: [clangd] Implement hover for C++ string literals

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry updated this revision to Diff 474117.
v1nh1shungry added a comment.

Improve the patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137650

Files:
  clang-tools-extra/clangd/Hover.cpp
  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
@@ -1300,7 +1300,6 @@
   "auto x = ^42.0i;",
   "auto x = ^42;",
   "auto x = ^nullptr;",
-  "auto x = ^\"asdf\";",
   };
 
   for (const auto &Test : Tests) {
@@ -1326,6 +1325,11 @@
  HI.Type = "char";
  HI.Value = "65 (0x41)";
}},
+  {"auto s = ^[[\"Hello, world!\"]]; // string literal",
+   [](HoverInfo &HI) {
+ HI.Name = "String Literal";
+ HI.Type = "const char[14]";
+   }},
   {
   R"cpp(// Local variable
 int main() {
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -777,6 +777,51 @@
   return HI;
 }
 
+llvm::Optional
+getStringLiteralHoverContents(const StringLiteral *SL,
+  const LangOptions &LangOpts) {
+  HoverInfo HI;
+
+  HI.Name = "String Literal";
+
+  // In C++ string literals have `const` qualifier, but in C they don't.
+  std::string Qualifier;
+
+  std::string CharType;
+  // C++
+  // TODO: Support different C++ standards
+  if (LangOpts.CPlusPlus) {
+Qualifier = "const";
+switch (SL->getKind()) {
+case StringLiteral::Ordinary:
+  CharType = "char";
+  break;
+case StringLiteral::Wide:
+  CharType = "wchar_t";
+  break;
+case StringLiteral::UTF8:
+  CharType = "char8_t";
+  break;
+case StringLiteral::UTF16:
+  CharType = "char16_t";
+  break;
+case StringLiteral::UTF32:
+  CharType = "char32_t";
+  break;
+}
+  } else {
+// TODO: For other languages
+return llvm::None;
+  }
+
+  HoverInfo::PrintedType Type;
+  Type.Type =
+  llvm::formatv("{0} {1}[{2}]", Qualifier, CharType, SL->getLength() + 1);
+  HI.Type = Type;
+
+  return HI;
+}
+
 bool isLiteral(const Expr *E) {
   // Unfortunately there's no common base Literal classes inherits from
   // (apart from Expr), therefore these exclusions.
@@ -806,8 +851,14 @@
const SymbolIndex *Index) {
   // There's not much value in hovering over "42" and getting a hover card
   // saying "42 is an int", similar for other literals.
-  if (isLiteral(E))
+  if (isLiteral(E)) {
+// Generate hover info for string literals showing
+// its character's type and length
+if (const StringLiteral *SL = dyn_cast(E)) {
+  return getStringLiteralHoverContents(SL, AST.getLangOpts());
+}
 return llvm::None;
+  }
 
   HoverInfo HI;
   // For `this` expr we currently generate hover with pointee type.


Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1300,7 +1300,6 @@
   "auto x = ^42.0i;",
   "auto x = ^42;",
   "auto x = ^nullptr;",
-  "auto x = ^\"asdf\";",
   };
 
   for (const auto &Test : Tests) {
@@ -1326,6 +1325,11 @@
  HI.Type = "char";
  HI.Value = "65 (0x41)";
}},
+  {"auto s = ^[[\"Hello, world!\"]]; // string literal",
+   [](HoverInfo &HI) {
+ HI.Name = "String Literal";
+ HI.Type = "const char[14]";
+   }},
   {
   R"cpp(// Local variable
 int main() {
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -777,6 +777,51 @@
   return HI;
 }
 
+llvm::Optional
+getStringLiteralHoverContents(const StringLiteral *SL,
+  const LangOptions &LangOpts) {
+  HoverInfo HI;
+
+  HI.Name = "String Literal";
+
+  // In C++ string literals have `const` qualifier, but in C they don't.
+  std::string Qualifier;
+
+  std::string CharType;
+  // C++
+  // TODO: Support different C++ standards
+  if (LangOpts.CPlusPlus) {
+Qualifier = "const";
+switch (SL->getKind()) {
+case StringLiteral::Ordinary:
+  CharType = "char";
+  break;
+case StringLiteral::Wide:
+  CharType = "wchar_t";
+  break;
+case StringLiteral::UTF8:
+  CharType = "char8_t";
+  break;
+case StringLiteral::UTF16:
+  CharType = "char16_t";
+  break;
+case StringLiteral::UTF32:
+  CharType = "char32_t";
+  break;
+}
+  } else {
+ 

[PATCH] D137550: [clangd] Fix the code action `RemoveUsingNamespace`

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry updated this revision to Diff 474127.
v1nh1shungry added a comment.

Improve the patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137550

Files:
  clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
  clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp


Index: clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
@@ -249,6 +249,21 @@
 ns::Foo foo;
 foo + 10;
   }
+)cpp"},
+  {// Does not qualify user-defined literals
+   R"cpp(
+  namespace ns {
+  long double operator "" _w(long double);
+  }
+  using namespace n^s;
+  int main() { 1.5_w; }
+)cpp",
+   R"cpp(
+  namespace ns {
+  long double operator "" _w(long double);
+  }
+  
+  int main() { 1.5_w; }
 )cpp"}};
   for (auto C : Cases)
 EXPECT_EQ(C.second, apply(C.first)) << C.first;
Index: clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
@@ -155,12 +155,21 @@
 if (!visibleContext(T->getDeclContext())
  ->Equals(TargetDirective->getNominatedNamespace()))
   return;
-// Avoid adding qualifiers before operators, e.g.
+// Avoid adding qualifiers before operators
+// and user-defined literals, e.g.
 //   using namespace std;
+//   auto s = "foo"s; // Must not changed to auto s = "foo" std::s;
 //   cout << "foo"; // Must not changed to std::cout std:: << "foo"
-// FIXME: User-defined literals are not handled
-if (T->isInIdentifierNamespace(
-Decl::IdentifierNamespace::IDNS_NonMemberOperator))
+// FIXME: Add a using-directive for user-defined literals
+// declared in an inline namespace, e.g.
+//   using namespace s^td;
+//   int main() { cout << "foo"s; }
+// change to
+//   using namespace std::literals;
+//   int main() { std::cout << "foo"s; }
+if (auto kind = T->getDeclName().getNameKind();
+kind == DeclarationName::NameKind::CXXOperatorName ||
+kind == DeclarationName::NameKind::CXXLiteralOperatorName)
   return;
   }
   SourceLocation Loc = Ref.NameLoc;


Index: clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
@@ -249,6 +249,21 @@
 ns::Foo foo;
 foo + 10;
   }
+)cpp"},
+  {// Does not qualify user-defined literals
+   R"cpp(
+  namespace ns {
+  long double operator "" _w(long double);
+  }
+  using namespace n^s;
+  int main() { 1.5_w; }
+)cpp",
+   R"cpp(
+  namespace ns {
+  long double operator "" _w(long double);
+  }
+  
+  int main() { 1.5_w; }
 )cpp"}};
   for (auto C : Cases)
 EXPECT_EQ(C.second, apply(C.first)) << C.first;
Index: clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
@@ -155,12 +155,21 @@
 if (!visibleContext(T->getDeclContext())
  ->Equals(TargetDirective->getNominatedNamespace()))
   return;
-// Avoid adding qualifiers before operators, e.g.
+// Avoid adding qualifiers before operators
+// and user-defined literals, e.g.
 //   using namespace std;
+//   auto s = "foo"s; // Must not changed to auto s = "foo" std::s;
 //   cout << "foo"; // Must not changed to std::cout std:: << "foo"
-// FIXME: User-defined literals are not handled
-if (T->isInIdentifierNamespace(
-Decl::IdentifierNamespace::IDNS_NonMemberOperator))
+// FIXME: Add a using-directive for user-defined literals
+// declared in an inline namespace, e.g.
+//   using namespace s^td;
+//   int main() { cout << "foo"s; }
+// change to
+//   using namespace std::literals;
+//   int main() { std::cout <<

[PATCH] D137550: [clangd] Fix the code action `RemoveUsingNamespace`

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry added a comment.

@tom-anders Thanks for reviewing and for your suggestions!

I think your suggestion is excellent, but for now, it is too complex for me to 
implement.

If this patch is okay, could you please help me commit it? Thanks again!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137550

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

[PATCH] D137650: [clangd] Implement hover for C++ string literals

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry updated this revision to Diff 474147.
v1nh1shungry added a comment.

Support different C++ standards


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137650

Files:
  clang-tools-extra/clangd/Hover.cpp
  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
@@ -1300,7 +1300,6 @@
   "auto x = ^42.0i;",
   "auto x = ^42;",
   "auto x = ^nullptr;",
-  "auto x = ^\"asdf\";",
   };
 
   for (const auto &Test : Tests) {
@@ -1326,6 +1325,11 @@
  HI.Type = "char";
  HI.Value = "65 (0x41)";
}},
+  {"auto s = ^[[\"Hello, world!\"]]; // string literal",
+   [](HoverInfo &HI) {
+ HI.Name = "String Literal";
+ HI.Type = "const char[14]";
+   }},
   {
   R"cpp(// Local variable
 int main() {
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -777,6 +777,80 @@
   return HI;
 }
 
+llvm::Optional
+getStringLiteralHoverContents(const StringLiteral *SL,
+  const LangOptions &LangOpts) {
+  HoverInfo HI;
+
+  HI.Name = "String Literal";
+
+  // In C++ string literals have `const` qualifier, but in C they don't.
+  std::string Qualifier;
+
+  std::string CharType;
+  // C++
+  if (LangOpts.CPlusPlus) {
+Qualifier = "const";
+if (LangOpts.CPlusPlus20 || LangOpts.CPlusPlus2b) {
+  switch (SL->getKind()) {
+  case StringLiteral::Ordinary:
+CharType = "char";
+break;
+  case StringLiteral::Wide:
+CharType = "wchar_t";
+break;
+  case StringLiteral::UTF8:
+CharType = "char8_t";
+break;
+  case StringLiteral::UTF16:
+CharType = "char16_t";
+break;
+  case StringLiteral::UTF32:
+CharType = "char32_t";
+break;
+  }
+} else if (LangOpts.CPlusPlus11 || LangOpts.CPlusPlus14 ||
+   LangOpts.CPlusPlus17) {
+  switch (SL->getKind()) {
+  case StringLiteral::Ordinary:
+  case StringLiteral::UTF8:
+CharType = "char";
+break;
+  case StringLiteral::Wide:
+CharType = "wchar_t";
+break;
+  case StringLiteral::UTF16:
+CharType = "char16_t";
+break;
+  case StringLiteral::UTF32:
+CharType = "char32_t";
+break;
+  }
+} else {
+  switch (SL->getKind()) {
+  case StringLiteral::Ordinary:
+CharType = "char";
+break;
+  case StringLiteral::Wide:
+CharType = "wchar_t";
+break;
+  default:
+llvm_unreachable("Unsupported until C++11");
+  }
+}
+  } else {
+// TODO: For other languages
+return llvm::None;
+  }
+
+  HoverInfo::PrintedType Type;
+  Type.Type =
+  llvm::formatv("{0} {1}[{2}]", Qualifier, CharType, SL->getLength() + 1);
+  HI.Type = Type;
+
+  return HI;
+}
+
 bool isLiteral(const Expr *E) {
   // Unfortunately there's no common base Literal classes inherits from
   // (apart from Expr), therefore these exclusions.
@@ -806,8 +880,14 @@
const SymbolIndex *Index) {
   // There's not much value in hovering over "42" and getting a hover card
   // saying "42 is an int", similar for other literals.
-  if (isLiteral(E))
+  if (isLiteral(E)) {
+// Generate hover info for string literals showing
+// its character's type and length
+if (const StringLiteral *SL = dyn_cast(E)) {
+  return getStringLiteralHoverContents(SL, AST.getLangOpts());
+}
 return llvm::None;
+  }
 
   HoverInfo HI;
   // For `this` expr we currently generate hover with pointee type.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D137650: [clangd] Implement hover for C++ string literals

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry updated this revision to Diff 474149.
v1nh1shungry added a comment.

Improve the patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137650

Files:
  clang-tools-extra/clangd/Hover.cpp
  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
@@ -1300,7 +1300,6 @@
   "auto x = ^42.0i;",
   "auto x = ^42;",
   "auto x = ^nullptr;",
-  "auto x = ^\"asdf\";",
   };
 
   for (const auto &Test : Tests) {
@@ -1326,6 +1325,11 @@
  HI.Type = "char";
  HI.Value = "65 (0x41)";
}},
+  {"auto s = ^[[\"Hello, world!\"]]; // string literal",
+   [](HoverInfo &HI) {
+ HI.Name = "String Literal";
+ HI.Type = "const char[14]";
+   }},
   {
   R"cpp(// Local variable
 int main() {
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -804,12 +804,23 @@
 llvm::Optional getHoverContents(const Expr *E, ParsedAST &AST,
const PrintingPolicy &PP,
const SymbolIndex *Index) {
+  HoverInfo HI;
+
   // There's not much value in hovering over "42" and getting a hover card
   // saying "42 is an int", similar for other literals.
-  if (isLiteral(E))
+  if (isLiteral(E)) {
+// Generate hover info for string literals showing
+// its character's type and length
+if (const StringLiteral *SL = dyn_cast(E)) {
+  HoverInfo::PrintedType PT;
+  PT.Type = SL->getType().getAsString(PP);
+  HI.Type = PT;
+  HI.Name = "String Literal";
+  return HI;
+}
 return llvm::None;
+  }
 
-  HoverInfo HI;
   // For `this` expr we currently generate hover with pointee type.
   if (const CXXThisExpr *CTE = dyn_cast(E))
 return getThisExprHoverContents(CTE, AST.getASTContext(), PP);


Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1300,7 +1300,6 @@
   "auto x = ^42.0i;",
   "auto x = ^42;",
   "auto x = ^nullptr;",
-  "auto x = ^\"asdf\";",
   };
 
   for (const auto &Test : Tests) {
@@ -1326,6 +1325,11 @@
  HI.Type = "char";
  HI.Value = "65 (0x41)";
}},
+  {"auto s = ^[[\"Hello, world!\"]]; // string literal",
+   [](HoverInfo &HI) {
+ HI.Name = "String Literal";
+ HI.Type = "const char[14]";
+   }},
   {
   R"cpp(// Local variable
 int main() {
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -804,12 +804,23 @@
 llvm::Optional getHoverContents(const Expr *E, ParsedAST &AST,
const PrintingPolicy &PP,
const SymbolIndex *Index) {
+  HoverInfo HI;
+
   // There's not much value in hovering over "42" and getting a hover card
   // saying "42 is an int", similar for other literals.
-  if (isLiteral(E))
+  if (isLiteral(E)) {
+// Generate hover info for string literals showing
+// its character's type and length
+if (const StringLiteral *SL = dyn_cast(E)) {
+  HoverInfo::PrintedType PT;
+  PT.Type = SL->getType().getAsString(PP);
+  HI.Type = PT;
+  HI.Name = "String Literal";
+  return HI;
+}
 return llvm::None;
+  }
 
-  HoverInfo HI;
   // For `this` expr we currently generate hover with pointee type.
   if (const CXXThisExpr *CTE = dyn_cast(E))
 return getThisExprHoverContents(CTE, AST.getASTContext(), PP);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D137650: [clangd] Implement hover for C++ string literals

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry added a comment.

Seems like I have made things too complex :(

Should work for C, C++, and other languages now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137650

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

[PATCH] D137650: [clangd] Implement hover for string literals

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry updated this revision to Diff 474179.
v1nh1shungry added a comment.

Improve the patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137650

Files:
  clang-tools-extra/clangd/Hover.cpp
  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
@@ -1300,7 +1300,6 @@
   "auto x = ^42.0i;",
   "auto x = ^42;",
   "auto x = ^nullptr;",
-  "auto x = ^\"asdf\";",
   };
 
   for (const auto &Test : Tests) {
@@ -1326,6 +1325,11 @@
  HI.Type = "char";
  HI.Value = "65 (0x41)";
}},
+  {"auto s = ^[[\"Hello, world!\"]]; // string literal",
+   [](HoverInfo &HI) {
+ HI.Name = "String Literal";
+ HI.Type = "const char[14]";
+   }},
   {
   R"cpp(// Local variable
 int main() {
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -785,7 +785,7 @@
  llvm::isa(E) ||
  llvm::isa(E) || llvm::isa(E) ||
  llvm::isa(E) || llvm::isa(E) ||
- llvm::isa(E) || llvm::isa(E);
+ llvm::isa(E);
 }
 
 llvm::StringLiteral getNameForExpr(const Expr *E) {
@@ -810,6 +810,15 @@
 return llvm::None;
 
   HoverInfo HI;
+  // Generate hover info for string literals showing
+  // its character's type and length
+  if (llvm::isa(E)) {
+HoverInfo::PrintedType PT;
+PT.Type = E->getType().getAsString(PP);
+HI.Type = PT;
+HI.Name = "String Literal";
+return HI;
+  }
   // For `this` expr we currently generate hover with pointee type.
   if (const CXXThisExpr *CTE = dyn_cast(E))
 return getThisExprHoverContents(CTE, AST.getASTContext(), PP);


Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1300,7 +1300,6 @@
   "auto x = ^42.0i;",
   "auto x = ^42;",
   "auto x = ^nullptr;",
-  "auto x = ^\"asdf\";",
   };
 
   for (const auto &Test : Tests) {
@@ -1326,6 +1325,11 @@
  HI.Type = "char";
  HI.Value = "65 (0x41)";
}},
+  {"auto s = ^[[\"Hello, world!\"]]; // string literal",
+   [](HoverInfo &HI) {
+ HI.Name = "String Literal";
+ HI.Type = "const char[14]";
+   }},
   {
   R"cpp(// Local variable
 int main() {
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -785,7 +785,7 @@
  llvm::isa(E) ||
  llvm::isa(E) || llvm::isa(E) ||
  llvm::isa(E) || llvm::isa(E) ||
- llvm::isa(E) || llvm::isa(E);
+ llvm::isa(E);
 }
 
 llvm::StringLiteral getNameForExpr(const Expr *E) {
@@ -810,6 +810,15 @@
 return llvm::None;
 
   HoverInfo HI;
+  // Generate hover info for string literals showing
+  // its character's type and length
+  if (llvm::isa(E)) {
+HoverInfo::PrintedType PT;
+PT.Type = E->getType().getAsString(PP);
+HI.Type = PT;
+HI.Name = "String Literal";
+return HI;
+  }
   // For `this` expr we currently generate hover with pointee type.
   if (const CXXThisExpr *CTE = dyn_cast(E))
 return getThisExprHoverContents(CTE, AST.getASTContext(), PP);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D137550: [clangd] Fix the code action `RemoveUsingNamespace`

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry updated this revision to Diff 474262.
v1nh1shungry added a comment.

Format codes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137550

Files:
  clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
  clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp


Index: clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
@@ -249,6 +249,21 @@
 ns::Foo foo;
 foo + 10;
   }
+)cpp"},
+  {// Does not qualify user-defined literals
+   R"cpp(
+  namespace ns {
+  long double operator "" _w(long double);
+  }
+  using namespace n^s;
+  int main() { 1.5_w; }
+)cpp",
+   R"cpp(
+  namespace ns {
+  long double operator "" _w(long double);
+  }
+  
+  int main() { 1.5_w; }
 )cpp"}};
   for (auto C : Cases)
 EXPECT_EQ(C.second, apply(C.first)) << C.first;
Index: clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
@@ -155,12 +155,23 @@
 if (!visibleContext(T->getDeclContext())
  ->Equals(TargetDirective->getNominatedNamespace()))
   return;
+auto Kind = T->getDeclName().getNameKind();
 // Avoid adding qualifiers before operators, e.g.
 //   using namespace std;
 //   cout << "foo"; // Must not changed to std::cout std:: << "foo"
-// FIXME: User-defined literals are not handled
-if (T->isInIdentifierNamespace(
-Decl::IdentifierNamespace::IDNS_NonMemberOperator))
+if (Kind == DeclarationName::CXXOperatorName)
+  return;
+// Avoid adding qualifiers before user-defined literals, e.g.
+//   using namespace std;
+//   auto s = "foo"s; // Must not changed to auto s = "foo" std::s;
+// FIXME: Add a using-directive for user-defined literals
+// declared in an inline namespace, e.g.
+//   using namespace s^td;
+//   int main() { cout << "foo"s; }
+// change to
+//   using namespace std::literals;
+//   int main() { std::cout << "foo"s; }
+if (Kind == DeclarationName::NameKind::CXXLiteralOperatorName)
   return;
   }
   SourceLocation Loc = Ref.NameLoc;


Index: clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
@@ -249,6 +249,21 @@
 ns::Foo foo;
 foo + 10;
   }
+)cpp"},
+  {// Does not qualify user-defined literals
+   R"cpp(
+  namespace ns {
+  long double operator "" _w(long double);
+  }
+  using namespace n^s;
+  int main() { 1.5_w; }
+)cpp",
+   R"cpp(
+  namespace ns {
+  long double operator "" _w(long double);
+  }
+  
+  int main() { 1.5_w; }
 )cpp"}};
   for (auto C : Cases)
 EXPECT_EQ(C.second, apply(C.first)) << C.first;
Index: clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
@@ -155,12 +155,23 @@
 if (!visibleContext(T->getDeclContext())
  ->Equals(TargetDirective->getNominatedNamespace()))
   return;
+auto Kind = T->getDeclName().getNameKind();
 // Avoid adding qualifiers before operators, e.g.
 //   using namespace std;
 //   cout << "foo"; // Must not changed to std::cout std:: << "foo"
-// FIXME: User-defined literals are not handled
-if (T->isInIdentifierNamespace(
-Decl::IdentifierNamespace::IDNS_NonMemberOperator))
+if (Kind == DeclarationName::CXXOperatorName)
+  return;
+// Avoid adding qualifiers before user-defined literals, e.g.
+//   using namespace std;
+//   auto s = "foo"s; // Must not changed to auto s = "foo" std::s;
+// FIXME: Add a using-directive for user-defined literals
+// declared in an inline namespace, e.g.
+//   using namespace s^td;
+   

[PATCH] D137650: [clangd] Implement hover for string literals

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry updated this revision to Diff 474399.
v1nh1shungry added a comment.

Apply the reviewing suggestions


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137650

Files:
  clang-tools-extra/clangd/Hover.cpp
  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
@@ -1300,7 +1300,6 @@
   "auto x = ^42.0i;",
   "auto x = ^42;",
   "auto x = ^nullptr;",
-  "auto x = ^\"asdf\";",
   };
 
   for (const auto &Test : Tests) {
@@ -1326,6 +1325,12 @@
  HI.Type = "char";
  HI.Value = "65 (0x41)";
}},
+  {"auto s = ^[[\"Hello, world!\"]]; // string literal",
+   [](HoverInfo &HI) {
+ HI.Name = "String Literal";
+ HI.Size = 14;
+ HI.Type = "const char[14]";
+   }},
   {
   R"cpp(// Local variable
 int main() {
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -777,6 +777,17 @@
   return HI;
 }
 
+HoverInfo getStringLiteralContents(const StringLiteral *SL,
+   const PrintingPolicy &PP) {
+  HoverInfo HI;
+
+  HI.Name = "String Literal";
+  HI.Size = (SL->getLength() + 1) * SL->getCharByteWidth();
+  HI.Type = SL->getType().getAsString(PP).c_str();
+
+  return HI;
+}
+
 bool isLiteral(const Expr *E) {
   // Unfortunately there's no common base Literal classes inherits from
   // (apart from Expr), therefore these exclusions.
@@ -785,7 +796,7 @@
  llvm::isa(E) ||
  llvm::isa(E) || llvm::isa(E) ||
  llvm::isa(E) || llvm::isa(E) ||
- llvm::isa(E) || llvm::isa(E);
+ llvm::isa(E);
 }
 
 llvm::StringLiteral getNameForExpr(const Expr *E) {
@@ -810,6 +821,9 @@
 return llvm::None;
 
   HoverInfo HI;
+  // Print the type and the size for string literals
+  if (const StringLiteral *SL = dyn_cast(E))
+return getStringLiteralContents(SL, PP);
   // For `this` expr we currently generate hover with pointee type.
   if (const CXXThisExpr *CTE = dyn_cast(E))
 return getThisExprHoverContents(CTE, AST.getASTContext(), PP);


Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1300,7 +1300,6 @@
   "auto x = ^42.0i;",
   "auto x = ^42;",
   "auto x = ^nullptr;",
-  "auto x = ^\"asdf\";",
   };
 
   for (const auto &Test : Tests) {
@@ -1326,6 +1325,12 @@
  HI.Type = "char";
  HI.Value = "65 (0x41)";
}},
+  {"auto s = ^[[\"Hello, world!\"]]; // string literal",
+   [](HoverInfo &HI) {
+ HI.Name = "String Literal";
+ HI.Size = 14;
+ HI.Type = "const char[14]";
+   }},
   {
   R"cpp(// Local variable
 int main() {
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -777,6 +777,17 @@
   return HI;
 }
 
+HoverInfo getStringLiteralContents(const StringLiteral *SL,
+   const PrintingPolicy &PP) {
+  HoverInfo HI;
+
+  HI.Name = "String Literal";
+  HI.Size = (SL->getLength() + 1) * SL->getCharByteWidth();
+  HI.Type = SL->getType().getAsString(PP).c_str();
+
+  return HI;
+}
+
 bool isLiteral(const Expr *E) {
   // Unfortunately there's no common base Literal classes inherits from
   // (apart from Expr), therefore these exclusions.
@@ -785,7 +796,7 @@
  llvm::isa(E) ||
  llvm::isa(E) || llvm::isa(E) ||
  llvm::isa(E) || llvm::isa(E) ||
- llvm::isa(E) || llvm::isa(E);
+ llvm::isa(E);
 }
 
 llvm::StringLiteral getNameForExpr(const Expr *E) {
@@ -810,6 +821,9 @@
 return llvm::None;
 
   HoverInfo HI;
+  // Print the type and the size for string literals
+  if (const StringLiteral *SL = dyn_cast(E))
+return getStringLiteralContents(SL, PP);
   // For `this` expr we currently generate hover with pointee type.
   if (const CXXThisExpr *CTE = dyn_cast(E))
 return getThisExprHoverContents(CTE, AST.getASTContext(), PP);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D137650: [clangd] Implement hover for string literals

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry added a comment.

@kadircet Thanks for your suggestions!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137650

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

[PATCH] D137650: [clangd] Implement hover for string literals

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry updated this revision to Diff 474466.
v1nh1shungry added a comment.

Modify the name in the hover info


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137650

Files:
  clang-tools-extra/clangd/Hover.cpp
  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
@@ -1300,7 +1300,6 @@
   "auto x = ^42.0i;",
   "auto x = ^42;",
   "auto x = ^nullptr;",
-  "auto x = ^\"asdf\";",
   };
 
   for (const auto &Test : Tests) {
@@ -1326,6 +1325,12 @@
  HI.Type = "char";
  HI.Value = "65 (0x41)";
}},
+  {"auto s = ^[[\"Hello, world!\"]]; // string literal",
+   [](HoverInfo &HI) {
+ HI.Name = "string-literal";
+ HI.Size = 14;
+ HI.Type = "const char[14]";
+   }},
   {
   R"cpp(// Local variable
 int main() {
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -777,6 +777,17 @@
   return HI;
 }
 
+HoverInfo getStringLiteralContents(const StringLiteral *SL,
+   const PrintingPolicy &PP) {
+  HoverInfo HI;
+
+  HI.Name = "string-literal";
+  HI.Size = (SL->getLength() + 1) * SL->getCharByteWidth();
+  HI.Type = SL->getType().getAsString(PP).c_str();
+
+  return HI;
+}
+
 bool isLiteral(const Expr *E) {
   // Unfortunately there's no common base Literal classes inherits from
   // (apart from Expr), therefore these exclusions.
@@ -785,7 +796,7 @@
  llvm::isa(E) ||
  llvm::isa(E) || llvm::isa(E) ||
  llvm::isa(E) || llvm::isa(E) ||
- llvm::isa(E) || llvm::isa(E);
+ llvm::isa(E);
 }
 
 llvm::StringLiteral getNameForExpr(const Expr *E) {
@@ -810,6 +821,9 @@
 return llvm::None;
 
   HoverInfo HI;
+  // Print the type and the size for string literals
+  if (const StringLiteral *SL = dyn_cast(E))
+return getStringLiteralContents(SL, PP);
   // For `this` expr we currently generate hover with pointee type.
   if (const CXXThisExpr *CTE = dyn_cast(E))
 return getThisExprHoverContents(CTE, AST.getASTContext(), PP);


Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1300,7 +1300,6 @@
   "auto x = ^42.0i;",
   "auto x = ^42;",
   "auto x = ^nullptr;",
-  "auto x = ^\"asdf\";",
   };
 
   for (const auto &Test : Tests) {
@@ -1326,6 +1325,12 @@
  HI.Type = "char";
  HI.Value = "65 (0x41)";
}},
+  {"auto s = ^[[\"Hello, world!\"]]; // string literal",
+   [](HoverInfo &HI) {
+ HI.Name = "string-literal";
+ HI.Size = 14;
+ HI.Type = "const char[14]";
+   }},
   {
   R"cpp(// Local variable
 int main() {
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -777,6 +777,17 @@
   return HI;
 }
 
+HoverInfo getStringLiteralContents(const StringLiteral *SL,
+   const PrintingPolicy &PP) {
+  HoverInfo HI;
+
+  HI.Name = "string-literal";
+  HI.Size = (SL->getLength() + 1) * SL->getCharByteWidth();
+  HI.Type = SL->getType().getAsString(PP).c_str();
+
+  return HI;
+}
+
 bool isLiteral(const Expr *E) {
   // Unfortunately there's no common base Literal classes inherits from
   // (apart from Expr), therefore these exclusions.
@@ -785,7 +796,7 @@
  llvm::isa(E) ||
  llvm::isa(E) || llvm::isa(E) ||
  llvm::isa(E) || llvm::isa(E) ||
- llvm::isa(E) || llvm::isa(E);
+ llvm::isa(E);
 }
 
 llvm::StringLiteral getNameForExpr(const Expr *E) {
@@ -810,6 +821,9 @@
 return llvm::None;
 
   HoverInfo HI;
+  // Print the type and the size for string literals
+  if (const StringLiteral *SL = dyn_cast(E))
+return getStringLiteralContents(SL, PP);
   // For `this` expr we currently generate hover with pointee type.
   if (const CXXThisExpr *CTE = dyn_cast(E))
 return getThisExprHoverContents(CTE, AST.getASTContext(), PP);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D137650: [clangd] Implement hover for string literals

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry added a comment.

@kadircet Could you please help me commit it? Thanks a lot!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137650

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

[PATCH] D137817: [clangd] Improve action `RemoveUsingNamespace` for user-defined literals

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry created this revision.
v1nh1shungry added reviewers: tom-anders, kadircet.
Herald added subscribers: arphaman, mgrang.
Herald added a project: All.
v1nh1shungry requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

If we are removing the namespace `ns1` and there are user-defined literals
declared in an inline namespace `ns2` declared in `ns1`, we can add a
`using namespace ns1::ns2;` to keep the program correct.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D137817

Files:
  clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
  clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp

Index: clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
@@ -263,6 +263,26 @@
   long double operator "" _w(long double);
   }
   
+  int main() { 1.5_w; }
+)cpp"},
+  {// Add using-directives for user-defined literals
+   // declared in an inline namespace
+   R"cpp(
+  namespace ns1 {
+  inline namespace ns2 {
+  long double operator"" _w(long double);
+  }
+  }
+  using namespace n^s1;
+  int main() { 1.5_w; }
+)cpp",
+   R"cpp(
+  namespace ns1 {
+  inline namespace ns2 {
+  long double operator"" _w(long double);
+  }
+  }
+  using namespace ns1::ns2;
   int main() { 1.5_w; }
 )cpp"}};
   for (auto C : Cases)
Index: clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
@@ -54,6 +54,10 @@
   : TargetNS(Target.getNominatedNamespace()),
 TargetCtx(Target.getDeclContext()), Results(Results) {}
 
+  FindSameUsings(const NamespaceDecl *TargetNS, const DeclContext *TargetCtx,
+ std::vector &Results)
+  : TargetNS(TargetNS), TargetCtx(TargetCtx), Results(Results) {}
+
   bool VisitUsingDirectiveDecl(UsingDirectiveDecl *D) {
 if (D->getNominatedNamespace() != TargetNS ||
 D->getDeclContext() != TargetCtx)
@@ -101,6 +105,40 @@
   return D;
 }
 
+// If the user-defined literal is declared in an inline namespace,
+// we add a using-directive for it
+void handleUserDefinedLiteral(const DeclContext *D, const DeclContext *Ctx,
+  ASTContext &AST,
+  std::vector &UsingToAdd) {
+  if (D->isInlineNamespace()) {
+const auto *ND = cast(D);
+// Find if there is already a wanted using-directive,
+// if there is, we are done. Otherwise we collect it
+std::vector AllDirectives;
+FindSameUsings(ND, Ctx, AllDirectives).TraverseAST(AST);
+if (AllDirectives.empty()) {
+  UsingToAdd.push_back(ND->getQualifiedNameAsString());
+}
+  }
+}
+
+// Produce edit adding 'using namespace xxx::yyy;'
+// for user-defined literals declared in an inline namespace
+llvm::Optional
+addUsingDirectives(const std::vector &UsingToAdd,
+   SourceManager &SM, SourceLocation Loc) {
+  if (UsingToAdd.empty())
+return llvm::None;
+  std::string Directives;
+  for (auto &Using : UsingToAdd)
+Directives += llvm::formatv("using namespace {0};\n", Using);
+  // Remove the trailing newline
+  Directives.pop_back();
+  // For now we insert the using-directives to where the first using-directive
+  // to remove stands
+  return tooling::Replacement(SM, Loc, 0, Directives);
+}
+
 bool RemoveUsingNamespace::prepare(const Selection &Inputs) {
   // Find the 'using namespace' directive under the cursor.
   auto *CA = Inputs.ASTSelection.commonAncestor();
@@ -144,6 +182,9 @@
   // Collect all references to symbols from the namespace for which we're
   // removing the directive.
   std::vector IdentsToQualify;
+  // Collect all namespaces' name to add for user-defined literals declared
+  // in an inline namespace
+  std::vector UsingToAdd;
   for (auto &D : Inputs.AST->getLocalTopLevelDecls()) {
 findExplicitReferences(
 D,
@@ -164,15 +205,19 @@
 // Avoid adding qualifiers before user-defined literals, e.g.
 //   using namespace std;
 //   auto s = "foo"s; // Must not changed to auto s = "foo" std::s;
-// FIXME: Add a using-directive for user-defined literals
+// And add a using-directive for user-defined literals
 // declared in an inline namespace, e.g.
 //   using namespace s^td;
 //   int main() { cout << "foo"s; }
 // change to
-//   using namespace std::literals;
+//   using namespace s

[PATCH] D137817: [clangd] Improve action `RemoveUsingNamespace` for user-defined literals

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry updated this revision to Diff 474808.
v1nh1shungry added a comment.

Apply the review suggestions


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137817

Files:
  clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
  clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp

Index: clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
@@ -264,6 +264,39 @@
   }
   
   int main() { 1.5_w; }
+)cpp"},
+  {// Add using-directives for user-defined literals
+   // declared in an inline namespace
+   R"cpp(
+namespace ns1 {
+inline namespace ns2 {
+long double operator "" _w(long double);
+}
+inline namespace ns3 {
+long double operator "" _h(long double);
+}
+}
+using namespace n^s1;
+int main() {
+  1.5_h;
+  1.5_w;
+}
+)cpp",
+   R"cpp(
+namespace ns1 {
+inline namespace ns2 {
+long double operator "" _w(long double);
+}
+inline namespace ns3 {
+long double operator "" _h(long double);
+}
+}
+using namespace ns1::ns2;
+using namespace ns1::ns3;
+int main() {
+  1.5_h;
+  1.5_w;
+}
 )cpp"}};
   for (auto C : Cases)
 EXPECT_EQ(C.second, apply(C.first)) << C.first;
Index: clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
@@ -54,6 +54,10 @@
   : TargetNS(Target.getNominatedNamespace()),
 TargetCtx(Target.getDeclContext()), Results(Results) {}
 
+  FindSameUsings(const NamespaceDecl *TargetNS, const DeclContext *TargetCtx,
+ std::vector &Results)
+  : TargetNS(TargetNS), TargetCtx(TargetCtx), Results(Results) {}
+
   bool VisitUsingDirectiveDecl(UsingDirectiveDecl *D) {
 if (D->getNominatedNamespace() != TargetNS ||
 D->getDeclContext() != TargetCtx)
@@ -101,6 +105,43 @@
   return D;
 }
 
+// If the user-defined literal is declared in an inline namespace,
+// we add a using-directive for it
+void handleUserDefinedLiteral(const DeclContext *D, const DeclContext *Ctx,
+  ASTContext &AST, llvm::StringSet<> &UsingToAdd) {
+  if (D->isInlineNamespace()) {
+const auto *ND = cast(D);
+// Find if there is already a wanted using-directive,
+// if there is, we are done. Otherwise we collect it
+std::vector AllDirectives;
+FindSameUsings(ND, Ctx, AllDirectives).TraverseAST(AST);
+if (AllDirectives.empty()) {
+  UsingToAdd.insert(ND->getQualifiedNameAsString());
+}
+  }
+}
+
+// Produce edit adding 'using namespace xxx::yyy;'
+// for user-defined literals declared in an inline namespace
+llvm::Optional
+addUsingDirectives(const llvm::StringSet<> &UsingToAdd, SourceManager &SM,
+   SourceLocation Loc) {
+  if (UsingToAdd.empty())
+return llvm::None;
+  // FIXME: We simply join all the using-directives with newline,
+  //so it can't handle indent. For now this is correct
+  //because the code action only runs on the using-directive
+  //declared in the top level
+  std::string Directives;
+  for (const auto &Using : UsingToAdd)
+Directives += llvm::formatv("using namespace {0};\n", Using.getKey());
+  // Remove the trailing newline
+  Directives.pop_back();
+  // For now we insert the using-directives to where the first using-directive
+  // to remove stands
+  return tooling::Replacement(SM, Loc, 0, Directives);
+}
+
 bool RemoveUsingNamespace::prepare(const Selection &Inputs) {
   // Find the 'using namespace' directive under the cursor.
   auto *CA = Inputs.ASTSelection.commonAncestor();
@@ -144,6 +185,9 @@
   // Collect all references to symbols from the namespace for which we're
   // removing the directive.
   std::vector IdentsToQualify;
+  // Collect all namespaces' name to add for user-defined literals declared
+  // in an inline namespace
+  llvm::StringSet<> UsingToAdd;
   for (auto &D : Inputs.AST->getLocalTopLevelDecls()) {
 findExplicitReferences(
 D,
@@ -164,15 +208,19 @@
 // Avoid adding qualifiers before user-defined literals, e.g.
 //   using namespace std;
 //   auto s = "foo"s; // Must not changed to auto s = "foo" std::s;
-// FIXME: Add a using-directive for user-defined literals
+// And add a using-directive for user-defined literals
 // declared in an inline namespace, e.g.
 //   using namespace s^td;
 //   int main() { cout << "foo"s; }
 // change to
-//   using namespace std::literals;
+//   us

[PATCH] D137817: [clangd] Improve action `RemoveUsingNamespace` for user-defined literals

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry added a comment.

@tom-anders Thank you for reviewing and giving suggestions!

Hope I use the `llvm::StringSet` correctly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137817

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

[PATCH] D138028: [clangd] Fix action `RemoveUsingNamespace` for inline namespace

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry created this revision.
v1nh1shungry added a reviewer: tom-anders.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
v1nh1shungry requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

Existing version ignores symbols declared in an inline namespace `ns`
when removing `using namespace ns`


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138028

Files:
  clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
  clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp


Index: clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
@@ -264,6 +264,17 @@
   }
   
   int main() { 1.5_w; }
+)cpp"},
+  {
+  R"cpp(
+  namespace a { inline namespace b { void foobar(); } }
+  using namespace a::[[b]];
+  int main() { foobar(); }
+)cpp",
+  R"cpp(
+  namespace a { inline namespace b { void foobar(); } }
+  
+  int main() { a::b::foobar(); }
 )cpp"}};
   for (auto C : Cases)
 EXPECT_EQ(C.second, apply(C.first)) << C.first;
Index: clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
@@ -89,16 +89,15 @@
   return Node->Parent && Node->Parent->ASTNode.get();
 }
 
-// Returns the first visible context that contains this DeclContext.
-// For example: Returns ns1 for S1 and a.
-// namespace ns1 {
-// inline namespace ns2 { struct S1 {}; }
-// enum E { a, b, c, d };
-// }
-const DeclContext *visibleContext(const DeclContext *D) {
-  while (D->isInlineNamespace() || D->isTransparentContext())
+// Return true if `LHS` is declared in `RHS`
+bool declareIn(const NamedDecl *LHS, const DeclContext *RHS) {
+  const auto *D = LHS->getDeclContext();
+  while (D->isInlineNamespace() || D->isTransparentContext()) {
+if (D->Equals(RHS))
+  return true;
 D = D->getParent();
-  return D;
+  }
+  return D->Equals(RHS);
 }
 
 bool RemoveUsingNamespace::prepare(const Selection &Inputs) {
@@ -152,8 +151,7 @@
 return; // This reference is already qualified.
 
   for (auto *T : Ref.Targets) {
-if (!visibleContext(T->getDeclContext())
- ->Equals(TargetDirective->getNominatedNamespace()))
+if (!declareIn(T, TargetDirective->getNominatedNamespace()))
   return;
 auto Kind = T->getDeclName().getNameKind();
 // Avoid adding qualifiers before operators, e.g.


Index: clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
@@ -264,6 +264,17 @@
   }
   
   int main() { 1.5_w; }
+)cpp"},
+  {
+  R"cpp(
+  namespace a { inline namespace b { void foobar(); } }
+  using namespace a::[[b]];
+  int main() { foobar(); }
+)cpp",
+  R"cpp(
+  namespace a { inline namespace b { void foobar(); } }
+  
+  int main() { a::b::foobar(); }
 )cpp"}};
   for (auto C : Cases)
 EXPECT_EQ(C.second, apply(C.first)) << C.first;
Index: clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
@@ -89,16 +89,15 @@
   return Node->Parent && Node->Parent->ASTNode.get();
 }
 
-// Returns the first visible context that contains this DeclContext.
-// For example: Returns ns1 for S1 and a.
-// namespace ns1 {
-// inline namespace ns2 { struct S1 {}; }
-// enum E { a, b, c, d };
-// }
-const DeclContext *visibleContext(const DeclContext *D) {
-  while (D->isInlineNamespace() || D->isTransparentContext())
+// Return true if `LHS` is declared in `RHS`
+bool declareIn(const NamedDecl *LHS, const DeclContext *RHS) {
+  const auto *D = LHS->getDeclContext();
+  while (D->isInlineNamespace() || D->isTransparentContext()) {
+if (D->Equals(RHS))
+  return true;
 D = D->getParent();
-  return D;
+  }
+  return D->Equals(RHS);
 }
 
 bool RemoveUsingNamespace::prepare(const Selection &Inputs) {
@@ -152,8 +151,7 @@
 return; // This reference is already qualified.
 
   for (auto *T : Ref.Targets) {
-if (!visibleContext(T->getDeclContext())
-   

[PATCH] D138028: [clangd] Fix action `RemoveUsingNamespace` for inline namespace

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry added inline comments.



Comment at: 
clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp:212
   // Produce replacements to add the qualifiers.
   std::string Qualifier = printUsingNamespaceName(Ctx, *TargetDirective) + 
"::";
   for (auto Loc : IdentsToQualify) {

We can replace `printUsingNamespaceName` with `printNamespaceScope` here so 
that we can get `a::foobar()` in the test. 

However, it can sometimes cause redundancy such as in the 10th test. 

And I don't know whether it is worth it. WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138028

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

[PATCH] D138028: [clangd] Fix action `RemoveUsingNamespace` for inline namespace

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry added inline comments.



Comment at: 
clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp:212
   // Produce replacements to add the qualifiers.
   std::string Qualifier = printUsingNamespaceName(Ctx, *TargetDirective) + 
"::";
   for (auto Loc : IdentsToQualify) {

tom-anders wrote:
> v1nh1shungry wrote:
> > We can replace `printUsingNamespaceName` with `printNamespaceScope` here so 
> > that we can get `a::foobar()` in the test. 
> > 
> > However, it can sometimes cause redundancy such as in the 10th test. 
> > 
> > And I don't know whether it is worth it. WDYT?
> Just making sure I understood this correctly:
> 
> If you replace `printUsingNamespaceName` with `printNamespaceScope`, then...
> 
> - ...in the test you added it would result in `a::foobar()` instead of 
> `a::b::foobar()` (which is better)
> - ... but in this test (which is the 10th test if I counted correctly):
>  
> ```
>  namespace a::b { struct Foo {}; }
>   using namespace a;
>   using namespace a::[[b]];
>   using namespace b;
>   int main() { Foo F;}
> ```
> what would be the result..? would you get `a::Foo` instead of `a::b::Foo`?
> 
Sorry, I mean the next test. I read `10` from the inlay hint but I forgot the 
index starts from `0` :(

The test I want to mention:
```
namespace a::b { struct Foo {}; }
using namespace a;
using namespace a::b;
using namespace [[b]];
int main() { Foo F;}
```

We will get `a::b::Foo` in both the 10th and 11th tests. So in the 10th test, 
we don't get any benefits and don't sacrifice anything. In the 11th test, we 
get more redundancy than the existing version.

Apologize again for my mistake.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138028

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

[PATCH] D137817: [clangd] Improve action `RemoveUsingNamespace` for user-defined literals

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry added a comment.

@kadircet Thank you for reviewing and giving suggestions! Please check my reply 
to your concerns. Sorry if I misunderstood anything.




Comment at: 
clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp:294
+}
+using namespace ns1::ns2;
+using namespace ns1::ns3;

kadircet wrote:
> sorry i am having trouble understanding why we are:
> - only handling user defined literals from inline namespaces and not others?
> - producing using directives and not using declarations
> - inserting these at top level rather than where the usage is
The first question:

Because others have already been handled. Say

```
namespace a { inline namespace b { void foobar(); } }
using namespace ^a;
int main() { foobar(); }
```

can become

```
namespace a { inline namespace b { void foobar(); } }

int main() { a::foobar(); }
```

But user-defined literals just can't add a qualifier, right?

---

The second question:

Yes, this is a good idea. I didn't realize we can use using-declarations 
instead of using-directives.

---

The last question:

Hmm, I think it is cleaner if there are multiple usages. Since we can only 
remove the using-directives at the top level, this doesn't hurt anything. And 
it is the easiest solution to implement as well :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137817

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

[PATCH] D137817: [clangd] Improve action `RemoveUsingNamespace` for user-defined literals

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry added a comment.

@kadircet Thank you for the excellent suggestions! Will dive into it to figure 
out the implementations.




Comment at: 
clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp:294
+}
+using namespace ns1::ns2;
+using namespace ns1::ns3;

kadircet wrote:
> kadircet wrote:
> > v1nh1shungry wrote:
> > > kadircet wrote:
> > > > sorry i am having trouble understanding why we are:
> > > > - only handling user defined literals from inline namespaces and not 
> > > > others?
> > > > - producing using directives and not using declarations
> > > > - inserting these at top level rather than where the usage is
> > > The first question:
> > > 
> > > Because others have already been handled. Say
> > > 
> > > ```
> > > namespace a { inline namespace b { void foobar(); } }
> > > using namespace ^a;
> > > int main() { foobar(); }
> > > ```
> > > 
> > > can become
> > > 
> > > ```
> > > namespace a { inline namespace b { void foobar(); } }
> > > 
> > > int main() { a::foobar(); }
> > > ```
> > > 
> > > But user-defined literals just can't add a qualifier, right?
> > > 
> > > ---
> > > 
> > > The second question:
> > > 
> > > Yes, this is a good idea. I didn't realize we can use using-declarations 
> > > instead of using-directives.
> > > 
> > > ---
> > > 
> > > The last question:
> > > 
> > > Hmm, I think it is cleaner if there are multiple usages. Since we can 
> > > only remove the using-directives at the top level, this doesn't hurt 
> > > anything. And it is the easiest solution to implement as well :)
> > > Because others have already been handled. Say
> > 
> > i was emphasizing on the difference between user defined literals inside 
> > inline namespaces, and user defined literals from regular namespaces. not 
> > other types of decls inside an inline namespace, eg:
> > ```
> > namespace ns { long double operator"" _o(long double); }
> > ```
> > 
> > we should also introduce a using-decl for `_o` despite it not being inside 
> > an inline namespace.
> > Hmm, I think it is cleaner if there are multiple usages. Since we can only 
> > remove the using-directives at the top level, this doesn't hurt anything. 
> > And it is the easiest solution to implement as well :)
> 
> right, but introducing these at the top level will have unintended 
> consequences (e.g. if this is a header, symbols will all of a sudden be 
> visible in all the dependents).
You're right. Sorry for misunderstanding it.



Comment at: 
clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp:294
+}
+using namespace ns1::ns2;
+using namespace ns1::ns3;

v1nh1shungry wrote:
> kadircet wrote:
> > kadircet wrote:
> > > v1nh1shungry wrote:
> > > > kadircet wrote:
> > > > > sorry i am having trouble understanding why we are:
> > > > > - only handling user defined literals from inline namespaces and not 
> > > > > others?
> > > > > - producing using directives and not using declarations
> > > > > - inserting these at top level rather than where the usage is
> > > > The first question:
> > > > 
> > > > Because others have already been handled. Say
> > > > 
> > > > ```
> > > > namespace a { inline namespace b { void foobar(); } }
> > > > using namespace ^a;
> > > > int main() { foobar(); }
> > > > ```
> > > > 
> > > > can become
> > > > 
> > > > ```
> > > > namespace a { inline namespace b { void foobar(); } }
> > > > 
> > > > int main() { a::foobar(); }
> > > > ```
> > > > 
> > > > But user-defined literals just can't add a qualifier, right?
> > > > 
> > > > ---
> > > > 
> > > > The second question:
> > > > 
> > > > Yes, this is a good idea. I didn't realize we can use 
> > > > using-declarations instead of using-directives.
> > > > 
> > > > ---
> > > > 
> > > > The last question:
> > > > 
> > > > Hmm, I think it is cleaner if there are multiple usages. Since we can 
> > > > only remove the using-directives at the top level, this doesn't hurt 
> > > > anything. And it is the easiest solution to implement as well :)
> > > > Because others have already been handled. Say
> > > 
> > > i was emphasizing on the difference between user defined literals inside 
> > > inline namespaces, and user defined literals from regular namespaces. not 
> > > other types of decls inside an inline namespace, eg:
> > > ```
> > > namespace ns { long double operator"" _o(long double); }
> > > ```
> > > 
> > > we should also introduce a using-decl for `_o` despite it not being 
> > > inside an inline namespace.
> > > Hmm, I think it is cleaner if there are multiple usages. Since we can 
> > > only remove the using-directives at the top level, this doesn't hurt 
> > > anything. And it is the easiest solution to implement as well :)
> > 
> > right, but introducing these at the top level will have unintended 
> > consequences (e.g. if this is a header, symbols will all of a sudden be 
> > visible in all the dependents).
> You're right. Sorry for misunderstanding 

[PATCH] D138204: [clang-tidy] Fix misc-unused-using-decls for user-defined literals

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry created this revision.
v1nh1shungry added reviewers: njames93, ymandel.
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a project: All.
v1nh1shungry requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Current version complains unused using-declaration even if the target
user-defined literal is used.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138204

Files:
  clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/misc/unused-using-decls.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/misc/unused-using-decls.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/misc/unused-using-decls.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/misc/unused-using-decls.cpp
@@ -48,6 +48,7 @@
 void OverloadFunc(int);
 void OverloadFunc(double);
 int FuncUsedByUsingDeclInMacro() { return 1; }
+long double operator""_w(long double);
 
 class ostream {
 public:
@@ -106,6 +107,7 @@
 using n::UnusedFunc; // UnusedFunc
 // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: using decl 'UnusedFunc' is unused
 // CHECK-FIXES: {{^}}// UnusedFunc
+using n::operator""_w;
 using n::cout;
 using n::endl;
 
@@ -183,6 +185,7 @@
   UsedInstance.i;
   UsedFunc();
   UsedTemplateFunc();
+  1.5_w;
   cout << endl;
   Color2 color2;
   int t1 = Color3::Yellow;
Index: clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
@@ -54,6 +54,7 @@
   Finder->addMatcher(loc(templateSpecializationType(forEachTemplateArgument(
  templateArgument().bind("used",
  this);
+  Finder->addMatcher(userDefinedLiteral().bind("used"), this);
   // Cases where we can identify the UsingShadowDecl directly, rather than
   // just its target.
   // FIXME: cover more cases in this way, as the AST supports it.
@@ -150,7 +151,11 @@
   if (const auto *USD = dyn_cast(ND))
 removeFromFoundDecls(USD->getTargetDecl()->getCanonicalDecl());
 }
+return;
   }
+  // Check user-defined literals
+  if (const auto *UDL = Result.Nodes.getNodeAs("used"))
+removeFromFoundDecls(UDL->getCalleeDecl());
 }
 
 void UnusedUsingDeclsCheck::removeFromFoundDecls(const Decl *D) {


Index: clang-tools-extra/test/clang-tidy/checkers/misc/unused-using-decls.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/misc/unused-using-decls.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/misc/unused-using-decls.cpp
@@ -48,6 +48,7 @@
 void OverloadFunc(int);
 void OverloadFunc(double);
 int FuncUsedByUsingDeclInMacro() { return 1; }
+long double operator""_w(long double);
 
 class ostream {
 public:
@@ -106,6 +107,7 @@
 using n::UnusedFunc; // UnusedFunc
 // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: using decl 'UnusedFunc' is unused
 // CHECK-FIXES: {{^}}// UnusedFunc
+using n::operator""_w;
 using n::cout;
 using n::endl;
 
@@ -183,6 +185,7 @@
   UsedInstance.i;
   UsedFunc();
   UsedTemplateFunc();
+  1.5_w;
   cout << endl;
   Color2 color2;
   int t1 = Color3::Yellow;
Index: clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
@@ -54,6 +54,7 @@
   Finder->addMatcher(loc(templateSpecializationType(forEachTemplateArgument(
  templateArgument().bind("used",
  this);
+  Finder->addMatcher(userDefinedLiteral().bind("used"), this);
   // Cases where we can identify the UsingShadowDecl directly, rather than
   // just its target.
   // FIXME: cover more cases in this way, as the AST supports it.
@@ -150,7 +151,11 @@
   if (const auto *USD = dyn_cast(ND))
 removeFromFoundDecls(USD->getTargetDecl()->getCanonicalDecl());
 }
+return;
   }
+  // Check user-defined literals
+  if (const auto *UDL = Result.Nodes.getNodeAs("used"))
+removeFromFoundDecls(UDL->getCalleeDecl());
 }
 
 void UnusedUsingDeclsCheck::removeFromFoundDecls(const Decl *D) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D138204: [clang-tidy] Fix misc-unused-using-decls for user-defined literals

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry added a comment.

@ymandel Thank you for reviewing!

If this patch is okay to land, could you please help me commit it? Thanks again!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138204

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

[PATCH] D138300: [clangd] Support type hints for `decltype(expr)` in variable declarations

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry created this revision.
v1nh1shungry added a reviewer: nridge.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
v1nh1shungry requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138300

Files:
  clang-tools-extra/clangd/InlayHints.cpp
  clang-tools-extra/clangd/unittests/InlayHintTests.cpp


Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -1141,6 +1141,10 @@
   ExpectedHint{": int &", "z"});
 }
 
+TEST(TypeHints, Decltype) {
+  assertTypeHints("decltype(0) $i[[i]];", ExpectedHint{": int", "i"});
+}
+
 TEST(TypeHints, NoQualifiers) {
   assertTypeHints(R"cpp(
 namespace A {
Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -296,17 +296,25 @@
   return true;
 }
 
-if (D->getType()->getContainedAutoType()) {
-  if (!D->getType()->isDependentType()) {
+auto T = D->getType();
+if (T->getContainedAutoType()) {
+  if (!T->isDependentType()) {
 // Our current approach is to place the hint on the variable
 // and accordingly print the full type
 // (e.g. for `const auto& x = 42`, print `const int&`).
 // Alternatively, we could place the hint on the `auto`
 // (and then just print the type deduced for the `auto`).
-addTypeHint(D->getLocation(), D->getType(), /*Prefix=*/": ");
+addTypeHint(D->getLocation(), T, /*Prefix=*/": ");
   }
 }
 
+// Show type hint for decltype, e.g.
+// for `decltype(0) i;`, show `decltype(0) i: int;`
+if (T->isDecltypeType())
+  addTypeHint(D->getLocation(),
+  llvm::cast(T)->getUnderlyingType(),
+  /*Prefix=*/": ");
+
 // Handle templates like `int foo(auto x)` with exactly one instantiation.
 if (auto *PVD = llvm::dyn_cast(D)) {
   if (D->getIdentifier() && PVD->getType()->isDependentType() &&


Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -1141,6 +1141,10 @@
   ExpectedHint{": int &", "z"});
 }
 
+TEST(TypeHints, Decltype) {
+  assertTypeHints("decltype(0) $i[[i]];", ExpectedHint{": int", "i"});
+}
+
 TEST(TypeHints, NoQualifiers) {
   assertTypeHints(R"cpp(
 namespace A {
Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -296,17 +296,25 @@
   return true;
 }
 
-if (D->getType()->getContainedAutoType()) {
-  if (!D->getType()->isDependentType()) {
+auto T = D->getType();
+if (T->getContainedAutoType()) {
+  if (!T->isDependentType()) {
 // Our current approach is to place the hint on the variable
 // and accordingly print the full type
 // (e.g. for `const auto& x = 42`, print `const int&`).
 // Alternatively, we could place the hint on the `auto`
 // (and then just print the type deduced for the `auto`).
-addTypeHint(D->getLocation(), D->getType(), /*Prefix=*/": ");
+addTypeHint(D->getLocation(), T, /*Prefix=*/": ");
   }
 }
 
+// Show type hint for decltype, e.g.
+// for `decltype(0) i;`, show `decltype(0) i: int;`
+if (T->isDecltypeType())
+  addTypeHint(D->getLocation(),
+  llvm::cast(T)->getUnderlyingType(),
+  /*Prefix=*/": ");
+
 // Handle templates like `int foo(auto x)` with exactly one instantiation.
 if (auto *PVD = llvm::dyn_cast(D)) {
   if (D->getIdentifier() && PVD->getType()->isDependentType() &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D138300: [clangd] Support type hints for `decltype(expr)` in variable declarations

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry updated this revision to Diff 476485.
v1nh1shungry added a comment.

Apply the comment's suggestion


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138300

Files:
  clang-tools-extra/clangd/InlayHints.cpp
  clang-tools-extra/clangd/unittests/InlayHintTests.cpp


Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -1141,6 +1141,10 @@
   ExpectedHint{": int &", "z"});
 }
 
+TEST(TypeHints, Decltype) {
+  assertTypeHints("decltype(0) $i[[i]];", ExpectedHint{": int", "i"});
+}
+
 TEST(TypeHints, NoQualifiers) {
   assertTypeHints(R"cpp(
 namespace A {
Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -296,17 +296,24 @@
   return true;
 }
 
-if (D->getType()->getContainedAutoType()) {
-  if (!D->getType()->isDependentType()) {
+auto T = D->getType();
+if (T->getContainedAutoType()) {
+  if (!T->isDependentType()) {
 // Our current approach is to place the hint on the variable
 // and accordingly print the full type
 // (e.g. for `const auto& x = 42`, print `const int&`).
 // Alternatively, we could place the hint on the `auto`
 // (and then just print the type deduced for the `auto`).
-addTypeHint(D->getLocation(), D->getType(), /*Prefix=*/": ");
+addTypeHint(D->getLocation(), T, /*Prefix=*/": ");
   }
 }
 
+// Show type hint for `decltype(expr)`, e.g.
+// for `decltype(0) i;`, show `decltype(0) i: int;`
+if (const auto *DT = llvm::dyn_cast(T))
+  addTypeHint(D->getLocation(), DT->getUnderlyingType(),
+  /*Prefix=*/": ");
+
 // Handle templates like `int foo(auto x)` with exactly one instantiation.
 if (auto *PVD = llvm::dyn_cast(D)) {
   if (D->getIdentifier() && PVD->getType()->isDependentType() &&


Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -1141,6 +1141,10 @@
   ExpectedHint{": int &", "z"});
 }
 
+TEST(TypeHints, Decltype) {
+  assertTypeHints("decltype(0) $i[[i]];", ExpectedHint{": int", "i"});
+}
+
 TEST(TypeHints, NoQualifiers) {
   assertTypeHints(R"cpp(
 namespace A {
Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -296,17 +296,24 @@
   return true;
 }
 
-if (D->getType()->getContainedAutoType()) {
-  if (!D->getType()->isDependentType()) {
+auto T = D->getType();
+if (T->getContainedAutoType()) {
+  if (!T->isDependentType()) {
 // Our current approach is to place the hint on the variable
 // and accordingly print the full type
 // (e.g. for `const auto& x = 42`, print `const int&`).
 // Alternatively, we could place the hint on the `auto`
 // (and then just print the type deduced for the `auto`).
-addTypeHint(D->getLocation(), D->getType(), /*Prefix=*/": ");
+addTypeHint(D->getLocation(), T, /*Prefix=*/": ");
   }
 }
 
+// Show type hint for `decltype(expr)`, e.g.
+// for `decltype(0) i;`, show `decltype(0) i: int;`
+if (const auto *DT = llvm::dyn_cast(T))
+  addTypeHint(D->getLocation(), DT->getUnderlyingType(),
+  /*Prefix=*/": ");
+
 // Handle templates like `int foo(auto x)` with exactly one instantiation.
 if (auto *PVD = llvm::dyn_cast(D)) {
   if (D->getIdentifier() && PVD->getType()->isDependentType() &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D138300: [clangd] Support type hints for `decltype(expr)` in variable declarations

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry marked an inline comment as done.
v1nh1shungry added a comment.

@Trass3r Thank you for the suggestion!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138300

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

[PATCH] D138300: [clangd] Support type hints for `decltype(expr)` in variable declarations

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry updated this revision to Diff 476667.
v1nh1shungry added a comment.

Avoid type hints for dependent types


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138300

Files:
  clang-tools-extra/clangd/InlayHints.cpp
  clang-tools-extra/clangd/unittests/InlayHintTests.cpp


Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -1141,6 +1141,10 @@
   ExpectedHint{": int &", "z"});
 }
 
+TEST(TypeHints, Decltype) {
+  assertTypeHints("decltype(0) $i[[i]];", ExpectedHint{": int", "i"});
+}
+
 TEST(TypeHints, NoQualifiers) {
   assertTypeHints(R"cpp(
 namespace A {
Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -296,14 +296,24 @@
   return true;
 }
 
-if (D->getType()->getContainedAutoType()) {
-  if (!D->getType()->isDependentType()) {
+auto T = D->getType();
+
+// Show type hint for `decltype(expr)`, e.g.
+// for `decltype(0) i;`, show `decltype(0) i: int;`
+if (const auto *DT = llvm::dyn_cast(T)) {
+  if (auto UT = DT->getUnderlyingType(); !UT->isDependentType())
+addTypeHint(D->getLocation(), UT, /*Prefix=*/": ");
+  return true;
+}
+
+if (T->getContainedAutoType()) {
+  if (!T->isDependentType()) {
 // Our current approach is to place the hint on the variable
 // and accordingly print the full type
 // (e.g. for `const auto& x = 42`, print `const int&`).
 // Alternatively, we could place the hint on the `auto`
 // (and then just print the type deduced for the `auto`).
-addTypeHint(D->getLocation(), D->getType(), /*Prefix=*/": ");
+addTypeHint(D->getLocation(), T, /*Prefix=*/": ");
   }
 }
 


Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -1141,6 +1141,10 @@
   ExpectedHint{": int &", "z"});
 }
 
+TEST(TypeHints, Decltype) {
+  assertTypeHints("decltype(0) $i[[i]];", ExpectedHint{": int", "i"});
+}
+
 TEST(TypeHints, NoQualifiers) {
   assertTypeHints(R"cpp(
 namespace A {
Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -296,14 +296,24 @@
   return true;
 }
 
-if (D->getType()->getContainedAutoType()) {
-  if (!D->getType()->isDependentType()) {
+auto T = D->getType();
+
+// Show type hint for `decltype(expr)`, e.g.
+// for `decltype(0) i;`, show `decltype(0) i: int;`
+if (const auto *DT = llvm::dyn_cast(T)) {
+  if (auto UT = DT->getUnderlyingType(); !UT->isDependentType())
+addTypeHint(D->getLocation(), UT, /*Prefix=*/": ");
+  return true;
+}
+
+if (T->getContainedAutoType()) {
+  if (!T->isDependentType()) {
 // Our current approach is to place the hint on the variable
 // and accordingly print the full type
 // (e.g. for `const auto& x = 42`, print `const int&`).
 // Alternatively, we could place the hint on the `auto`
 // (and then just print the type deduced for the `auto`).
-addTypeHint(D->getLocation(), D->getType(), /*Prefix=*/": ");
+addTypeHint(D->getLocation(), T, /*Prefix=*/": ");
   }
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D138300: [clangd] Support type hints for `decltype(expr)` in variable declarations

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry updated this revision to Diff 476692.
v1nh1shungry added a comment.

- fix qualifiers
- fix recursive `decltype` (partly)
- support return type


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138300

Files:
  clang-tools-extra/clangd/InlayHints.cpp
  clang-tools-extra/clangd/unittests/InlayHintTests.cpp

Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -1141,6 +1141,29 @@
   ExpectedHint{": int &", "z"});
 }
 
+TEST(TypeHints, Decltype) {
+  assertTypeHints(R"cpp(
+decltype(0) $a[[a]];
+decltype(a) $b[[b]];
+const decltype(0) $c[[c]] = 0;
+  )cpp",
+  ExpectedHint{": int", "a"}, ExpectedHint{": int", "b"},
+  ExpectedHint{": const int", "c"});
+  assertTypeHints(R"cpp(
+decltype(0) $i[[i]] = 0;
+// FIXME: Nice to show `: int &`
+decltype((i)) $a[[a]] = i;
+  )cpp",
+  ExpectedHint{": int", "i"},
+  ExpectedHint{": decltype(0) &", "a"});
+  // FIXME: Support type hints for l/r reference
+  assertTypeHints(R"cpp(
+int i = 0;
+decltype(0) &$a[[a]] = i;
+decltype(0) &&$b[[b]] = 0;
+  )cpp");
+}
+
 TEST(TypeHints, NoQualifiers) {
   assertTypeHints(R"cpp(
 namespace A {
@@ -1274,6 +1297,8 @@
 
 auto f5($noreturn[[)]] {}
 
+decltype(0) f6($ret3[[)]];
+
 // `auto` conversion operator
 struct A {
   operator auto($retConv[[)]] { return 42; }
@@ -1287,7 +1312,7 @@
   )cpp",
   ExpectedHint{"-> int", "ret1a"}, ExpectedHint{"-> int", "ret1b"},
   ExpectedHint{"-> int &", "ret2"}, ExpectedHint{"-> void", "noreturn"},
-  ExpectedHint{"-> int", "retConv"});
+  ExpectedHint{"-> int", "ret3"}, ExpectedHint{"-> int", "retConv"});
 }
 
 TEST(TypeHints, DependentType) {
Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -279,10 +279,15 @@
   }
 
   void addReturnTypeHint(FunctionDecl *D, SourceRange Range) {
-auto *AT = D->getReturnType()->getContainedAutoType();
-if (!AT || AT->getDeducedType().isNull())
-  return;
-addTypeHint(Range, D->getReturnType(), /*Prefix=*/"-> ");
+auto RT = D->getReturnType();
+
+if (auto *AT = RT->getContainedAutoType();
+AT && !AT->getDeducedType().isNull())
+  addTypeHint(Range, RT, /*Prefix=*/"-> ");
+
+if (llvm::isa(RT))
+  if (auto UT = getUnderlyingType(RT); !UT->isDependentType())
+addTypeHint(Range, UT, /*Prefix=*/"-> ");
   }
 
   bool VisitVarDecl(VarDecl *D) {
@@ -296,14 +301,24 @@
   return true;
 }
 
-if (D->getType()->getContainedAutoType()) {
-  if (!D->getType()->isDependentType()) {
+auto T = D->getType();
+
+// Show type hint for `decltype(expr)`, e.g.
+// for `decltype(0) i;`, show `decltype(0) i: int;`
+if (llvm::isa(T)) {
+  if (auto UT = getUnderlyingType(T); !UT->isDependentType())
+addTypeHint(D->getLocation(), UT, /*Prefix=*/": ");
+  return true;
+}
+
+if (T->getContainedAutoType()) {
+  if (!T->isDependentType()) {
 // Our current approach is to place the hint on the variable
 // and accordingly print the full type
 // (e.g. for `const auto& x = 42`, print `const int&`).
 // Alternatively, we could place the hint on the `auto`
 // (and then just print the type deduced for the `auto`).
-addTypeHint(D->getLocation(), D->getType(), /*Prefix=*/": ");
+addTypeHint(D->getLocation(), T, /*Prefix=*/": ");
   }
 }
 
@@ -375,6 +390,14 @@
 private:
   using NameVec = SmallVector;
 
+  static QualType getUnderlyingType(QualType T) {
+auto LFQ = T.getLocalFastQualifiers();
+while (const auto *DT = dyn_cast(T))
+  T = DT->getUnderlyingType();
+T.setLocalFastQualifiers(LFQ);
+return T;
+  }
+
   void processCall(const FunctionDecl *Callee,
llvm::ArrayRef Args) {
 if (!Cfg.InlayHints.Parameters || Args.size() == 0 || !Callee)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D138425: [clangd] Parameter hints for template specialization

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry created this revision.
v1nh1shungry added reviewers: nridge, sammccall.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
v1nh1shungry requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

Relevant issue: https://github.com/clangd/clangd/issues/1387


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138425

Files:
  clang-tools-extra/clangd/InlayHints.cpp
  clang-tools-extra/clangd/unittests/InlayHintTests.cpp

Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -459,12 +459,13 @@
   foo(args...);
 }
 template <>
-void bar(int b);
+void bar<$par0[[int]]>(int b);
 void baz() {
-  bar($param[[42]]);
+  bar($par1[[42]]);
 }
   )cpp",
-   ExpectedHint{"b: ", "param"});
+   ExpectedHint{"Args[0]: ", "par0"},
+   ExpectedHint{"b: ", "par1"});
 }
 
 TEST(ParameterHints, VariadicNameFromSpecializationRecursive) {
@@ -481,12 +482,13 @@
   foo(args...);
 }
 template <>
-void foo(int b);
+void foo<$par0[[int]]>(int b);
 void baz() {
-  bar($param[[42]]);
+  bar($par1[[42]]);
 }
   )cpp",
-   ExpectedHint{"b: ", "param"});
+   ExpectedHint{"Args[0]: ", "par0"},
+   ExpectedHint{"b: ", "par1"});
 }
 
 TEST(ParameterHints, VariadicOverloaded) {
@@ -671,13 +673,13 @@
   }
 };
 void foo() {
-  container c;
+  container<$param0[[S]]> c;
   c.emplace($param1[[1]]);
   c.emplace($param2[[2]], $param3[[3]]);
 }
   )cpp",
-  ExpectedHint{"A: ", "param1"}, ExpectedHint{"B: ", "param2"},
-  ExpectedHint{"C: ", "param3"});
+  ExpectedHint{"T: ", "param0"}, ExpectedHint{"A: ", "param1"},
+  ExpectedHint{"B: ", "param2"}, ExpectedHint{"C: ", "param3"});
 }
 
 TEST(ParameterHints, VariadicReferenceHint) {
@@ -1113,6 +1115,70 @@
   EXPECT_EQ(hintsOfKind(*AST, InlayHintKind::Parameter).size(), 0u);
 }
 
+TEST(ParameterHints, TemplateSpecialization) {
+  assertParameterHints(
+  R"cpp(
+template  struct A {};
+template  struct B {};
+template  struct C {};
+template  struct D {};
+
+template 
+using E = A;
+template 
+using F = A<$par0[[U]]>;
+template 
+using G = C<$par1[[int]], $par2[[float]], Ts...>;
+
+B b;
+D<$par3[[int]], $par4[[float]]> d;
+  )cpp",
+  ExpectedHint{"T: ", "par0"}, ExpectedHint{"Ts[0]: ", "par1"},
+  ExpectedHint{"Ts[1]: ", "par2"}, ExpectedHint{"[0]: ", "par3"},
+  ExpectedHint{"[1]: ", "par4"});
+}
+
+TEST(ParameterHints, ConceptSpecialization) {
+  assertParameterHints(R"cpp(
+template  concept A = true;
+bool a = A<$par0[[int]]>;
+  )cpp",
+   ExpectedHint{"T: ", "par0"});
+}
+
+TEST(ParameterHints, ClassSpecialization) {
+  assertParameterHints(R"cpp(
+template  struct A {};
+template  struct A<$par0[[int]], $par1[[T]]> {};
+template <> struct A<$par2[[int]], $par3[[float]]> {};
+  )cpp",
+   ExpectedHint{"T: ", "par0"}, ExpectedHint{"U: ", "par1"},
+   ExpectedHint{"T: ", "par2"},
+   ExpectedHint{"U: ", "par3"});
+}
+
+TEST(ParameterHints, VarSpecialization) {
+  assertParameterHints(R"cpp(
+template 
+constexpr int value = 0;
+template 
+constexpr int value<$par0[[int]], $par1[[T]]> = 0;
+template <>
+constexpr int value<$par2[[int]], $par3[[float]]> = 0;
+   )cpp",
+   ExpectedHint{"T: ", "par0"}, ExpectedHint{"U: ", "par1"},
+   ExpectedHint{"T: ", "par2"},
+   ExpectedHint{"U: ", "par3"});
+}
+
+TEST(ParameterHints, FunctionSpecialization) {
+  assertParameterHints(R"cpp(
+template  T add(T lhs, T rhs);
+template <> int add<$par0[[int]]>(int lhs, int rhs);
+  )cpp",
+   ExpectedHint{"T: ", "par0"});
+}
+
 TEST(TypeHints, Smoke) {
   assertTypeHints(R"cpp(
 auto $waldo[[waldo]] = 42;
Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -266,6 +266,11 @@
   addReturnTypeHint(D, FTL.getRParenLoc());
   }
 }
+if (D->isFunctionTemplateSpecialization()) {
+  auto TP = D->getPrimaryTemplate()->getTemplateParameters()->asArray();
+  if (auto *Args = D->getTemplateSpecializationArgsAsWritten())
+addTemplateArgHint(TP, Args->arguments());
+}
 return true;
   }
 
@@ -370,11 +375,80 @@
 return true;
   }
 
+  bool VisitTemplateSpecializationTypeLoc(Templat

[PATCH] D138425: [clangd] Parameter hints for template specialization

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry added a comment.

This patch implements:

- Template specialization type

- Class template specialization declaration

- Variable template specialization declaration

- Function specialization declaration

It would be sweet to also implement:

  template  constexpr int value = 0;
  int I = value;
  
  template  T add(T lhs, T rhs);
  add(1, 2);

But I got stuck on implementing these. I tried `VisitDeclRefExpr` but failed 
and have no idea. I prefer to implement these in future patches. But yes, if 
anyone could give me some hints I am happy to update the patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138425

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

[PATCH] D138425: [clangd] Parameter hints for template specialization

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry added a comment.

@sammccall Thank you for reviewing and giving suggestions!

I must admit I didn't use it for very long. But I do think this is helpful, at 
least for templates I'm unfamiliar with.

Yes, there is a common situation where people use a meaningless template 
parameter name, but I think the same for functions. I have seen many 
meaningless parameter names like `D`, `E` even in the LLVM codebase. Since we 
can tolerate these, why can't we bear the template parameter?

And yes, it is a serious problem this is unconfigurable.




Comment at: clang-tools-extra/clangd/InlayHints.cpp:434
+break;
+  if (auto Name = TP[I]->getName(); shouldHintName(TA[I], Name))
+addInlayHint(TA[I].getSourceRange(), HintSide::Left,

sammccall wrote:
> we're missing some mangling of the name to remove the leading `_`
> (stripLeadingUnderscore?)
I don't think we should, since we don't do this for function parameters, are 
there any special reasons for us to do this for template parameters?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138425

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

[PATCH] D138425: [clangd] Parameter hints for template specialization

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry abandoned this revision.
v1nh1shungry added a comment.

Sorry for the delay.

I'd say I was convinced and about to abandon this patch.

Sincere apologies for wasting everyone's time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138425

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

[PATCH] D138028: [clangd] Fix action `RemoveUsingNamespace` for inline namespace

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry added a comment.

A gentle ping? Sorry if it bothers you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138028

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

[PATCH] D138028: [clangd] Fix action `RemoveUsingNamespace` for inline namespace

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry added a comment.

Oops! Thank you for your thoughtful consideration.

You're right, but I currently get stuck there and need more time. And I prefer 
to clear the existing patches simultaneously if you don't mind.

Or do you think I should merge the modification of this patch into there and 
give up this?




Comment at: 
clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp:212
   // Produce replacements to add the qualifiers.
   std::string Qualifier = printUsingNamespaceName(Ctx, *TargetDirective) + 
"::";
   for (auto Loc : IdentsToQualify) {

v1nh1shungry wrote:
> tom-anders wrote:
> > v1nh1shungry wrote:
> > > We can replace `printUsingNamespaceName` with `printNamespaceScope` here 
> > > so that we can get `a::foobar()` in the test. 
> > > 
> > > However, it can sometimes cause redundancy such as in the 10th test. 
> > > 
> > > And I don't know whether it is worth it. WDYT?
> > Just making sure I understood this correctly:
> > 
> > If you replace `printUsingNamespaceName` with `printNamespaceScope`, then...
> > 
> > - ...in the test you added it would result in `a::foobar()` instead of 
> > `a::b::foobar()` (which is better)
> > - ... but in this test (which is the 10th test if I counted correctly):
> >  
> > ```
> >  namespace a::b { struct Foo {}; }
> >   using namespace a;
> >   using namespace a::[[b]];
> >   using namespace b;
> >   int main() { Foo F;}
> > ```
> > what would be the result..? would you get `a::Foo` instead of `a::b::Foo`?
> > 
> Sorry, I mean the next test. I read `10` from the inlay hint but I forgot the 
> index starts from `0` :(
> 
> The test I want to mention:
> ```
> namespace a::b { struct Foo {}; }
> using namespace a;
> using namespace a::b;
> using namespace [[b]];
> int main() { Foo F;}
> ```
> 
> We will get `a::b::Foo` in both the 10th and 11th tests. So in the 10th test, 
> we don't get any benefits and don't sacrifice anything. In the 11th test, we 
> get more redundancy than the existing version.
> 
> Apologize again for my mistake.
FYI, we have a discussion left here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138028

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

[PATCH] D139705: [clang] fix zero-initialization fix-it for variable template

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry created this revision.
v1nh1shungry added reviewers: aaron.ballman, erichkeane.
Herald added a project: All.
v1nh1shungry requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Current version there is a fix-it for

  cpp
  template  constexpr int x = 0;
  template <> constexpr int x; // fix-it here

but it will cause

  cpp
  template <> constexpr int x = 0;


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139705

Files:
  clang/lib/Sema/SemaInit.cpp
  clang/test/FixIt/fixit-const-var-init.cpp


Index: clang/test/FixIt/fixit-const-var-init.cpp
===
--- /dev/null
+++ clang/test/FixIt/fixit-const-var-init.cpp
@@ -0,0 +1,25 @@
+// RUN: not %clang_cc1 -fdiagnostics-parseable-fixits -x c++ -std=c++14 %s 
2>&1 | FileCheck %s
+
+const int a; // expected-error {{default initialization of an object of const 
type}}
+// CHECK: fix-it:"{{.*}}":{3:12-3:12}:" = 0"
+
+template  const int b; // expected-error {{default 
initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{6:36-6:36}:" = 0"
+
+template  const int b; // expected-error {{default 
initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{9:39-9:39}:" = 0"
+
+template <> const int b; // expected-error {{default 
initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{12:36-12:36}:" = 0"
+
+constexpr float c; // expected-error {{must be initialized by a constant 
expression}}
+// CHECK: fix-it:"{{.*}}":{15:18-15:18}:" = 0.0"
+
+template  constexpr float d; // expected-error {{must be 
initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{18:42-18:42}:" = 0.0"
+
+template  constexpr float d; // expected-error {{must be 
initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{21:45-21:45}:" = 0.0"
+
+template <> constexpr float d; // expected-error {{must be 
initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{24:42-24:42}:" = 0.0"
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -3860,8 +3860,20 @@
   if (VD->getInit() || VD->getEndLoc().isMacroID())
 return false;
 
+  SourceLocation EndLoc = VD->getEndLoc();
+  if (const auto *VTSD = dyn_cast(VD)) {
+if (const auto *VTPSD =
+dyn_cast(VD)) {
+  if (const ASTTemplateArgumentListInfo *Info =
+  VTPSD->getTemplateArgsAsWritten())
+EndLoc = Info->getRAngleLoc();
+}
+if (const ASTTemplateArgumentListInfo *Info = VTSD->getTemplateArgsInfo())
+  EndLoc = Info->getRAngleLoc();
+  }
+
   QualType VariableTy = VD->getType().getCanonicalType();
-  SourceLocation Loc = S.getLocForEndOfToken(VD->getEndLoc());
+  SourceLocation Loc = S.getLocForEndOfToken(EndLoc);
   std::string Init = S.getFixItZeroInitializerForType(VariableTy, Loc);
   if (!Init.empty()) {
 Sequence.AddZeroInitializationStep(Entity.getType());


Index: clang/test/FixIt/fixit-const-var-init.cpp
===
--- /dev/null
+++ clang/test/FixIt/fixit-const-var-init.cpp
@@ -0,0 +1,25 @@
+// RUN: not %clang_cc1 -fdiagnostics-parseable-fixits -x c++ -std=c++14 %s 2>&1 | FileCheck %s
+
+const int a; // expected-error {{default initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{3:12-3:12}:" = 0"
+
+template  const int b; // expected-error {{default initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{6:36-6:36}:" = 0"
+
+template  const int b; // expected-error {{default initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{9:39-9:39}:" = 0"
+
+template <> const int b; // expected-error {{default initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{12:36-12:36}:" = 0"
+
+constexpr float c; // expected-error {{must be initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{15:18-15:18}:" = 0.0"
+
+template  constexpr float d; // expected-error {{must be initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{18:42-18:42}:" = 0.0"
+
+template  constexpr float d; // expected-error {{must be initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{21:45-21:45}:" = 0.0"
+
+template <> constexpr float d; // expected-error {{must be initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{24:42-24:42}:" = 0.0"
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -3860,8 +3860,20 @@
   if (VD->getInit() || VD->getEndLoc().isMacroID())
 return false;
 
+  SourceLocation EndLoc = VD->getEndLoc();
+  if (const auto *VTSD = dyn_cast(VD)) {
+if (const auto *VTPSD =
+dyn_cast(VD)) {
+  if (const ASTTemplateArgumentListInfo *Info =
+  VTPSD->getTemp

[PATCH] D139705: [clang] fix zero-initialization fix-it for variable template

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry updated this revision to Diff 481819.
v1nh1shungry added a comment.

address the comment's suggestion


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139705

Files:
  clang/lib/Sema/SemaInit.cpp
  clang/test/FixIt/fixit-const-var-init.cpp


Index: clang/test/FixIt/fixit-const-var-init.cpp
===
--- /dev/null
+++ clang/test/FixIt/fixit-const-var-init.cpp
@@ -0,0 +1,25 @@
+// RUN: not %clang_cc1 -fdiagnostics-parseable-fixits -x c++ -std=c++14 %s 
2>&1 | FileCheck %s
+
+const int a; // expected-error {{default initialization of an object of const 
type}}
+// CHECK: fix-it:"{{.*}}":{3:12-3:12}:" = 0"
+
+template  const int b; // expected-error {{default 
initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{6:36-6:36}:" = 0"
+
+template  const int b; // expected-error {{default 
initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{9:39-9:39}:" = 0"
+
+template <> const int b; // expected-error {{default 
initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{12:36-12:36}:" = 0"
+
+constexpr float c; // expected-error {{must be initialized by a constant 
expression}}
+// CHECK: fix-it:"{{.*}}":{15:18-15:18}:" = 0.0"
+
+template  constexpr float d; // expected-error {{must be 
initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{18:42-18:42}:" = 0.0"
+
+template  constexpr float d; // expected-error {{must be 
initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{21:45-21:45}:" = 0.0"
+
+template <> constexpr float d; // expected-error {{must be 
initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{24:42-24:42}:" = 0.0"
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -3860,8 +3860,21 @@
   if (VD->getInit() || VD->getEndLoc().isMacroID())
 return false;
 
+  SourceLocation EndLoc = VD->getEndLoc();
+  if (const auto *VTSD = dyn_cast(VD)) {
+if (const auto *VTPSD =
+dyn_cast(VD)) {
+  if (const ASTTemplateArgumentListInfo *Info =
+  VTPSD->getTemplateArgsAsWritten())
+EndLoc = Info->getRAngleLoc();
+} else if (const ASTTemplateArgumentListInfo *Info =
+   VTSD->getTemplateArgsInfo()) {
+  EndLoc = Info->getRAngleLoc();
+}
+  }
+
   QualType VariableTy = VD->getType().getCanonicalType();
-  SourceLocation Loc = S.getLocForEndOfToken(VD->getEndLoc());
+  SourceLocation Loc = S.getLocForEndOfToken(EndLoc);
   std::string Init = S.getFixItZeroInitializerForType(VariableTy, Loc);
   if (!Init.empty()) {
 Sequence.AddZeroInitializationStep(Entity.getType());


Index: clang/test/FixIt/fixit-const-var-init.cpp
===
--- /dev/null
+++ clang/test/FixIt/fixit-const-var-init.cpp
@@ -0,0 +1,25 @@
+// RUN: not %clang_cc1 -fdiagnostics-parseable-fixits -x c++ -std=c++14 %s 2>&1 | FileCheck %s
+
+const int a; // expected-error {{default initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{3:12-3:12}:" = 0"
+
+template  const int b; // expected-error {{default initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{6:36-6:36}:" = 0"
+
+template  const int b; // expected-error {{default initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{9:39-9:39}:" = 0"
+
+template <> const int b; // expected-error {{default initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{12:36-12:36}:" = 0"
+
+constexpr float c; // expected-error {{must be initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{15:18-15:18}:" = 0.0"
+
+template  constexpr float d; // expected-error {{must be initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{18:42-18:42}:" = 0.0"
+
+template  constexpr float d; // expected-error {{must be initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{21:45-21:45}:" = 0.0"
+
+template <> constexpr float d; // expected-error {{must be initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{24:42-24:42}:" = 0.0"
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -3860,8 +3860,21 @@
   if (VD->getInit() || VD->getEndLoc().isMacroID())
 return false;
 
+  SourceLocation EndLoc = VD->getEndLoc();
+  if (const auto *VTSD = dyn_cast(VD)) {
+if (const auto *VTPSD =
+dyn_cast(VD)) {
+  if (const ASTTemplateArgumentListInfo *Info =
+  VTPSD->getTemplateArgsAsWritten())
+EndLoc = Info->getRAngleLoc();
+} else if (const ASTTemplateArgumentListInfo *Info =
+   VTSD->getTemplateArgsInfo()) {
+  EndLoc = Info->getRAngleLoc();
+}
+

[PATCH] D139705: [clang] fix zero-initialization fix-it for variable template

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry added a reviewer: shafik.
v1nh1shungry marked an inline comment as done.
v1nh1shungry added a comment.

@shafik Thank you for reviewing and giving suggestions!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139705

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

[PATCH] D138028: [clangd] Fix action `RemoveUsingNamespace` for inline namespace

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry updated this revision to Diff 482170.
v1nh1shungry added a comment.

address the comment's suggestion


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138028

Files:
  clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
  clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp


Index: clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
@@ -264,6 +264,17 @@
   }
   
   int main() { 1.5_w; }
+)cpp"},
+  {
+  R"cpp(
+  namespace a { inline namespace b { void foobar(); } }
+  using namespace a::[[b]];
+  int main() { foobar(); }
+)cpp",
+  R"cpp(
+  namespace a { inline namespace b { void foobar(); } }
+  
+  int main() { a::b::foobar(); }
 )cpp"}};
   for (auto C : Cases)
 EXPECT_EQ(C.second, apply(C.first)) << C.first;
Index: clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
@@ -89,16 +89,15 @@
   return Node->Parent && Node->Parent->ASTNode.get();
 }
 
-// Returns the first visible context that contains this DeclContext.
-// For example: Returns ns1 for S1 and a.
-// namespace ns1 {
-// inline namespace ns2 { struct S1 {}; }
-// enum E { a, b, c, d };
-// }
-const DeclContext *visibleContext(const DeclContext *D) {
-  while (D->isInlineNamespace() || D->isTransparentContext())
+// Return true if `LHS` is declared in `RHS`
+bool isDeclaredIn(const NamedDecl *LHS, const DeclContext *RHS) {
+  const auto *D = LHS->getDeclContext();
+  while (D->isInlineNamespace() || D->isTransparentContext()) {
+if (D->Equals(RHS))
+  return true;
 D = D->getParent();
-  return D;
+  }
+  return D->Equals(RHS);
 }
 
 bool RemoveUsingNamespace::prepare(const Selection &Inputs) {
@@ -152,8 +151,7 @@
 return; // This reference is already qualified.
 
   for (auto *T : Ref.Targets) {
-if (!visibleContext(T->getDeclContext())
- ->Equals(TargetDirective->getNominatedNamespace()))
+if (!isDeclaredIn(T, TargetDirective->getNominatedNamespace()))
   return;
 auto Kind = T->getDeclName().getNameKind();
 // Avoid adding qualifiers before operators, e.g.


Index: clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
@@ -264,6 +264,17 @@
   }
   
   int main() { 1.5_w; }
+)cpp"},
+  {
+  R"cpp(
+  namespace a { inline namespace b { void foobar(); } }
+  using namespace a::[[b]];
+  int main() { foobar(); }
+)cpp",
+  R"cpp(
+  namespace a { inline namespace b { void foobar(); } }
+  
+  int main() { a::b::foobar(); }
 )cpp"}};
   for (auto C : Cases)
 EXPECT_EQ(C.second, apply(C.first)) << C.first;
Index: clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
@@ -89,16 +89,15 @@
   return Node->Parent && Node->Parent->ASTNode.get();
 }
 
-// Returns the first visible context that contains this DeclContext.
-// For example: Returns ns1 for S1 and a.
-// namespace ns1 {
-// inline namespace ns2 { struct S1 {}; }
-// enum E { a, b, c, d };
-// }
-const DeclContext *visibleContext(const DeclContext *D) {
-  while (D->isInlineNamespace() || D->isTransparentContext())
+// Return true if `LHS` is declared in `RHS`
+bool isDeclaredIn(const NamedDecl *LHS, const DeclContext *RHS) {
+  const auto *D = LHS->getDeclContext();
+  while (D->isInlineNamespace() || D->isTransparentContext()) {
+if (D->Equals(RHS))
+  return true;
 D = D->getParent();
-  return D;
+  }
+  return D->Equals(RHS);
 }
 
 bool RemoveUsingNamespace::prepare(const Selection &Inputs) {
@@ -152,8 +151,7 @@
 return; // This reference is already qualified.
 
   for (auto *T : Ref.Targets) {
-if (!visibleContext(T->getDeclContext())
- ->Equals(TargetDirective->getNominatedNamespace()))
+if (!isDeclaredIn(T, TargetDirective->getNominatedNamespace()))
   return;
 auto Kind = T->getDeclName().getNameKind();

[PATCH] D138028: [clangd] Fix action `RemoveUsingNamespace` for inline namespace

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry marked an inline comment as done.
v1nh1shungry added a comment.

Sorry for the delay! And many thanks for reviewing and giving suggestions!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138028

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