[clang-tools-extra] [clangd] Reduce superfluous rename conflicts (PR #121515)
ujan-r wrote: @llvm-beanz Could you take a look at this? https://github.com/llvm/llvm-project/pull/121515 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Reduce superfluous rename conflicts (PR #121515)
https://github.com/ujan-r created
https://github.com/llvm/llvm-project/pull/121515
This commit adds a namespace check to the code for detecting name collisions,
allowing `bar` to be renamed to `foo` in the following snippet:
```c
typedef struct foo {} Foo;
Foo bar;
```
Previously, such a rename would fail because a declaration for `foo` already
exists in the same scope.
>From 5d86f27c5dc04217252c2569edca23cc0d7edc23 Mon Sep 17 00:00:00 2001
From: Ujan RoyBandyopadhyay <116058173+uja...@users.noreply.github.com>
Date: Thu, 2 Jan 2025 00:20:52 -0600
Subject: [PATCH] [clangd] Reduce superfluous rename conflicts
Check namespaces before reporting a conflict, allowing `bar` to be
renamed to `foo` in the following snippet:
```c
typedef struct foo {} Foo;
Foo bar;
```
Previously, such a rename would fail because a declaration for `foo`
already exists in the same scope.
---
clang-tools-extra/clangd/refactor/Rename.cpp | 14 +---
.../clangd/unittests/RenameTests.cpp | 33 +++
2 files changed, 43 insertions(+), 4 deletions(-)
diff --git a/clang-tools-extra/clangd/refactor/Rename.cpp
b/clang-tools-extra/clangd/refactor/Rename.cpp
index c85e13dbdfe97f..b7894b8918eede 100644
--- a/clang-tools-extra/clangd/refactor/Rename.cpp
+++ b/clang-tools-extra/clangd/refactor/Rename.cpp
@@ -338,7 +338,8 @@ const NamedDecl
*lookupSiblingWithinEnclosingScope(ASTContext &Ctx,
for (const auto &Child : DS->getDeclGroup())
if (const auto *ND = dyn_cast(Child))
if (ND != &RenamedDecl && ND->getDeclName().isIdentifier() &&
-ND->getName() == Name)
+ND->getName() == Name &&
+ND->getIdentifierNamespace() &
RenamedDecl.getIdentifierNamespace())
return ND;
return nullptr;
};
@@ -380,7 +381,9 @@ const NamedDecl
*lookupSiblingWithinEnclosingScope(ASTContext &Ctx,
// Also check if there is a name collision with function arguments.
if (const auto *Function = ScopeParent->get())
for (const auto *Parameter : Function->parameters())
-if (Parameter->getName() == NewName)
+if (Parameter->getName() == NewName &&
+Parameter->getIdentifierNamespace() &
+RenamedDecl.getIdentifierNamespace())
return Parameter;
return nullptr;
}
@@ -405,7 +408,9 @@ const NamedDecl
*lookupSiblingWithinEnclosingScope(ASTContext &Ctx,
if (const auto *EnclosingFunction = Parent->get()) {
// Check for conflicts with other arguments.
for (const auto *Parameter : EnclosingFunction->parameters())
- if (Parameter != &RenamedDecl && Parameter->getName() == NewName)
+ if (Parameter != &RenamedDecl && Parameter->getName() == NewName &&
+ Parameter->getIdentifierNamespace() &
+ RenamedDecl.getIdentifierNamespace())
return Parameter;
// FIXME: We don't modify all references to function parameters when
// renaming from forward declaration now, so using a name colliding with
@@ -450,7 +455,8 @@ const NamedDecl *lookupSiblingsWithinContext(ASTContext
&Ctx,
}
// Lookup may contain the RenameDecl itself, exclude it.
for (const auto *D : LookupResult)
-if (D->getCanonicalDecl() != RenamedDecl.getCanonicalDecl())
+if (D->getCanonicalDecl() != RenamedDecl.getCanonicalDecl() &&
+D->getIdentifierNamespace() & RenamedDecl.getIdentifierNamespace())
return D;
return nullptr;
}
diff --git a/clang-tools-extra/clangd/unittests/RenameTests.cpp
b/clang-tools-extra/clangd/unittests/RenameTests.cpp
index 142ed171d1a1cb..879d76e03f2a6a 100644
--- a/clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ b/clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -1269,6 +1269,39 @@ TEST(RenameTest, Renameable) {
)cpp",
"conflict", !HeaderFile, "Conflict"},
+ {R"cpp(
+struct conflict {};
+enum v^ar {};
+ )cpp",
+ "conflict", !HeaderFile, "conflict"},
+
+ {R"cpp(
+struct conflict {};
+int [[v^ar]];
+ )cpp",
+ nullptr, !HeaderFile, "conflict"},
+
+ {R"cpp(
+enum conflict {};
+int [[v^ar]];
+ )cpp",
+ nullptr, !HeaderFile, "conflict"},
+
+ {R"cpp(
+void func(int conflict) {
+ struct [[t^ag]] {};
+}
+ )cpp",
+ nullptr, !HeaderFile, "conflict"},
+
+ {R"cpp(
+void func(void) {
+ struct conflict {};
+ int [[v^ar]];
+}
+ )cpp",
+ nullptr, !HeaderFile, "conflict"},
+
{R"cpp(
void func(int);
void [[o^therFunc]](double);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Reduce superfluous rename conflicts (PR #121515)
https://github.com/ujan-r updated
https://github.com/llvm/llvm-project/pull/121515
>From 5d86f27c5dc04217252c2569edca23cc0d7edc23 Mon Sep 17 00:00:00 2001
From: Ujan RoyBandyopadhyay <116058173+uja...@users.noreply.github.com>
Date: Thu, 2 Jan 2025 00:20:52 -0600
Subject: [PATCH] [clangd] Reduce superfluous rename conflicts
Check namespaces before reporting a conflict, allowing `bar` to be
renamed to `foo` in the following snippet:
```c
typedef struct foo {} Foo;
Foo bar;
```
Previously, such a rename would fail because a declaration for `foo`
already exists in the same scope.
---
clang-tools-extra/clangd/refactor/Rename.cpp | 14 +---
.../clangd/unittests/RenameTests.cpp | 33 +++
2 files changed, 43 insertions(+), 4 deletions(-)
diff --git a/clang-tools-extra/clangd/refactor/Rename.cpp
b/clang-tools-extra/clangd/refactor/Rename.cpp
index c85e13dbdfe97..b7894b8918eed 100644
--- a/clang-tools-extra/clangd/refactor/Rename.cpp
+++ b/clang-tools-extra/clangd/refactor/Rename.cpp
@@ -338,7 +338,8 @@ const NamedDecl
*lookupSiblingWithinEnclosingScope(ASTContext &Ctx,
for (const auto &Child : DS->getDeclGroup())
if (const auto *ND = dyn_cast(Child))
if (ND != &RenamedDecl && ND->getDeclName().isIdentifier() &&
-ND->getName() == Name)
+ND->getName() == Name &&
+ND->getIdentifierNamespace() &
RenamedDecl.getIdentifierNamespace())
return ND;
return nullptr;
};
@@ -380,7 +381,9 @@ const NamedDecl
*lookupSiblingWithinEnclosingScope(ASTContext &Ctx,
// Also check if there is a name collision with function arguments.
if (const auto *Function = ScopeParent->get())
for (const auto *Parameter : Function->parameters())
-if (Parameter->getName() == NewName)
+if (Parameter->getName() == NewName &&
+Parameter->getIdentifierNamespace() &
+RenamedDecl.getIdentifierNamespace())
return Parameter;
return nullptr;
}
@@ -405,7 +408,9 @@ const NamedDecl
*lookupSiblingWithinEnclosingScope(ASTContext &Ctx,
if (const auto *EnclosingFunction = Parent->get()) {
// Check for conflicts with other arguments.
for (const auto *Parameter : EnclosingFunction->parameters())
- if (Parameter != &RenamedDecl && Parameter->getName() == NewName)
+ if (Parameter != &RenamedDecl && Parameter->getName() == NewName &&
+ Parameter->getIdentifierNamespace() &
+ RenamedDecl.getIdentifierNamespace())
return Parameter;
// FIXME: We don't modify all references to function parameters when
// renaming from forward declaration now, so using a name colliding with
@@ -450,7 +455,8 @@ const NamedDecl *lookupSiblingsWithinContext(ASTContext
&Ctx,
}
// Lookup may contain the RenameDecl itself, exclude it.
for (const auto *D : LookupResult)
-if (D->getCanonicalDecl() != RenamedDecl.getCanonicalDecl())
+if (D->getCanonicalDecl() != RenamedDecl.getCanonicalDecl() &&
+D->getIdentifierNamespace() & RenamedDecl.getIdentifierNamespace())
return D;
return nullptr;
}
diff --git a/clang-tools-extra/clangd/unittests/RenameTests.cpp
b/clang-tools-extra/clangd/unittests/RenameTests.cpp
index 142ed171d1a1c..879d76e03f2a6 100644
--- a/clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ b/clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -1269,6 +1269,39 @@ TEST(RenameTest, Renameable) {
)cpp",
"conflict", !HeaderFile, "Conflict"},
+ {R"cpp(
+struct conflict {};
+enum v^ar {};
+ )cpp",
+ "conflict", !HeaderFile, "conflict"},
+
+ {R"cpp(
+struct conflict {};
+int [[v^ar]];
+ )cpp",
+ nullptr, !HeaderFile, "conflict"},
+
+ {R"cpp(
+enum conflict {};
+int [[v^ar]];
+ )cpp",
+ nullptr, !HeaderFile, "conflict"},
+
+ {R"cpp(
+void func(int conflict) {
+ struct [[t^ag]] {};
+}
+ )cpp",
+ nullptr, !HeaderFile, "conflict"},
+
+ {R"cpp(
+void func(void) {
+ struct conflict {};
+ int [[v^ar]];
+}
+ )cpp",
+ nullptr, !HeaderFile, "conflict"},
+
{R"cpp(
void func(int);
void [[o^therFunc]](double);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Reduce superfluous rename conflicts (PR #121515)
ujan-r wrote: No worries, thanks for the review! I don't have commit access, so you'll have to merge the changes on my behalf. https://github.com/llvm/llvm-project/pull/121515 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Reduce superfluous rename conflicts (PR #121515)
https://github.com/ujan-r updated
https://github.com/llvm/llvm-project/pull/121515
>From e2d63656b94c2a1f9071397abc64d1cdcdb62c75 Mon Sep 17 00:00:00 2001
From: Ujan RoyBandyopadhyay <116058173+uja...@users.noreply.github.com>
Date: Thu, 2 Jan 2025 00:20:52 -0600
Subject: [PATCH] [clangd] Reduce superfluous rename conflicts
Check namespaces before reporting a conflict, allowing `bar` to be
renamed to `foo` in the following snippet:
```c
typedef struct foo {} Foo;
Foo bar;
```
Previously, such a rename would fail because a declaration for `foo`
already exists in the same scope.
---
clang-tools-extra/clangd/refactor/Rename.cpp | 14 +---
.../clangd/unittests/RenameTests.cpp | 33 +++
2 files changed, 43 insertions(+), 4 deletions(-)
diff --git a/clang-tools-extra/clangd/refactor/Rename.cpp
b/clang-tools-extra/clangd/refactor/Rename.cpp
index c85e13dbdfe97..b7894b8918eed 100644
--- a/clang-tools-extra/clangd/refactor/Rename.cpp
+++ b/clang-tools-extra/clangd/refactor/Rename.cpp
@@ -338,7 +338,8 @@ const NamedDecl
*lookupSiblingWithinEnclosingScope(ASTContext &Ctx,
for (const auto &Child : DS->getDeclGroup())
if (const auto *ND = dyn_cast(Child))
if (ND != &RenamedDecl && ND->getDeclName().isIdentifier() &&
-ND->getName() == Name)
+ND->getName() == Name &&
+ND->getIdentifierNamespace() &
RenamedDecl.getIdentifierNamespace())
return ND;
return nullptr;
};
@@ -380,7 +381,9 @@ const NamedDecl
*lookupSiblingWithinEnclosingScope(ASTContext &Ctx,
// Also check if there is a name collision with function arguments.
if (const auto *Function = ScopeParent->get())
for (const auto *Parameter : Function->parameters())
-if (Parameter->getName() == NewName)
+if (Parameter->getName() == NewName &&
+Parameter->getIdentifierNamespace() &
+RenamedDecl.getIdentifierNamespace())
return Parameter;
return nullptr;
}
@@ -405,7 +408,9 @@ const NamedDecl
*lookupSiblingWithinEnclosingScope(ASTContext &Ctx,
if (const auto *EnclosingFunction = Parent->get()) {
// Check for conflicts with other arguments.
for (const auto *Parameter : EnclosingFunction->parameters())
- if (Parameter != &RenamedDecl && Parameter->getName() == NewName)
+ if (Parameter != &RenamedDecl && Parameter->getName() == NewName &&
+ Parameter->getIdentifierNamespace() &
+ RenamedDecl.getIdentifierNamespace())
return Parameter;
// FIXME: We don't modify all references to function parameters when
// renaming from forward declaration now, so using a name colliding with
@@ -450,7 +455,8 @@ const NamedDecl *lookupSiblingsWithinContext(ASTContext
&Ctx,
}
// Lookup may contain the RenameDecl itself, exclude it.
for (const auto *D : LookupResult)
-if (D->getCanonicalDecl() != RenamedDecl.getCanonicalDecl())
+if (D->getCanonicalDecl() != RenamedDecl.getCanonicalDecl() &&
+D->getIdentifierNamespace() & RenamedDecl.getIdentifierNamespace())
return D;
return nullptr;
}
diff --git a/clang-tools-extra/clangd/unittests/RenameTests.cpp
b/clang-tools-extra/clangd/unittests/RenameTests.cpp
index 15866f43affa0..65558440e36e3 100644
--- a/clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ b/clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -1269,6 +1269,39 @@ TEST(RenameTest, Renameable) {
)cpp",
"conflict", !HeaderFile, "Conflict"},
+ {R"cpp(
+struct conflict {};
+enum v^ar {};
+ )cpp",
+ "conflict", !HeaderFile, "conflict"},
+
+ {R"cpp(
+struct conflict {};
+int [[v^ar]];
+ )cpp",
+ nullptr, !HeaderFile, "conflict"},
+
+ {R"cpp(
+enum conflict {};
+int [[v^ar]];
+ )cpp",
+ nullptr, !HeaderFile, "conflict"},
+
+ {R"cpp(
+void func(int conflict) {
+ struct [[t^ag]] {};
+}
+ )cpp",
+ nullptr, !HeaderFile, "conflict"},
+
+ {R"cpp(
+void func(void) {
+ struct conflict {};
+ int [[v^ar]];
+}
+ )cpp",
+ nullptr, !HeaderFile, "conflict"},
+
{R"cpp(
void func(int);
void [[o^therFunc]](double);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
