[PATCH] D135506: [clangd] FindTarget: UsingEnumDecl is not an alias

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2bb34cc462ff: [clangd] FindTarget: UsingEnumDecl is not an 
alias (authored by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135506

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp


Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -279,6 +279,21 @@
   )cpp";
   EXPECT_DECLS("UnresolvedUsingTypeLoc",
{"using typename Foo::foo", Rel::Alias});
+
+  // Using enum.
+  Flags.push_back("-std=c++20");
+  Code = R"cpp(
+namespace ns { enum class A { X }; }
+[[using enum ns::A]];
+  )cpp";
+  EXPECT_DECLS("UsingEnumDecl", "enum class A : int");
+
+  Code = R"cpp(
+namespace ns { enum class A { X }; }
+using enum ns::A;
+auto m = [[X]];
+  )cpp";
+  EXPECT_DECLS("DeclRefExpr", "X");
 }
 
 TEST_F(TargetDeclTest, BaseSpecifier) {
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -189,8 +189,8 @@
 add(S->getUnderlyingDecl(), Flags);
   Flags |= Rel::Alias; // continue with the alias.
 } else if (const UsingEnumDecl *UED = dyn_cast(D)) {
-  add(UED->getEnumDecl(), Flags);
-  Flags |= Rel::Alias; // continue with the alias.
+  // UsingEnumDecl is not an alias at all, just a reference.
+  D = UED->getEnumDecl();
 } else if (const auto *NAD = dyn_cast(D)) {
   add(NAD->getUnderlyingDecl(), Flags | Rel::Underlying);
   Flags |= Rel::Alias; // continue with the alias
@@ -207,9 +207,12 @@
   // templates.
   Flags |= Rel::Alias;
 } else if (const UsingShadowDecl *USD = dyn_cast(D)) {
-  // Include the Introducing decl, but don't traverse it. This may end up
-  // including *all* shadows, which we don't want.
-  report(USD->getIntroducer(), Flags | Rel::Alias);
+  // Include the introducing UsingDecl, but don't traverse it. This may end
+  // up including *all* shadows, which we don't want.
+  // Don't apply this logic to UsingEnumDecl, which can't easily be
+  // conflated with the aliases it introduces.
+  if (llvm::isa(USD->getIntroducer()))
+report(USD->getIntroducer(), Flags | Rel::Alias);
   // Shadow decls are synthetic and not themselves interesting.
   // Record the underlying decl instead, if allowed.
   D = USD->getTargetDecl();


Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -279,6 +279,21 @@
   )cpp";
   EXPECT_DECLS("UnresolvedUsingTypeLoc",
{"using typename Foo::foo", Rel::Alias});
+
+  // Using enum.
+  Flags.push_back("-std=c++20");
+  Code = R"cpp(
+namespace ns { enum class A { X }; }
+[[using enum ns::A]];
+  )cpp";
+  EXPECT_DECLS("UsingEnumDecl", "enum class A : int");
+
+  Code = R"cpp(
+namespace ns { enum class A { X }; }
+using enum ns::A;
+auto m = [[X]];
+  )cpp";
+  EXPECT_DECLS("DeclRefExpr", "X");
 }
 
 TEST_F(TargetDeclTest, BaseSpecifier) {
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -189,8 +189,8 @@
 add(S->getUnderlyingDecl(), Flags);
   Flags |= Rel::Alias; // continue with the alias.
 } else if (const UsingEnumDecl *UED = dyn_cast(D)) {
-  add(UED->getEnumDecl(), Flags);
-  Flags |= Rel::Alias; // continue with the alias.
+  // UsingEnumDecl is not an alias at all, just a reference.
+  D = UED->getEnumDecl();
 } else if (const auto *NAD = dyn_cast(D)) {
   add(NAD->getUnderlyingDecl(), Flags | Rel::Underlying);
   Flags |= Rel::Alias; // continue with the alias
@@ -207,9 +207,12 @@
   // templates.
   Flags |= Rel::Alias;
 } else if (const UsingShadowDecl *USD = dyn_cast(D)) {
-  // Include the Introducing decl, but don't traverse it. This may end up
-  // including *all* shadows, which we don't want.
-  report(USD->getIntroducer(), Flags | Rel::Alias);
+  // Include the introducing UsingDecl, but don't traverse it. This may end
+  // up including *all* shadows, which we don't want.
+  // Don't apply this logic to UsingEnumDecl, which can't easily be
+  // conflated with the aliases it introduces.
+  if (llvm::isa(USD->getIntroducer()))
+

[PATCH] D135506: [clangd] FindTarget: UsingEnumDecl is not an alias

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D135506#3844919 , @tom-anders 
wrote:

> lgtm, thanks! With this change in place, we could probably ajdust 
> `pickDeclToUse` in Hover.cpp (introduced in https://reviews.llvm.org/D133664) 
> to also check for `UsingDecl` instead of `BaseUsingDecl`, right?

Yes, I think so!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135506

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

[PATCH] D135506: [clangd] FindTarget: UsingEnumDecl is not an alias

[Tom Praschan via Phabricator via cfe-commits]

tom-anders accepted this revision.
tom-anders added a comment.
This revision is now accepted and ready to land.

lgtm, thanks! With this change in place, we could probably ajdust 
`pickDeclToUse` in Hover.cpp (introduced in https://reviews.llvm.org/D133664) 
to also check for `UsingDecl` instead of `BaseUsingDecl`, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135506

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

[PATCH] D135506: [clangd] FindTarget: UsingEnumDecl is not an alias

[Sam McCall via Phabricator via cfe-commits]

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

Unlike UsingDecl it doesn't name the UsingShadowDecls it emits, so it doesn't
make sense to consider them the same thing. Don't consider the UsingEnumDecl
a target when the UsingShadowDecl is referenced.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135506

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp


Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -279,6 +279,21 @@
   )cpp";
   EXPECT_DECLS("UnresolvedUsingTypeLoc",
{"using typename Foo::foo", Rel::Alias});
+
+  // Using enum.
+  Flags.push_back("-std=c++20");
+  Code = R"cpp(
+namespace ns { enum class A { X }; }
+[[using enum ns::A]];
+  )cpp";
+  EXPECT_DECLS("UsingEnumDecl", "enum class A : int");
+
+  Code = R"cpp(
+namespace ns { enum class A { X }; }
+using enum ns::A;
+auto m = [[X]];
+  )cpp";
+  EXPECT_DECLS("DeclRefExpr", "X");
 }
 
 TEST_F(TargetDeclTest, BaseSpecifier) {
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -189,8 +189,8 @@
 add(S->getUnderlyingDecl(), Flags);
   Flags |= Rel::Alias; // continue with the alias.
 } else if (const UsingEnumDecl *UED = dyn_cast(D)) {
-  add(UED->getEnumDecl(), Flags);
-  Flags |= Rel::Alias; // continue with the alias.
+  // UsingEnumDecl is not an alias at all, just a reference.
+  D = UED->getEnumDecl();
 } else if (const auto *NAD = dyn_cast(D)) {
   add(NAD->getUnderlyingDecl(), Flags | Rel::Underlying);
   Flags |= Rel::Alias; // continue with the alias
@@ -207,9 +207,12 @@
   // templates.
   Flags |= Rel::Alias;
 } else if (const UsingShadowDecl *USD = dyn_cast(D)) {
-  // Include the Introducing decl, but don't traverse it. This may end up
-  // including *all* shadows, which we don't want.
-  report(USD->getIntroducer(), Flags | Rel::Alias);
+  // Include the introducing UsingDecl, but don't traverse it. This may end
+  // up including *all* shadows, which we don't want.
+  // Don't apply this logic to UsingEnumDecl, which can't easily be
+  // conflated with the aliases it introduces.
+  if (llvm::isa(USD->getIntroducer()))
+report(USD->getIntroducer(), Flags | Rel::Alias);
   // Shadow decls are synthetic and not themselves interesting.
   // Record the underlying decl instead, if allowed.
   D = USD->getTargetDecl();


Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -279,6 +279,21 @@
   )cpp";
   EXPECT_DECLS("UnresolvedUsingTypeLoc",
{"using typename Foo::foo", Rel::Alias});
+
+  // Using enum.
+  Flags.push_back("-std=c++20");
+  Code = R"cpp(
+namespace ns { enum class A { X }; }
+[[using enum ns::A]];
+  )cpp";
+  EXPECT_DECLS("UsingEnumDecl", "enum class A : int");
+
+  Code = R"cpp(
+namespace ns { enum class A { X }; }
+using enum ns::A;
+auto m = [[X]];
+  )cpp";
+  EXPECT_DECLS("DeclRefExpr", "X");
 }
 
 TEST_F(TargetDeclTest, BaseSpecifier) {
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -189,8 +189,8 @@
 add(S->getUnderlyingDecl(), Flags);
   Flags |= Rel::Alias; // continue with the alias.
 } else if (const UsingEnumDecl *UED = dyn_cast(D)) {
-  add(UED->getEnumDecl(), Flags);
-  Flags |= Rel::Alias; // continue with the alias.
+  // UsingEnumDecl is not an alias at all, just a reference.
+  D = UED->getEnumDecl();
 } else if (const auto *NAD = dyn_cast(D)) {
   add(NAD->getUnderlyingDecl(), Flags | Rel::Underlying);
   Flags |= Rel::Alias; // continue with the alias
@@ -207,9 +207,12 @@
   // templates.
   Flags |= Rel::Alias;
 } else if (const UsingShadowDecl *USD = dyn_cast(D)) {
-  // Include the Introducing decl, but don't traverse it. This may end up
-  // including *all* shadows, which we don't want.
-  report(USD->getIntroducer(), Flags | Rel::Alias);
+  // Include the introducing UsingDecl, but don't traverse it.