[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
https://github.com/DavidGoldman created
https://github.com/llvm/llvm-project/pull/76466
This is based on top of #76410
>From 4caf5b3c779bf18236b4b0be5bc7147d10339f2b Mon Sep 17 00:00:00 2001
From: David Goldman
Date: Tue, 26 Dec 2023 15:59:01 -0500
Subject: [PATCH 1/4] [clangd][SymbolCollector] Treat ObjC methods as spelled
We'll treat multi-arg methods as spelled once we have full rename
support for them.
---
.../clangd/index/SymbolCollector.cpp | 6 ++-
.../clangd/unittests/SymbolCollectorTests.cpp | 42 +++
2 files changed, 47 insertions(+), 1 deletion(-)
diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp
b/clang-tools-extra/clangd/index/SymbolCollector.cpp
index 7ef4b15febad22..336bc3506bb360 100644
--- a/clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -174,7 +174,9 @@ bool isSpelled(SourceLocation Loc, const NamedDecl &ND) {
auto Name = ND.getDeclName();
const auto NameKind = Name.getNameKind();
if (NameKind != DeclarationName::Identifier &&
- NameKind != DeclarationName::CXXConstructorName)
+ NameKind != DeclarationName::CXXConstructorName &&
+ NameKind != DeclarationName::ObjCZeroArgSelector &&
+ NameKind != DeclarationName::ObjCOneArgSelector)
return false;
const auto &AST = ND.getASTContext();
const auto &SM = AST.getSourceManager();
@@ -183,6 +185,8 @@ bool isSpelled(SourceLocation Loc, const NamedDecl &ND) {
if (clang::Lexer::getRawToken(Loc, Tok, SM, LO))
return false;
auto StrName = Name.getAsString();
+ if (const auto *MD = dyn_cast(&ND))
+StrName = MD->getSelector().getNameForSlot(0).str();
return clang::Lexer::getSpelling(Tok, SM, LO) == StrName;
}
} // namespace
diff --git a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
index 9cdc57ec01f327..1d4e1c1d75ea23 100644
--- a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -105,6 +105,9 @@ MATCHER(refRange, "") {
MATCHER_P2(OverriddenBy, Subject, Object, "") {
return arg == Relation{Subject.ID, RelationKind::OverriddenBy, Object.ID};
}
+MATCHER(isSpelled, "") {
+ return static_cast(arg.Kind & RefKind::Spelled);
+}
::testing::Matcher &>
haveRanges(const std::vector Ranges) {
return ::testing::UnorderedPointwise(refRange(), Ranges);
@@ -524,6 +527,45 @@ TEST_F(SymbolCollectorTest, templateArgs) {
forCodeCompletion(false);
}
+TEST_F(SymbolCollectorTest, ObjCRefs) {
+ Annotations Header(R"(
+ @interface Person
+ - (void)$talk[[talk]];
+ - (void)$say[[say]]:(id)something;
+ @end
+ @interface Person (Category)
+ - (void)categoryMethod;
+ - (void)multiArg:(id)a method:(id)b;
+ @end
+ )");
+ Annotations Main(R"(
+ @implementation Person
+ - (void)$talk[[talk]] {}
+ - (void)$say[[say]]:(id)something {}
+ @end
+
+ void fff(Person *p) {
+[p $talk[[talk]]];
+[p $say[[say]]:0];
+[p categoryMethod];
+[p multiArg:0 method:0];
+ }
+ )");
+ CollectorOpts.RefFilter = RefKind::All;
+ CollectorOpts.CollectMainFileRefs = true;
+ TestFileName = testPath("test.m");
+ runSymbolCollector(Header.code(), Main.code(),
+ {"-fblocks", "-xobjective-c++", "-Wno-objc-root-class"});
+ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Person::talk").ID,
+ haveRanges(Main.ranges("talk");
+ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Person::say:").ID,
+ haveRanges(Main.ranges("say");
+ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols,
"Person::categoryMethod").ID,
+ ElementsAre(isSpelled();
+ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols,
"Person::multiArg:method:").ID,
+ ElementsAre(Not(isSpelled());
+}
+
TEST_F(SymbolCollectorTest, ObjCSymbols) {
const std::string Header = R"(
@interface Person
>From 1b6a09464ff5c7b1988fcb479d0a4ff876f696e6 Mon Sep 17 00:00:00 2001
From: David Goldman
Date: Tue, 26 Dec 2023 16:12:03 -0500
Subject: [PATCH 2/4] Run clang-format
---
.../clangd/unittests/SymbolCollectorTests.cpp | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
index 1d4e1c1d75ea23..5c20b950e4eac0 100644
--- a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -560,10 +560,12 @@ TEST_F(SymbolCollectorTest, ObjCRefs) {
haveRanges(Main.ranges("talk");
EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Person::say:").ID,
haveRanges(Main.ranges("say");
- EXPECT_T
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
llvmbot wrote:
@llvm/pr-subscribers-clangd
Author: David Goldman (DavidGoldman)
Changes
This is based on top of #76410
---
Patch is 37.52 KiB, truncated to 20.00 KiB below, full version:
https://github.com/llvm/llvm-project/pull/76466.diff
11 Files Affected:
- (modified) clang-tools-extra/clangd/ClangdLSPServer.cpp (+5-2)
- (modified) clang-tools-extra/clangd/ClangdLSPServer.h (+1-1)
- (modified) clang-tools-extra/clangd/Protocol.cpp (+10)
- (modified) clang-tools-extra/clangd/Protocol.h (+8)
- (modified) clang-tools-extra/clangd/SourceCode.cpp (+8)
- (modified) clang-tools-extra/clangd/SourceCode.h (+4)
- (modified) clang-tools-extra/clangd/index/SymbolCollector.cpp (+6-1)
- (modified) clang-tools-extra/clangd/refactor/Rename.cpp (+283-38)
- (modified) clang-tools-extra/clangd/refactor/Rename.h (+37-7)
- (modified) clang-tools-extra/clangd/unittests/RenameTests.cpp (+34-14)
- (modified) clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp (+44)
``diff
diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp
b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index a87da252b7a7e9..0e628cfc71c2de 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -844,14 +844,17 @@ void ClangdLSPServer::onWorkspaceSymbol(
}
void ClangdLSPServer::onPrepareRename(const TextDocumentPositionParams &Params,
- Callback> Reply) {
+
Callback> Reply) {
Server->prepareRename(
Params.textDocument.uri.file(), Params.position, /*NewName*/
std::nullopt,
Opts.Rename,
[Reply = std::move(Reply)](llvm::Expected Result) mutable {
if (!Result)
return Reply(Result.takeError());
-return Reply(std::move(Result->Target));
+PrepareRenameResult PrepareResult;
+PrepareResult.range = Result->Target;
+PrepareResult.placeholder = Result->Placeholder;
+return Reply(std::move(PrepareResult));
});
}
diff --git a/clang-tools-extra/clangd/ClangdLSPServer.h
b/clang-tools-extra/clangd/ClangdLSPServer.h
index 79579c22b788a9..3a60b6e5db86bc 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.h
+++ b/clang-tools-extra/clangd/ClangdLSPServer.h
@@ -134,7 +134,7 @@ class ClangdLSPServer : private ClangdServer::Callbacks,
void onWorkspaceSymbol(const WorkspaceSymbolParams &,
Callback>);
void onPrepareRename(const TextDocumentPositionParams &,
- Callback>);
+ Callback>);
void onRename(const RenameParams &, Callback);
void onHover(const TextDocumentPositionParams &,
Callback>);
diff --git a/clang-tools-extra/clangd/Protocol.cpp
b/clang-tools-extra/clangd/Protocol.cpp
index a6370649f5ad1c..29b99da27101a8 100644
--- a/clang-tools-extra/clangd/Protocol.cpp
+++ b/clang-tools-extra/clangd/Protocol.cpp
@@ -1187,6 +1187,16 @@ bool fromJSON(const llvm::json::Value &Params,
RenameParams &R,
O.map("position", R.position) && O.map("newName", R.newName);
}
+llvm::json::Value toJSON(const PrepareRenameResult &PRR) {
+ if (!PRR.placeholder) {
+return toJSON(PRR.range);
+ }
+ return llvm::json::Object{
+ {"range", toJSON(PRR.range)},
+ {"placeholder", PRR.placeholder},
+ };
+}
+
llvm::json::Value toJSON(const DocumentHighlight &DH) {
return llvm::json::Object{
{"range", toJSON(DH.range)},
diff --git a/clang-tools-extra/clangd/Protocol.h
b/clang-tools-extra/clangd/Protocol.h
index e88c804692568f..18ac530e9c1a0f 100644
--- a/clang-tools-extra/clangd/Protocol.h
+++ b/clang-tools-extra/clangd/Protocol.h
@@ -1436,6 +1436,14 @@ struct RenameParams {
};
bool fromJSON(const llvm::json::Value &, RenameParams &, llvm::json::Path);
+struct PrepareRenameResult {
+ /// Range of the string to rename.
+ Range range;
+ /// Placeholder text to use in the editor, if set.
+ std::optional placeholder;
+};
+llvm::json::Value toJSON(const PrepareRenameResult &PRR);
+
enum class DocumentHighlightKind { Text = 1, Read = 2, Write = 3 };
/// A document highlight is a range inside a text document which deserves
diff --git a/clang-tools-extra/clangd/SourceCode.cpp
b/clang-tools-extra/clangd/SourceCode.cpp
index 835038423fdf37..73e12fc7ebde51 100644
--- a/clang-tools-extra/clangd/SourceCode.cpp
+++ b/clang-tools-extra/clangd/SourceCode.cpp
@@ -1243,6 +1243,14 @@ SourceLocation
translatePreamblePatchLocation(SourceLocation Loc,
return Loc;
}
+clangd::Range tokenRangeForLoc(SourceLocation TokLoc, const SourceManager &SM,
const LangOptions &LangOpts) {
+ auto TokenLength = clang::Lexer::MeasureTokenLength(TokLoc, SM, LangOpts);
+ clangd::Range Result;
+ Result.start = sourceLocToPosition(SM, TokLoc);
+ Result.end = sourceLocToPosition(SM, TokLoc.getLocWithOffset(TokenLength));
+ return Result;
+}
+
clangd::Range rangeTillEOL(llvm::StringRef Code, unsigned HashOffset) {
c
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
https://github.com/DavidGoldman updated
https://github.com/llvm/llvm-project/pull/76466
>From 4caf5b3c779bf18236b4b0be5bc7147d10339f2b Mon Sep 17 00:00:00 2001
From: David Goldman
Date: Tue, 26 Dec 2023 15:59:01 -0500
Subject: [PATCH 1/5] [clangd][SymbolCollector] Treat ObjC methods as spelled
We'll treat multi-arg methods as spelled once we have full rename
support for them.
---
.../clangd/index/SymbolCollector.cpp | 6 ++-
.../clangd/unittests/SymbolCollectorTests.cpp | 42 +++
2 files changed, 47 insertions(+), 1 deletion(-)
diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp
b/clang-tools-extra/clangd/index/SymbolCollector.cpp
index 7ef4b15febad22..336bc3506bb360 100644
--- a/clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -174,7 +174,9 @@ bool isSpelled(SourceLocation Loc, const NamedDecl &ND) {
auto Name = ND.getDeclName();
const auto NameKind = Name.getNameKind();
if (NameKind != DeclarationName::Identifier &&
- NameKind != DeclarationName::CXXConstructorName)
+ NameKind != DeclarationName::CXXConstructorName &&
+ NameKind != DeclarationName::ObjCZeroArgSelector &&
+ NameKind != DeclarationName::ObjCOneArgSelector)
return false;
const auto &AST = ND.getASTContext();
const auto &SM = AST.getSourceManager();
@@ -183,6 +185,8 @@ bool isSpelled(SourceLocation Loc, const NamedDecl &ND) {
if (clang::Lexer::getRawToken(Loc, Tok, SM, LO))
return false;
auto StrName = Name.getAsString();
+ if (const auto *MD = dyn_cast(&ND))
+StrName = MD->getSelector().getNameForSlot(0).str();
return clang::Lexer::getSpelling(Tok, SM, LO) == StrName;
}
} // namespace
diff --git a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
index 9cdc57ec01f327..1d4e1c1d75ea23 100644
--- a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -105,6 +105,9 @@ MATCHER(refRange, "") {
MATCHER_P2(OverriddenBy, Subject, Object, "") {
return arg == Relation{Subject.ID, RelationKind::OverriddenBy, Object.ID};
}
+MATCHER(isSpelled, "") {
+ return static_cast(arg.Kind & RefKind::Spelled);
+}
::testing::Matcher &>
haveRanges(const std::vector Ranges) {
return ::testing::UnorderedPointwise(refRange(), Ranges);
@@ -524,6 +527,45 @@ TEST_F(SymbolCollectorTest, templateArgs) {
forCodeCompletion(false);
}
+TEST_F(SymbolCollectorTest, ObjCRefs) {
+ Annotations Header(R"(
+ @interface Person
+ - (void)$talk[[talk]];
+ - (void)$say[[say]]:(id)something;
+ @end
+ @interface Person (Category)
+ - (void)categoryMethod;
+ - (void)multiArg:(id)a method:(id)b;
+ @end
+ )");
+ Annotations Main(R"(
+ @implementation Person
+ - (void)$talk[[talk]] {}
+ - (void)$say[[say]]:(id)something {}
+ @end
+
+ void fff(Person *p) {
+[p $talk[[talk]]];
+[p $say[[say]]:0];
+[p categoryMethod];
+[p multiArg:0 method:0];
+ }
+ )");
+ CollectorOpts.RefFilter = RefKind::All;
+ CollectorOpts.CollectMainFileRefs = true;
+ TestFileName = testPath("test.m");
+ runSymbolCollector(Header.code(), Main.code(),
+ {"-fblocks", "-xobjective-c++", "-Wno-objc-root-class"});
+ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Person::talk").ID,
+ haveRanges(Main.ranges("talk");
+ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Person::say:").ID,
+ haveRanges(Main.ranges("say");
+ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols,
"Person::categoryMethod").ID,
+ ElementsAre(isSpelled();
+ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols,
"Person::multiArg:method:").ID,
+ ElementsAre(Not(isSpelled());
+}
+
TEST_F(SymbolCollectorTest, ObjCSymbols) {
const std::string Header = R"(
@interface Person
>From 1b6a09464ff5c7b1988fcb479d0a4ff876f696e6 Mon Sep 17 00:00:00 2001
From: David Goldman
Date: Tue, 26 Dec 2023 16:12:03 -0500
Subject: [PATCH 2/5] Run clang-format
---
.../clangd/unittests/SymbolCollectorTests.cpp | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
index 1d4e1c1d75ea23..5c20b950e4eac0 100644
--- a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -560,10 +560,12 @@ TEST_F(SymbolCollectorTest, ObjCRefs) {
haveRanges(Main.ranges("talk");
EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Person::say:").ID,
haveRanges(Main.ranges("say");
- EXPECT_THAT(Refs, Contains(Pair(findSymbol
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
git-clang-format --diff 8b485070844d03cda467e75aa8c924184ba671cf
76385b51db0e33a25df9646eb6d59f04a4f65af7 --
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/ClangdLSPServer.h
clang-tools-extra/clangd/Protocol.cpp clang-tools-extra/clangd/Protocol.h
clang-tools-extra/clangd/SourceCode.cpp clang-tools-extra/clangd/SourceCode.h
clang-tools-extra/clangd/index/SymbolCollector.cpp
clang-tools-extra/clangd/refactor/Rename.cpp
clang-tools-extra/clangd/refactor/Rename.h
clang-tools-extra/clangd/unittests/RenameTests.cpp
clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
``
View the diff from clang-format here.
``diff
diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp
b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index 0e628cfc71..641266b76c 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -843,8 +843,9 @@ void ClangdLSPServer::onWorkspaceSymbol(
});
}
-void ClangdLSPServer::onPrepareRename(const TextDocumentPositionParams &Params,
-
Callback> Reply) {
+void ClangdLSPServer::onPrepareRename(
+const TextDocumentPositionParams &Params,
+Callback> Reply) {
Server->prepareRename(
Params.textDocument.uri.file(), Params.position, /*NewName*/
std::nullopt,
Opts.Rename,
diff --git a/clang-tools-extra/clangd/SourceCode.cpp
b/clang-tools-extra/clangd/SourceCode.cpp
index 73e12fc7eb..ac20ed095b 100644
--- a/clang-tools-extra/clangd/SourceCode.cpp
+++ b/clang-tools-extra/clangd/SourceCode.cpp
@@ -1243,7 +1243,8 @@ SourceLocation
translatePreamblePatchLocation(SourceLocation Loc,
return Loc;
}
-clangd::Range tokenRangeForLoc(SourceLocation TokLoc, const SourceManager &SM,
const LangOptions &LangOpts) {
+clangd::Range tokenRangeForLoc(SourceLocation TokLoc, const SourceManager &SM,
+ const LangOptions &LangOpts) {
auto TokenLength = clang::Lexer::MeasureTokenLength(TokLoc, SM, LangOpts);
clangd::Range Result;
Result.start = sourceLocToPosition(SM, TokLoc);
diff --git a/clang-tools-extra/clangd/SourceCode.h
b/clang-tools-extra/clangd/SourceCode.h
index d671e05aed..efe6d66c16 100644
--- a/clang-tools-extra/clangd/SourceCode.h
+++ b/clang-tools-extra/clangd/SourceCode.h
@@ -338,8 +338,7 @@ inline bool isReservedName(llvm::StringRef Name) {
SourceLocation translatePreamblePatchLocation(SourceLocation Loc,
const SourceManager &SM);
-clangd::Range tokenRangeForLoc(SourceLocation TokLoc,
- const SourceManager &SM,
+clangd::Range tokenRangeForLoc(SourceLocation TokLoc, const SourceManager &SM,
const LangOptions &LangOpts);
/// Returns the range starting at offset and spanning the whole line. Escaped
diff --git a/clang-tools-extra/clangd/refactor/Rename.cpp
b/clang-tools-extra/clangd/refactor/Rename.cpp
index 4f945f74e8..e5686c963a 100644
--- a/clang-tools-extra/clangd/refactor/Rename.cpp
+++ b/clang-tools-extra/clangd/refactor/Rename.cpp
@@ -585,11 +585,9 @@ renameWithinFile(ParsedAST &AST, const NamedDecl
&RenameDecl,
for (const auto &Loc : Locs)
Ranges.push_back(tokenRangeForLoc(Loc, SM, LangOpts));
auto FilePath = AST.tuPath();
-auto RenameRanges =
-adjustRenameRanges(Code, RenameIdentifier,
- std::move(Ranges),
- RenameDecl.getASTContext().getLangOpts(),
- MD->getSelector());
+auto RenameRanges = adjustRenameRanges(
+Code, RenameIdentifier, std::move(Ranges),
+RenameDecl.getASTContext().getLangOpts(), MD->getSelector());
if (!RenameRanges) {
// Our heuristics fails to adjust rename ranges to the current state of
// the file, it is most likely the index is stale, so we give up the
@@ -598,8 +596,7 @@ renameWithinFile(ParsedAST &AST, const NamedDecl
&RenameDecl,
"(selector handling may be incorrect)",
FilePath);
}
-auto RenameEdit =
-buildRenameEdit(FilePath, Code, *RenameRanges, NewNames);
+auto RenameEdit = buildRenameEdit(FilePath, Code, *RenameRanges, NewNames);
if (!RenameEdit)
return error("failed to rename in file {0}: {1}", FilePath,
RenameEdit.takeError());
@@ -736,8 +733,7 @@ renameOutsideFile(const NamedDecl &RenameDecl,
llvm::StringRef MainFilePath,
auto RenameRanges =
adjustRenameRanges(AffectedFileCode, RenameIdentifier,
std::move(FileAndOccurrences.second),
- RenameDecl.getASTContext().getLangOpts(),
- Selector);
+
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
https://github.com/DavidGoldman updated
https://github.com/llvm/llvm-project/pull/76466
>From 4caf5b3c779bf18236b4b0be5bc7147d10339f2b Mon Sep 17 00:00:00 2001
From: David Goldman
Date: Tue, 26 Dec 2023 15:59:01 -0500
Subject: [PATCH 1/6] [clangd][SymbolCollector] Treat ObjC methods as spelled
We'll treat multi-arg methods as spelled once we have full rename
support for them.
---
.../clangd/index/SymbolCollector.cpp | 6 ++-
.../clangd/unittests/SymbolCollectorTests.cpp | 42 +++
2 files changed, 47 insertions(+), 1 deletion(-)
diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp
b/clang-tools-extra/clangd/index/SymbolCollector.cpp
index 7ef4b15febad22..336bc3506bb360 100644
--- a/clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -174,7 +174,9 @@ bool isSpelled(SourceLocation Loc, const NamedDecl &ND) {
auto Name = ND.getDeclName();
const auto NameKind = Name.getNameKind();
if (NameKind != DeclarationName::Identifier &&
- NameKind != DeclarationName::CXXConstructorName)
+ NameKind != DeclarationName::CXXConstructorName &&
+ NameKind != DeclarationName::ObjCZeroArgSelector &&
+ NameKind != DeclarationName::ObjCOneArgSelector)
return false;
const auto &AST = ND.getASTContext();
const auto &SM = AST.getSourceManager();
@@ -183,6 +185,8 @@ bool isSpelled(SourceLocation Loc, const NamedDecl &ND) {
if (clang::Lexer::getRawToken(Loc, Tok, SM, LO))
return false;
auto StrName = Name.getAsString();
+ if (const auto *MD = dyn_cast(&ND))
+StrName = MD->getSelector().getNameForSlot(0).str();
return clang::Lexer::getSpelling(Tok, SM, LO) == StrName;
}
} // namespace
diff --git a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
index 9cdc57ec01f327..1d4e1c1d75ea23 100644
--- a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -105,6 +105,9 @@ MATCHER(refRange, "") {
MATCHER_P2(OverriddenBy, Subject, Object, "") {
return arg == Relation{Subject.ID, RelationKind::OverriddenBy, Object.ID};
}
+MATCHER(isSpelled, "") {
+ return static_cast(arg.Kind & RefKind::Spelled);
+}
::testing::Matcher &>
haveRanges(const std::vector Ranges) {
return ::testing::UnorderedPointwise(refRange(), Ranges);
@@ -524,6 +527,45 @@ TEST_F(SymbolCollectorTest, templateArgs) {
forCodeCompletion(false);
}
+TEST_F(SymbolCollectorTest, ObjCRefs) {
+ Annotations Header(R"(
+ @interface Person
+ - (void)$talk[[talk]];
+ - (void)$say[[say]]:(id)something;
+ @end
+ @interface Person (Category)
+ - (void)categoryMethod;
+ - (void)multiArg:(id)a method:(id)b;
+ @end
+ )");
+ Annotations Main(R"(
+ @implementation Person
+ - (void)$talk[[talk]] {}
+ - (void)$say[[say]]:(id)something {}
+ @end
+
+ void fff(Person *p) {
+[p $talk[[talk]]];
+[p $say[[say]]:0];
+[p categoryMethod];
+[p multiArg:0 method:0];
+ }
+ )");
+ CollectorOpts.RefFilter = RefKind::All;
+ CollectorOpts.CollectMainFileRefs = true;
+ TestFileName = testPath("test.m");
+ runSymbolCollector(Header.code(), Main.code(),
+ {"-fblocks", "-xobjective-c++", "-Wno-objc-root-class"});
+ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Person::talk").ID,
+ haveRanges(Main.ranges("talk");
+ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Person::say:").ID,
+ haveRanges(Main.ranges("say");
+ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols,
"Person::categoryMethod").ID,
+ ElementsAre(isSpelled();
+ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols,
"Person::multiArg:method:").ID,
+ ElementsAre(Not(isSpelled());
+}
+
TEST_F(SymbolCollectorTest, ObjCSymbols) {
const std::string Header = R"(
@interface Person
>From 1b6a09464ff5c7b1988fcb479d0a4ff876f696e6 Mon Sep 17 00:00:00 2001
From: David Goldman
Date: Tue, 26 Dec 2023 16:12:03 -0500
Subject: [PATCH 2/6] Run clang-format
---
.../clangd/unittests/SymbolCollectorTests.cpp | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
index 1d4e1c1d75ea23..5c20b950e4eac0 100644
--- a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -560,10 +560,12 @@ TEST_F(SymbolCollectorTest, ObjCRefs) {
haveRanges(Main.ranges("talk");
EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Person::say:").ID,
haveRanges(Main.ranges("say");
- EXPECT_THAT(Refs, Contains(Pair(findSymbol
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
https://github.com/DavidGoldman updated
https://github.com/llvm/llvm-project/pull/76466
>From 4caf5b3c779bf18236b4b0be5bc7147d10339f2b Mon Sep 17 00:00:00 2001
From: David Goldman
Date: Tue, 26 Dec 2023 15:59:01 -0500
Subject: [PATCH 1/7] [clangd][SymbolCollector] Treat ObjC methods as spelled
We'll treat multi-arg methods as spelled once we have full rename
support for them.
---
.../clangd/index/SymbolCollector.cpp | 6 ++-
.../clangd/unittests/SymbolCollectorTests.cpp | 42 +++
2 files changed, 47 insertions(+), 1 deletion(-)
diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp
b/clang-tools-extra/clangd/index/SymbolCollector.cpp
index 7ef4b15febad22..336bc3506bb360 100644
--- a/clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -174,7 +174,9 @@ bool isSpelled(SourceLocation Loc, const NamedDecl &ND) {
auto Name = ND.getDeclName();
const auto NameKind = Name.getNameKind();
if (NameKind != DeclarationName::Identifier &&
- NameKind != DeclarationName::CXXConstructorName)
+ NameKind != DeclarationName::CXXConstructorName &&
+ NameKind != DeclarationName::ObjCZeroArgSelector &&
+ NameKind != DeclarationName::ObjCOneArgSelector)
return false;
const auto &AST = ND.getASTContext();
const auto &SM = AST.getSourceManager();
@@ -183,6 +185,8 @@ bool isSpelled(SourceLocation Loc, const NamedDecl &ND) {
if (clang::Lexer::getRawToken(Loc, Tok, SM, LO))
return false;
auto StrName = Name.getAsString();
+ if (const auto *MD = dyn_cast(&ND))
+StrName = MD->getSelector().getNameForSlot(0).str();
return clang::Lexer::getSpelling(Tok, SM, LO) == StrName;
}
} // namespace
diff --git a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
index 9cdc57ec01f327..1d4e1c1d75ea23 100644
--- a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -105,6 +105,9 @@ MATCHER(refRange, "") {
MATCHER_P2(OverriddenBy, Subject, Object, "") {
return arg == Relation{Subject.ID, RelationKind::OverriddenBy, Object.ID};
}
+MATCHER(isSpelled, "") {
+ return static_cast(arg.Kind & RefKind::Spelled);
+}
::testing::Matcher &>
haveRanges(const std::vector Ranges) {
return ::testing::UnorderedPointwise(refRange(), Ranges);
@@ -524,6 +527,45 @@ TEST_F(SymbolCollectorTest, templateArgs) {
forCodeCompletion(false);
}
+TEST_F(SymbolCollectorTest, ObjCRefs) {
+ Annotations Header(R"(
+ @interface Person
+ - (void)$talk[[talk]];
+ - (void)$say[[say]]:(id)something;
+ @end
+ @interface Person (Category)
+ - (void)categoryMethod;
+ - (void)multiArg:(id)a method:(id)b;
+ @end
+ )");
+ Annotations Main(R"(
+ @implementation Person
+ - (void)$talk[[talk]] {}
+ - (void)$say[[say]]:(id)something {}
+ @end
+
+ void fff(Person *p) {
+[p $talk[[talk]]];
+[p $say[[say]]:0];
+[p categoryMethod];
+[p multiArg:0 method:0];
+ }
+ )");
+ CollectorOpts.RefFilter = RefKind::All;
+ CollectorOpts.CollectMainFileRefs = true;
+ TestFileName = testPath("test.m");
+ runSymbolCollector(Header.code(), Main.code(),
+ {"-fblocks", "-xobjective-c++", "-Wno-objc-root-class"});
+ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Person::talk").ID,
+ haveRanges(Main.ranges("talk");
+ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Person::say:").ID,
+ haveRanges(Main.ranges("say");
+ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols,
"Person::categoryMethod").ID,
+ ElementsAre(isSpelled();
+ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols,
"Person::multiArg:method:").ID,
+ ElementsAre(Not(isSpelled());
+}
+
TEST_F(SymbolCollectorTest, ObjCSymbols) {
const std::string Header = R"(
@interface Person
>From 1b6a09464ff5c7b1988fcb479d0a4ff876f696e6 Mon Sep 17 00:00:00 2001
From: David Goldman
Date: Tue, 26 Dec 2023 16:12:03 -0500
Subject: [PATCH 2/7] Run clang-format
---
.../clangd/unittests/SymbolCollectorTests.cpp | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
index 1d4e1c1d75ea23..5c20b950e4eac0 100644
--- a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -560,10 +560,12 @@ TEST_F(SymbolCollectorTest, ObjCRefs) {
haveRanges(Main.ranges("talk");
EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Person::say:").ID,
haveRanges(Main.ranges("say");
- EXPECT_THAT(Refs, Contains(Pair(findSymbol
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
https://github.com/kadircet edited https://github.com/llvm/llvm-project/pull/76466 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
https://github.com/kadircet commented: thanks, this LG in direction. only reviewed the pieces leading to in-file rename so far. https://github.com/llvm/llvm-project/pull/76466 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -1187,6 +1187,16 @@ bool fromJSON(const llvm::json::Value &Params,
RenameParams &R,
O.map("position", R.position) && O.map("newName", R.newName);
}
+llvm::json::Value toJSON(const PrepareRenameResult &PRR) {
+ if (!PRR.placeholder) {
kadircet wrote:
nit: braces
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -1436,6 +1436,14 @@ struct RenameParams {
};
bool fromJSON(const llvm::json::Value &, RenameParams &, llvm::json::Path);
+struct PrepareRenameResult {
+ /// Range of the string to rename.
+ Range range;
+ /// Placeholder text to use in the editor, if set.
+ std::optional placeholder;
kadircet wrote:
let's drop `optional` to prevent confusion here, there isn't any difference
between an empty placeholder and nullopt, right?
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -53,13 +55,38 @@ struct RenameInputs {
struct RenameResult {
// The range of the symbol that the user can attempt to rename.
Range Target;
+ // Placeholder text for the rename operation, if set.
+ std::optional Placeholder;
// Rename occurrences for the current main file.
std::vector LocalChanges;
// Complete edits for the rename, including LocalChanges.
// If the full set of changes is unknown, this field is empty.
FileEdits GlobalChanges;
};
+/// Represents a symbol range where the symbol can potentially have multiple
+/// tokens.
+struct SymbolRange {
+ /// All ranges. If multiple, corresponds to an ObjC selector.
kadircet wrote:
maybe also talk about the general case?
```
Ranges for the tokens that make up the symbol's name.
Usually a single range, there can be multiple ranges if the tokens for symbol
are split, e.g. ObjC selectors.
```
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -183,6 +185,8 @@ bool isSpelled(SourceLocation Loc, const NamedDecl &ND) {
if (clang::Lexer::getRawToken(Loc, Tok, SM, LO))
return false;
auto StrName = Name.getAsString();
+ if (const auto *MD = dyn_cast(&ND))
+StrName = MD->getSelector().getNameForSlot(0).str();
kadircet wrote:
nit: we can rewrite as:
```
auto TokSpelling = clang::Lexer::getSpelling(Tok, SM, LO);
if (const auto *MD = dyn_cast(&ND))
return TokSpelling == MD->getSelector().getNameForSlot(0);
return TokSpelling == Name.getAsString();
```
(to prevent a string construction)
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -569,8 +571,43 @@ renameWithinFile(ParsedAST &AST, const NamedDecl
&RenameDecl,
// }
if (!isInsideMainFile(RenameLoc, SM))
continue;
+Locs.push_back(RenameLoc);
+ }
+ if (const auto *MD = dyn_cast(&RenameDecl)) {
kadircet wrote:
let's extract this branch into a separate function?
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -134,7 +134,7 @@ class ClangdLSPServer : private ClangdServer::Callbacks, void onWorkspaceSymbol(const WorkspaceSymbolParams &, Callback>); void onPrepareRename(const TextDocumentPositionParams &, - Callback>); + Callback>); kadircet wrote: you can drop the optional now, we either return an Error or PrepareRenameResult https://github.com/llvm/llvm-project/pull/76466 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -508,7 +508,8 @@ static bool mayBeValidIdentifier(llvm::StringRef Ident) {
!isAsciiIdentifierStart(Ident.front(), AllowDollar))
return false;
for (char C : Ident) {
-if (llvm::isASCII(C) && !isAsciiIdentifierContinue(C, AllowDollar))
+if (llvm::isASCII(C) && !isAsciiIdentifierContinue(C, AllowDollar) &&
kadircet wrote:
nit:
```cpp
if (AllowColon && C == ':')
continue;
if (llvm::isASCII ...)
```
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -338,6 +338,10 @@ inline bool isReservedName(llvm::StringRef Name) {
SourceLocation translatePreamblePatchLocation(SourceLocation Loc,
const SourceManager &SM);
+clangd::Range tokenRangeForLoc(SourceLocation TokLoc,
kadircet wrote:
this isn't used outside rename.cpp, let's move it there?
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -53,13 +55,38 @@ struct RenameInputs {
struct RenameResult {
// The range of the symbol that the user can attempt to rename.
Range Target;
+ // Placeholder text for the rename operation, if set.
+ std::optional Placeholder;
kadircet wrote:
same here, let's drop optional
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -71,8 +98,8 @@ llvm::Expected rename(const RenameInputs &RInputs); /// REQUIRED: Occurrences is sorted and doesn't have duplicated ranges. llvm::Expected buildRenameEdit(llvm::StringRef AbsFilePath, llvm::StringRef InitialCode, - std::vector Occurrences, - llvm::StringRef NewName); + std::vector Occurrences, + llvm::SmallVectorImpl &NewNames); kadircet wrote: why not `llvm::ArrayRef NewNames` ? https://github.com/llvm/llvm-project/pull/76466 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -569,8 +571,43 @@ renameWithinFile(ParsedAST &AST, const NamedDecl
&RenameDecl,
// }
if (!isInsideMainFile(RenameLoc, SM))
continue;
+Locs.push_back(RenameLoc);
+ }
+ if (const auto *MD = dyn_cast(&RenameDecl)) {
+auto Code = SM.getBufferData(SM.getMainFileID());
+auto RenameIdentifier = MD->getSelector().getNameForSlot(0).str();
+llvm::SmallVector NewNames;
+NewName.split(NewNames, ":");
+if (NewNames.empty())
+ NewNames.push_back(NewName);
+std::vector Ranges;
+const auto &LangOpts = RenameDecl.getASTContext().getLangOpts();
+for (const auto &Loc : Locs)
+ Ranges.push_back(tokenRangeForLoc(Loc, SM, LangOpts));
+auto FilePath = AST.tuPath();
+auto RenameRanges =
+adjustRenameRanges(Code, RenameIdentifier,
kadircet wrote:
we don't need to adjust ranges again in here. we're already using ranges from
the AST (not from index)
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -893,22 +964,36 @@ llvm::Expected buildRenameEdit(llvm::StringRef
AbsFilePath,
return LastOffset;
};
- std::vector> OccurrencesOffsets;
- for (const auto &R : Occurrences) {
-auto StartOffset = Offset(R.start);
-if (!StartOffset)
- return StartOffset.takeError();
-auto EndOffset = Offset(R.end);
-if (!EndOffset)
- return EndOffset.takeError();
-OccurrencesOffsets.push_back({*StartOffset, *EndOffset});
+ struct OccurrenceOffset {
+size_t Start;
+size_t End;
+llvm::StringRef NewName;
+
+OccurrenceOffset(size_t Start, size_t End, llvm::StringRef NewName) :
+ Start(Start), End(End), NewName(NewName) {}
+ };
+
+ std::vector OccurrencesOffsets;
+ for (const auto &SR : Occurrences) {
+for (auto It = SR.Ranges.begin(); It != SR.Ranges.end(); ++It) {
kadircet wrote:
nit: `for (auto [Range, NewName] : llvm::zip(SR.Ranges, NewNames))`
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -53,13 +55,38 @@ struct RenameInputs {
struct RenameResult {
// The range of the symbol that the user can attempt to rename.
Range Target;
+ // Placeholder text for the rename operation, if set.
+ std::optional Placeholder;
// Rename occurrences for the current main file.
std::vector LocalChanges;
// Complete edits for the rename, including LocalChanges.
// If the full set of changes is unknown, this field is empty.
FileEdits GlobalChanges;
};
+/// Represents a symbol range where the symbol can potentially have multiple
+/// tokens.
+struct SymbolRange {
+ /// All ranges. If multiple, corresponds to an ObjC selector.
+ std::vector Ranges;
+
+ /// Returns the first range.
+ Range range() const { return Ranges.front(); }
+
+ SymbolRange(Range R) : Ranges({R}) {}
+ SymbolRange(std::vector Ranges) : Ranges(std::move(Ranges)) {}
+
+ friend bool operator==(const SymbolRange &LHS, const SymbolRange &RHS) {
+return LHS.Ranges == RHS.Ranges;
+ }
+ friend bool operator!=(const SymbolRange &LHS, const SymbolRange &RHS) {
+return !(LHS == RHS);
+ }
+ friend bool operator<(const SymbolRange &LHS, const SymbolRange &RHS) {
+return LHS.range() < RHS.range();
+ }
+};
kadircet wrote:
can you move function definitions to implementation file?
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -569,8 +571,43 @@ renameWithinFile(ParsedAST &AST, const NamedDecl
&RenameDecl,
// }
if (!isInsideMainFile(RenameLoc, SM))
continue;
+Locs.push_back(RenameLoc);
+ }
+ if (const auto *MD = dyn_cast(&RenameDecl)) {
+auto Code = SM.getBufferData(SM.getMainFileID());
+auto RenameIdentifier = MD->getSelector().getNameForSlot(0).str();
+llvm::SmallVector NewNames;
+NewName.split(NewNames, ":");
+if (NewNames.empty())
+ NewNames.push_back(NewName);
+std::vector Ranges;
+const auto &LangOpts = RenameDecl.getASTContext().getLangOpts();
+for (const auto &Loc : Locs)
+ Ranges.push_back(tokenRangeForLoc(Loc, SM, LangOpts));
kadircet wrote:
instead of re-lexing, you can use tokenbuffer inside AST and call
`spelledTokenAt`, then convert it into a range
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -1436,6 +1436,14 @@ struct RenameParams {
};
bool fromJSON(const llvm::json::Value &, RenameParams &, llvm::json::Path);
+struct PrepareRenameResult {
+ /// Range of the string to rename.
+ Range range;
+ /// Placeholder text to use in the editor, if set.
+ std::optional placeholder;
DavidGoldman wrote:
I think it'll be encoded slightly different but VS Code did appear to handle an
empty placeholder properly, but I'll have clangd omit encoding it if empty to
be safe.
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -893,22 +964,36 @@ llvm::Expected buildRenameEdit(llvm::StringRef
AbsFilePath,
return LastOffset;
};
- std::vector> OccurrencesOffsets;
- for (const auto &R : Occurrences) {
-auto StartOffset = Offset(R.start);
-if (!StartOffset)
- return StartOffset.takeError();
-auto EndOffset = Offset(R.end);
-if (!EndOffset)
- return EndOffset.takeError();
-OccurrencesOffsets.push_back({*StartOffset, *EndOffset});
+ struct OccurrenceOffset {
+size_t Start;
+size_t End;
+llvm::StringRef NewName;
+
+OccurrenceOffset(size_t Start, size_t End, llvm::StringRef NewName) :
+ Start(Start), End(End), NewName(NewName) {}
+ };
+
+ std::vector OccurrencesOffsets;
+ for (const auto &SR : Occurrences) {
+for (auto It = SR.Ranges.begin(); It != SR.Ranges.end(); ++It) {
DavidGoldman wrote:
Not sure I follow, the loop construction doesn't use NewNames at all, you might
have misread it?
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
https://github.com/DavidGoldman updated
https://github.com/llvm/llvm-project/pull/76466
>From 4caf5b3c779bf18236b4b0be5bc7147d10339f2b Mon Sep 17 00:00:00 2001
From: David Goldman
Date: Tue, 26 Dec 2023 15:59:01 -0500
Subject: [PATCH 1/3] [clangd][SymbolCollector] Treat ObjC methods as spelled
We'll treat multi-arg methods as spelled once we have full rename
support for them.
---
.../clangd/index/SymbolCollector.cpp | 6 ++-
.../clangd/unittests/SymbolCollectorTests.cpp | 42 +++
2 files changed, 47 insertions(+), 1 deletion(-)
diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp
b/clang-tools-extra/clangd/index/SymbolCollector.cpp
index 7ef4b15febad22..336bc3506bb360 100644
--- a/clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -174,7 +174,9 @@ bool isSpelled(SourceLocation Loc, const NamedDecl &ND) {
auto Name = ND.getDeclName();
const auto NameKind = Name.getNameKind();
if (NameKind != DeclarationName::Identifier &&
- NameKind != DeclarationName::CXXConstructorName)
+ NameKind != DeclarationName::CXXConstructorName &&
+ NameKind != DeclarationName::ObjCZeroArgSelector &&
+ NameKind != DeclarationName::ObjCOneArgSelector)
return false;
const auto &AST = ND.getASTContext();
const auto &SM = AST.getSourceManager();
@@ -183,6 +185,8 @@ bool isSpelled(SourceLocation Loc, const NamedDecl &ND) {
if (clang::Lexer::getRawToken(Loc, Tok, SM, LO))
return false;
auto StrName = Name.getAsString();
+ if (const auto *MD = dyn_cast(&ND))
+StrName = MD->getSelector().getNameForSlot(0).str();
return clang::Lexer::getSpelling(Tok, SM, LO) == StrName;
}
} // namespace
diff --git a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
index 9cdc57ec01f327..1d4e1c1d75ea23 100644
--- a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -105,6 +105,9 @@ MATCHER(refRange, "") {
MATCHER_P2(OverriddenBy, Subject, Object, "") {
return arg == Relation{Subject.ID, RelationKind::OverriddenBy, Object.ID};
}
+MATCHER(isSpelled, "") {
+ return static_cast(arg.Kind & RefKind::Spelled);
+}
::testing::Matcher &>
haveRanges(const std::vector Ranges) {
return ::testing::UnorderedPointwise(refRange(), Ranges);
@@ -524,6 +527,45 @@ TEST_F(SymbolCollectorTest, templateArgs) {
forCodeCompletion(false);
}
+TEST_F(SymbolCollectorTest, ObjCRefs) {
+ Annotations Header(R"(
+ @interface Person
+ - (void)$talk[[talk]];
+ - (void)$say[[say]]:(id)something;
+ @end
+ @interface Person (Category)
+ - (void)categoryMethod;
+ - (void)multiArg:(id)a method:(id)b;
+ @end
+ )");
+ Annotations Main(R"(
+ @implementation Person
+ - (void)$talk[[talk]] {}
+ - (void)$say[[say]]:(id)something {}
+ @end
+
+ void fff(Person *p) {
+[p $talk[[talk]]];
+[p $say[[say]]:0];
+[p categoryMethod];
+[p multiArg:0 method:0];
+ }
+ )");
+ CollectorOpts.RefFilter = RefKind::All;
+ CollectorOpts.CollectMainFileRefs = true;
+ TestFileName = testPath("test.m");
+ runSymbolCollector(Header.code(), Main.code(),
+ {"-fblocks", "-xobjective-c++", "-Wno-objc-root-class"});
+ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Person::talk").ID,
+ haveRanges(Main.ranges("talk");
+ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Person::say:").ID,
+ haveRanges(Main.ranges("say");
+ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols,
"Person::categoryMethod").ID,
+ ElementsAre(isSpelled();
+ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols,
"Person::multiArg:method:").ID,
+ ElementsAre(Not(isSpelled());
+}
+
TEST_F(SymbolCollectorTest, ObjCSymbols) {
const std::string Header = R"(
@interface Person
>From 1b6a09464ff5c7b1988fcb479d0a4ff876f696e6 Mon Sep 17 00:00:00 2001
From: David Goldman
Date: Tue, 26 Dec 2023 16:12:03 -0500
Subject: [PATCH 2/3] Run clang-format
---
.../clangd/unittests/SymbolCollectorTests.cpp | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
index 1d4e1c1d75ea23..5c20b950e4eac0 100644
--- a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -560,10 +560,12 @@ TEST_F(SymbolCollectorTest, ObjCRefs) {
haveRanges(Main.ranges("talk");
EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Person::say:").ID,
haveRanges(Main.ranges("say");
- EXPECT_THAT(Refs, Contains(Pair(findSymbol
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
https://github.com/ahoppen commented:
The outstanding comments from last review are:
- Use `SymbolName` instead of `StringRef`
- https://github.com/llvm/llvm-project/pull/76466#discussion_r1476409221
- https://github.com/llvm/llvm-project/pull/76466#discussion_r1476427027
- Don’t re-lex the source file in `collectRenameIdentifierRanges`
- https://github.com/llvm/llvm-project/pull/76466#discussion_r1476471971
- Use index or AST data to find edits in the current file. AFAICT we do use
index-data for the multi-file rename case in `adjustRenameRanges` but I AFAICT
no semantic information is used for the current file.
- https://github.com/llvm/llvm-project/pull/76466/files#r1479029824
- I think a good test case to add would be
```cpp
// Input
R"cpp(
@interface Foo
- (void)performA^ction:(int)action with:(int)value;
@end
@implementation Foo
- (void)performAction:(int)action with:(int)value {
[self performAction:action with:value];
}
@end
@interface Bar
- (void)performAction:(int)action with:(int)value;
@end
@implementation Bar
- (void)performAction:(int)action with:(int)value {
}
@end
)cpp",
// New name
"performNewAction:by:",
// Expected
R"cpp(
@interface Foo
- (void)performNewAction:(int)action by:(int)value;
@end
@implementation Foo
- (void)performNewAction:(int)action by:(int)value {
[self performNewAction:action by:value];
}
@end
@interface Bar
- (void)performAction:(int)action with:(int)value;
@end
@implementation Bar
- (void)performAction:(int)action with:(int)value {
}
)cpp",
```
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
https://github.com/ahoppen edited https://github.com/llvm/llvm-project/pull/76466 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -538,11 +564,222 @@ std::optional checkName(const NamedDecl
&RenameDecl,
Conflict->getLocation().printToString(ASTCtx.getSourceManager())};
}
}
- if (Result)
+ if (Result) {
InvalidNameMetric.record(1, toString(Result->K));
+return makeError(*Result);
+ }
+ return llvm::Error::success();
+}
+
+bool isSelectorLike(const syntax::Token &Cur, const syntax::Token &Next) {
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ // We require the selector name and : to be contiguous.
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+bool isMatchingSelectorName(const syntax::Token &Cur, const syntax::Token
&Next,
+const SourceManager &SM,
+llvm::StringRef SelectorName) {
+ if (SelectorName.empty())
+return Cur.kind() == tok::colon;
+ return isSelectorLike(Cur, Next) && Cur.text(SM) == SelectorName;
+}
+
+// Scan through Tokens to find ranges for each selector fragment in Sel at the
+// top level (not nested in any () or {} or []). The search will terminate upon
ahoppen wrote:
I think the `not nested in any () or {} or []` is quite misleading. Based on
this comment, I would have expected `doStuff` in `[someObject someArgLabel:
[foo doStuff: 1]]` or
```objectivec
- (void) test {
[foo doStuff: 1];
}
```
to not be found because it’s nested in `{}` or `[]`.
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -538,11 +564,222 @@ std::optional checkName(const NamedDecl
&RenameDecl,
Conflict->getLocation().printToString(ASTCtx.getSourceManager())};
}
}
- if (Result)
+ if (Result) {
InvalidNameMetric.record(1, toString(Result->K));
+return makeError(*Result);
+ }
+ return llvm::Error::success();
+}
+
+bool isSelectorLike(const syntax::Token &Cur, const syntax::Token &Next) {
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ // We require the selector name and : to be contiguous.
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+bool isMatchingSelectorName(const syntax::Token &Cur, const syntax::Token
&Next,
+const SourceManager &SM,
+llvm::StringRef SelectorName) {
+ if (SelectorName.empty())
+return Cur.kind() == tok::colon;
+ return isSelectorLike(Cur, Next) && Cur.text(SM) == SelectorName;
+}
+
+// Scan through Tokens to find ranges for each selector fragment in Sel at the
+// top level (not nested in any () or {} or []). The search will terminate upon
+// seeing Terminator or a ; at the top level.
+std::optional
+findAllSelectorPieces(llvm::ArrayRef Tokens,
+ const SourceManager &SM, Selector Sel,
+ tok::TokenKind Terminator) {
+
+ unsigned NumArgs = Sel.getNumArgs();
+ llvm::SmallVector Closes;
+ std::vector SelectorPieces;
+ unsigned Last = Tokens.size() - 1;
+ for (unsigned Index = 0; Index < Last; ++Index) {
+const auto &Tok = Tokens[Index];
+
+if (Closes.empty()) {
+ auto PieceCount = SelectorPieces.size();
+ if (PieceCount < NumArgs &&
+ isMatchingSelectorName(Tok, Tokens[Index + 1], SM,
+ Sel.getNameForSlot(PieceCount))) {
+// If 'foo:' instead of ':' (empty selector), we need to skip the ':'
+// token after the name.
+if (!Sel.getNameForSlot(PieceCount).empty())
+ ++Index;
+SelectorPieces.push_back(
+halfOpenToRange(SM, Tok.range(SM).toCharRange(SM)));
+continue;
+ }
+ // If we've found all pieces but the current token looks like another
+ // selector piece, it means the method being renamed is a strict prefix
of
+ // the selector we've found - should be skipped.
+ if (SelectorPieces.size() >= NumArgs &&
+ isSelectorLike(Tok, Tokens[Index + 1]))
+return std::nullopt;
+}
+
+if (Closes.empty() && Tok.kind() == Terminator)
+ return SelectorPieces.size() == NumArgs
+ ? std::optional(SymbolRange(SelectorPieces))
+ : std::nullopt;
+
+switch (Tok.kind()) {
+case tok::l_square:
+ Closes.push_back(tok::r_square);
+ break;
+case tok::l_paren:
+ Closes.push_back(tok::r_paren);
+ break;
+case tok::l_brace:
+ Closes.push_back(tok::r_brace);
+ break;
+case tok::r_square:
+case tok::r_paren:
+case tok::r_brace:
+ if (Closes.empty() || Closes.back() != Tok.kind())
+return std::nullopt;
+ Closes.pop_back();
+ break;
+case tok::semi:
+ // top level ; terminates all statements.
+ if (Closes.empty())
+return SelectorPieces.size() == NumArgs
+ ? std::optional(SymbolRange(SelectorPieces))
+ : std::nullopt;
+ break;
+default:
+ break;
+}
+ }
+ return std::nullopt;
+}
+
+/// Collects all ranges of the given identifier/selector in the source code.
+///
+/// If a selector is given, this does a full lex of the given source code in
+/// order to identify all selector fragments (e.g. in method exprs/decls) since
+/// they are non-contiguous.
+std::vector collectRenameIdentifierRanges(
+llvm::StringRef Identifier, llvm::StringRef Content,
+const LangOptions &LangOpts, std::optional Selector) {
+ std::vector Ranges;
+ if (!Selector) {
+auto IdentifierRanges =
+collectIdentifierRanges(Identifier, Content, LangOpts);
+for (const auto &R : IdentifierRanges)
+ Ranges.emplace_back(R);
+return Ranges;
+ }
+ // FIXME: InMemoryFileAdapter crashes unless the buffer is null terminated!
ahoppen wrote:
In my rename PR I found that this is no longer an issue. See
https://github.com/apple/llvm-project/pull/7915#discussion_r1442402332
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -538,11 +564,222 @@ std::optional checkName(const NamedDecl
&RenameDecl,
Conflict->getLocation().printToString(ASTCtx.getSourceManager())};
}
}
- if (Result)
+ if (Result) {
InvalidNameMetric.record(1, toString(Result->K));
+return makeError(*Result);
+ }
+ return llvm::Error::success();
+}
+
+bool isSelectorLike(const syntax::Token &Cur, const syntax::Token &Next) {
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ // We require the selector name and : to be contiguous.
+ // e.g. support `foo:` but not `foo :`.
ahoppen wrote:
Why don’t we support `foo : `? Or what am I missing here? `[bar foo : 1]` is
perfectly valid AFAICT.
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
https://github.com/kadircet edited https://github.com/llvm/llvm-project/pull/76466 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -538,11 +564,222 @@ std::optional checkName(const NamedDecl
&RenameDecl,
Conflict->getLocation().printToString(ASTCtx.getSourceManager())};
}
}
- if (Result)
+ if (Result) {
InvalidNameMetric.record(1, toString(Result->K));
+return makeError(*Result);
+ }
+ return llvm::Error::success();
+}
+
+bool isSelectorLike(const syntax::Token &Cur, const syntax::Token &Next) {
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ // We require the selector name and : to be contiguous.
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+bool isMatchingSelectorName(const syntax::Token &Cur, const syntax::Token
&Next,
+const SourceManager &SM,
+llvm::StringRef SelectorName) {
+ if (SelectorName.empty())
+return Cur.kind() == tok::colon;
+ return isSelectorLike(Cur, Next) && Cur.text(SM) == SelectorName;
+}
+
+// Scan through Tokens to find ranges for each selector fragment in Sel at the
+// top level (not nested in any () or {} or []). The search will terminate upon
kadircet wrote:
but that's true for this function, as it's achieved by the outer loop that
calls this function in `collectRenameIdentifierRanges`.
in here we're only trying to match `Sel` assuming its first segment is located
at `Tokens.front()`.
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
https://github.com/kadircet approved this pull request. thanks, lgtm! https://github.com/llvm/llvm-project/pull/76466 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -538,11 +564,222 @@ std::optional checkName(const NamedDecl
&RenameDecl,
Conflict->getLocation().printToString(ASTCtx.getSourceManager())};
}
}
- if (Result)
+ if (Result) {
InvalidNameMetric.record(1, toString(Result->K));
+return makeError(*Result);
+ }
+ return llvm::Error::success();
+}
+
+bool isSelectorLike(const syntax::Token &Cur, const syntax::Token &Next) {
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ // We require the selector name and : to be contiguous.
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+bool isMatchingSelectorName(const syntax::Token &Cur, const syntax::Token
&Next,
+const SourceManager &SM,
+llvm::StringRef SelectorName) {
+ if (SelectorName.empty())
+return Cur.kind() == tok::colon;
+ return isSelectorLike(Cur, Next) && Cur.text(SM) == SelectorName;
+}
+
+// Scan through Tokens to find ranges for each selector fragment in Sel at the
+// top level (not nested in any () or {} or []). The search will terminate upon
+// seeing Terminator or a ; at the top level.
+std::optional
+findAllSelectorPieces(llvm::ArrayRef Tokens,
+ const SourceManager &SM, Selector Sel,
+ tok::TokenKind Terminator) {
+
kadircet wrote:
`assert(!Tokens.empty())`
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -696,7 +982,7 @@ renameOutsideFile(const NamedDecl &RenameDecl, llvm::StringRef MainFilePath, FilePath); } auto RenameEdit = -buildRenameEdit(FilePath, AffectedFileCode, *RenameRanges, NewName); +buildRenameEdit(FilePath, AffectedFileCode, *RenameRanges, NewNames); kadircet wrote: as discussed yesterday, i was suggesting to move: ``` llvm::SmallVector NewNames; NewName.split(NewNames, ":"); ``` from lines 928/936 to right before this call to `buildRenameEdit` https://github.com/llvm/llvm-project/pull/76466 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -538,11 +564,222 @@ std::optional checkName(const NamedDecl
&RenameDecl,
Conflict->getLocation().printToString(ASTCtx.getSourceManager())};
}
}
- if (Result)
+ if (Result) {
InvalidNameMetric.record(1, toString(Result->K));
+return makeError(*Result);
+ }
+ return llvm::Error::success();
+}
+
+bool isSelectorLike(const syntax::Token &Cur, const syntax::Token &Next) {
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ // We require the selector name and : to be contiguous.
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+bool isMatchingSelectorName(const syntax::Token &Cur, const syntax::Token
&Next,
+const SourceManager &SM,
+llvm::StringRef SelectorName) {
+ if (SelectorName.empty())
+return Cur.kind() == tok::colon;
+ return isSelectorLike(Cur, Next) && Cur.text(SM) == SelectorName;
+}
+
+// Scan through Tokens to find ranges for each selector fragment in Sel at the
+// top level (not nested in any () or {} or []). The search will terminate upon
+// seeing Terminator or a ; at the top level.
+std::optional
+findAllSelectorPieces(llvm::ArrayRef Tokens,
+ const SourceManager &SM, Selector Sel,
+ tok::TokenKind Terminator) {
+
+ unsigned NumArgs = Sel.getNumArgs();
+ llvm::SmallVector Closes;
+ std::vector SelectorPieces;
+ unsigned Last = Tokens.size() - 1;
+ for (unsigned Index = 0; Index < Last; ++Index) {
+const auto &Tok = Tokens[Index];
+
+if (Closes.empty()) {
+ auto PieceCount = SelectorPieces.size();
+ if (PieceCount < NumArgs &&
+ isMatchingSelectorName(Tok, Tokens[Index + 1], SM,
+ Sel.getNameForSlot(PieceCount))) {
+// If 'foo:' instead of ':' (empty selector), we need to skip the ':'
kadircet wrote:
as discussed we aren't actually handling this case probably (we won't have a
token for an empty selector name).
can you just leave a comment explaining what & why it doesn't work (i don't
necessarily see it as a TODO, as utility/complexity ratio is pretty low)?
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -538,11 +564,222 @@ std::optional checkName(const NamedDecl
&RenameDecl,
Conflict->getLocation().printToString(ASTCtx.getSourceManager())};
}
}
- if (Result)
+ if (Result) {
InvalidNameMetric.record(1, toString(Result->K));
+return makeError(*Result);
+ }
+ return llvm::Error::success();
+}
+
+bool isSelectorLike(const syntax::Token &Cur, const syntax::Token &Next) {
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ // We require the selector name and : to be contiguous.
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+bool isMatchingSelectorName(const syntax::Token &Cur, const syntax::Token
&Next,
+const SourceManager &SM,
+llvm::StringRef SelectorName) {
+ if (SelectorName.empty())
+return Cur.kind() == tok::colon;
+ return isSelectorLike(Cur, Next) && Cur.text(SM) == SelectorName;
+}
+
+// Scan through Tokens to find ranges for each selector fragment in Sel at the
+// top level (not nested in any () or {} or []). The search will terminate upon
+// seeing Terminator or a ; at the top level.
+std::optional
+findAllSelectorPieces(llvm::ArrayRef Tokens,
+ const SourceManager &SM, Selector Sel,
+ tok::TokenKind Terminator) {
+
+ unsigned NumArgs = Sel.getNumArgs();
+ llvm::SmallVector Closes;
+ std::vector SelectorPieces;
+ unsigned Last = Tokens.size() - 1;
kadircet wrote:
nit: `Last` is not used outside the loop, and it's still confusing me to see
loop condition below as `Index < Last`. what about
`for (unsigned Index = 0, Last = Tokens.size(); Index < Last - 1; ++Index)`
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
https://github.com/DavidGoldman closed https://github.com/llvm/llvm-project/pull/76466 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -508,24 +513,46 @@ static bool mayBeValidIdentifier(llvm::StringRef Ident) {
!isAsciiIdentifierStart(Ident.front(), AllowDollar))
return false;
for (char C : Ident) {
+if (AllowColon && C == ':')
+ continue;
if (llvm::isASCII(C) && !isAsciiIdentifierContinue(C, AllowDollar))
return false;
}
return true;
}
+std::string getName(const NamedDecl &RenameDecl) {
+ if (const auto *MD = dyn_cast(&RenameDecl))
+return MD->getSelector().getAsString();
+ if (const auto *ID = RenameDecl.getIdentifier())
+return ID->getName().str();
+ return "";
+}
+
// Check if we can rename the given RenameDecl into NewName.
// Return details if the rename would produce a conflict.
-std::optional checkName(const NamedDecl &RenameDecl,
- llvm::StringRef NewName) {
+std::optional checkName(const NamedDecl &RenameDecl,
kadircet wrote:
no need for optional, you can just return `Error::success()` in the happy path
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -538,11 +565,254 @@ std::optional checkName(const NamedDecl
&RenameDecl,
Conflict->getLocation().printToString(ASTCtx.getSourceManager())};
}
}
- if (Result)
+ if (Result) {
InvalidNameMetric.record(1, toString(Result->K));
+return makeError(*Result);
+ }
+ return std::nullopt;
+}
+
+bool isMatchingSelectorName(const syntax::Token &Cur, const syntax::Token
&Next,
+const SourceManager &SM,
+llvm::StringRef SelectorName) {
+ if (SelectorName.empty())
+return Cur.kind() == tok::colon;
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ Cur.text(SM) == SelectorName &&
+ // We require the selector name and : to be contiguous to avoid
+ // potential conflicts with ternary expression.
+ //
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+bool isSelectorLike(const syntax::Token &Cur, const syntax::Token &Next) {
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ // We require the selector name and : to be contiguous.
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+bool parseMessageExpression(llvm::ArrayRef Tokens,
+const SourceManager &SM, unsigned Index,
+unsigned Last, Selector Sel,
+std::vector &SelectorPieces) {
+
+ unsigned NumArgs = Sel.getNumArgs();
+ llvm::SmallVector Closes;
+ SelectorPieces.clear();
+ while (Index < Last) {
+const auto &Tok = Tokens[Index];
+
+if (Closes.empty()) {
+ auto PieceCount = SelectorPieces.size();
+ if (PieceCount < NumArgs &&
+ isMatchingSelectorName(Tok, Tokens[Index + 1], SM,
+ Sel.getNameForSlot(PieceCount))) {
+// If 'foo:' instead of ':' (empty selector), we need to skip the ':'
+// token after the name.
+if (!Sel.getNameForSlot(PieceCount).empty()) {
+ ++Index;
+}
+SelectorPieces.push_back(
+halfOpenToRange(SM, Tok.range(SM).toCharRange(SM)));
+continue;
+ }
+ // If we've found all pieces but the current token looks like another
+ // selector piece, it means the method being renamed is a strict prefix
of
+ // the selector we've found - should be skipped.
+ if (SelectorPieces.size() >= NumArgs &&
+ isSelectorLike(Tok, Tokens[Index + 1]))
+return false;
+}
+
+switch (Tok.kind()) {
+case tok::l_square:
+ Closes.push_back(']');
+ break;
+case tok::l_paren:
+ Closes.push_back(')');
+ break;
+case tok::l_brace:
+ Closes.push_back('}');
+ break;
+case tok::r_square:
+ if (Closes.empty())
+return SelectorPieces.size() == NumArgs;
+
+ if (Closes.back() != ']')
+return false;
+ Closes.pop_back();
+ break;
+case tok::r_paren:
+ if (Closes.empty() || Closes.back() != ')')
+return false;
+ Closes.pop_back();
+ break;
+case tok::r_brace:
+ if (Closes.empty() || Closes.back() != '}')
+return false;
+ Closes.pop_back();
+ break;
+case tok::semi:
+ // top level ; ends all statements.
+ if (Closes.empty())
+return false;
+ break;
+default:
+ break;
+}
+
+++Index;
+ }
+ return false;
+}
+
+/// Collects all ranges of the given identifier/selector in the source code.
+///
+/// If a selector is given, this does a full lex of the given source code in
+/// order to identify all selector fragments (e.g. in method exprs/decls) since
+/// they are non-contiguous.
+std::vector collectRenameIdentifierRanges(
+llvm::StringRef Identifier, llvm::StringRef Content,
+const LangOptions &LangOpts, std::optional Selector) {
+ std::vector Ranges;
+ if (!Selector) {
+auto IdentifierRanges =
+collectIdentifierRanges(Identifier, Content, LangOpts);
+for (const auto &R : IdentifierRanges)
+ Ranges.emplace_back(R);
+return Ranges;
+ }
+ // FIXME: InMemoryFileAdapter crashes unless the buffer is null terminated!
+ std::string NullTerminatedCode = Content.str();
+ SourceManagerForFile FileSM("mock_file_name.cpp", NullTerminatedCode);
+ auto &SM = FileSM.get();
+
+ // We track parens and braces to ensure that we don't accidentally try
parsing
+ // a method declaration or definition which isn't at the top level or similar
+ // looking expressions (e.g. an @selector() expression).
+ unsigned ParenCount = 0;
+ unsigned BraceCount = 0;
+ unsigned NumArgs = Selector->getNumArgs();
+
+ std::vector SelectorPieces;
+ auto Tokens = syntax::tokenize(SM.getMainFileID(), SM, LangOpts);
+ unsigned Last = Tokens.size() - 1;
+ for (unsigned Index = 0; Index < Last; ++Index) {
+const auto &Tok = Tokens[Index];
+
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -681,12 +957,22 @@ renameOutsideFile(const NamedDecl &RenameDecl,
llvm::StringRef MainFilePath,
ExpBuffer.getError().message());
continue;
}
+std::string RenameIdentifier = RenameDecl.getNameAsString();
+std::optional Selector = std::nullopt;
+llvm::SmallVector NewNames;
+if (const auto *MD = dyn_cast(&RenameDecl)) {
kadircet wrote:
can we just push this logic all the way into `collectRenameIdentifierRanges`?
it already performs a different logic when we have a selector set, we can just
tart by setting `Identifier = Selector.getNameForSlot(0)`
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -538,11 +565,254 @@ std::optional checkName(const NamedDecl
&RenameDecl,
Conflict->getLocation().printToString(ASTCtx.getSourceManager())};
}
}
- if (Result)
+ if (Result) {
InvalidNameMetric.record(1, toString(Result->K));
+return makeError(*Result);
+ }
+ return std::nullopt;
+}
+
+bool isMatchingSelectorName(const syntax::Token &Cur, const syntax::Token
&Next,
+const SourceManager &SM,
+llvm::StringRef SelectorName) {
+ if (SelectorName.empty())
+return Cur.kind() == tok::colon;
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ Cur.text(SM) == SelectorName &&
+ // We require the selector name and : to be contiguous to avoid
+ // potential conflicts with ternary expression.
+ //
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+bool isSelectorLike(const syntax::Token &Cur, const syntax::Token &Next) {
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ // We require the selector name and : to be contiguous.
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+bool parseMessageExpression(llvm::ArrayRef Tokens,
+const SourceManager &SM, unsigned Index,
+unsigned Last, Selector Sel,
+std::vector &SelectorPieces) {
+
+ unsigned NumArgs = Sel.getNumArgs();
+ llvm::SmallVector Closes;
+ SelectorPieces.clear();
+ while (Index < Last) {
+const auto &Tok = Tokens[Index];
+
+if (Closes.empty()) {
+ auto PieceCount = SelectorPieces.size();
+ if (PieceCount < NumArgs &&
+ isMatchingSelectorName(Tok, Tokens[Index + 1], SM,
+ Sel.getNameForSlot(PieceCount))) {
+// If 'foo:' instead of ':' (empty selector), we need to skip the ':'
+// token after the name.
+if (!Sel.getNameForSlot(PieceCount).empty()) {
+ ++Index;
+}
+SelectorPieces.push_back(
+halfOpenToRange(SM, Tok.range(SM).toCharRange(SM)));
+continue;
+ }
+ // If we've found all pieces but the current token looks like another
+ // selector piece, it means the method being renamed is a strict prefix
of
+ // the selector we've found - should be skipped.
+ if (SelectorPieces.size() >= NumArgs &&
+ isSelectorLike(Tok, Tokens[Index + 1]))
+return false;
+}
+
+switch (Tok.kind()) {
+case tok::l_square:
+ Closes.push_back(']');
+ break;
+case tok::l_paren:
+ Closes.push_back(')');
+ break;
+case tok::l_brace:
+ Closes.push_back('}');
+ break;
+case tok::r_square:
+ if (Closes.empty())
+return SelectorPieces.size() == NumArgs;
+
+ if (Closes.back() != ']')
+return false;
+ Closes.pop_back();
+ break;
+case tok::r_paren:
+ if (Closes.empty() || Closes.back() != ')')
+return false;
+ Closes.pop_back();
+ break;
+case tok::r_brace:
+ if (Closes.empty() || Closes.back() != '}')
+return false;
+ Closes.pop_back();
+ break;
+case tok::semi:
+ // top level ; ends all statements.
+ if (Closes.empty())
+return false;
+ break;
+default:
+ break;
+}
+
+++Index;
+ }
+ return false;
+}
+
+/// Collects all ranges of the given identifier/selector in the source code.
+///
+/// If a selector is given, this does a full lex of the given source code in
+/// order to identify all selector fragments (e.g. in method exprs/decls) since
+/// they are non-contiguous.
+std::vector collectRenameIdentifierRanges(
+llvm::StringRef Identifier, llvm::StringRef Content,
+const LangOptions &LangOpts, std::optional Selector) {
+ std::vector Ranges;
+ if (!Selector) {
+auto IdentifierRanges =
+collectIdentifierRanges(Identifier, Content, LangOpts);
+for (const auto &R : IdentifierRanges)
+ Ranges.emplace_back(R);
+return Ranges;
+ }
+ // FIXME: InMemoryFileAdapter crashes unless the buffer is null terminated!
+ std::string NullTerminatedCode = Content.str();
+ SourceManagerForFile FileSM("mock_file_name.cpp", NullTerminatedCode);
+ auto &SM = FileSM.get();
+
+ // We track parens and braces to ensure that we don't accidentally try
parsing
+ // a method declaration or definition which isn't at the top level or similar
+ // looking expressions (e.g. an @selector() expression).
+ unsigned ParenCount = 0;
+ unsigned BraceCount = 0;
+ unsigned NumArgs = Selector->getNumArgs();
+
+ std::vector SelectorPieces;
+ auto Tokens = syntax::tokenize(SM.getMainFileID(), SM, LangOpts);
+ unsigned Last = Tokens.size() - 1;
+ for (unsigned Index = 0; Index < Last; ++Index) {
+const auto &Tok = Tokens[Index];
+
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -543,6 +550,45 @@ std::optional checkName(const NamedDecl
&RenameDecl,
return Result;
}
+clangd::Range tokenRangeForLoc(ParsedAST &AST, SourceLocation TokLoc,
+ const SourceManager &SM,
+ const LangOptions &LangOpts) {
+ const auto *Token = AST.getTokens().spelledTokenAt(TokLoc);
+ assert(Token && "got inclusion at wrong offset");
+ clangd::Range Result;
+ Result.start = sourceLocToPosition(SM, Token->location());
+ Result.end = sourceLocToPosition(SM, Token->endLocation());
+ return Result;
+}
+
+// AST-based ObjC method rename, it renames all occurrences in the main file
+// even for selectors which may have multiple tokens.
+llvm::Expected
+renameObjCMethodWithinFile(ParsedAST &AST, const ObjCMethodDecl *MD,
+ llvm::StringRef NewName,
+ std::vector Locs) {
+ const SourceManager &SM = AST.getSourceManager();
+ auto Code = SM.getBufferData(SM.getMainFileID());
+ auto RenameIdentifier = MD->getSelector().getNameForSlot(0).str();
+ llvm::SmallVector NewNames;
+ NewName.split(NewNames, ":");
+ if (NewNames.empty())
+NewNames.push_back(NewName);
+
+ std::vector Ranges;
+ const auto &LangOpts = MD->getASTContext().getLangOpts();
+ for (const auto &Loc : Locs)
+Ranges.push_back(tokenRangeForLoc(AST, Loc, SM, LangOpts));
+ auto FilePath = AST.tuPath();
+ auto RenameRanges = collectRenameIdentifierRanges(
kadircet wrote:
well you know how I hate complexity :)) so I am definitely in favor of leaving
it as-is.
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -696,7 +982,7 @@ renameOutsideFile(const NamedDecl &RenameDecl, llvm::StringRef MainFilePath, FilePath); } auto RenameEdit = -buildRenameEdit(FilePath, AffectedFileCode, *RenameRanges, NewName); +buildRenameEdit(FilePath, AffectedFileCode, *RenameRanges, NewNames); kadircet wrote: can you also move the logic that splits `NewName` into `NewNames` here? with a comment explaining why it's done for objc selectors? https://github.com/llvm/llvm-project/pull/76466 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -538,11 +565,254 @@ std::optional checkName(const NamedDecl
&RenameDecl,
Conflict->getLocation().printToString(ASTCtx.getSourceManager())};
}
}
- if (Result)
+ if (Result) {
InvalidNameMetric.record(1, toString(Result->K));
+return makeError(*Result);
+ }
+ return std::nullopt;
+}
+
+bool isMatchingSelectorName(const syntax::Token &Cur, const syntax::Token
&Next,
+const SourceManager &SM,
+llvm::StringRef SelectorName) {
+ if (SelectorName.empty())
+return Cur.kind() == tok::colon;
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ Cur.text(SM) == SelectorName &&
+ // We require the selector name and : to be contiguous to avoid
+ // potential conflicts with ternary expression.
+ //
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+bool isSelectorLike(const syntax::Token &Cur, const syntax::Token &Next) {
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ // We require the selector name and : to be contiguous.
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+bool parseMessageExpression(llvm::ArrayRef Tokens,
+const SourceManager &SM, unsigned Index,
+unsigned Last, Selector Sel,
+std::vector &SelectorPieces) {
kadircet wrote:
can we return `std::vector` instead? indicating failure with an empty
vector?
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -538,11 +565,254 @@ std::optional checkName(const NamedDecl
&RenameDecl,
Conflict->getLocation().printToString(ASTCtx.getSourceManager())};
}
}
- if (Result)
+ if (Result) {
InvalidNameMetric.record(1, toString(Result->K));
+return makeError(*Result);
+ }
+ return std::nullopt;
+}
+
+bool isMatchingSelectorName(const syntax::Token &Cur, const syntax::Token
&Next,
+const SourceManager &SM,
+llvm::StringRef SelectorName) {
+ if (SelectorName.empty())
+return Cur.kind() == tok::colon;
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ Cur.text(SM) == SelectorName &&
+ // We require the selector name and : to be contiguous to avoid
+ // potential conflicts with ternary expression.
+ //
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+bool isSelectorLike(const syntax::Token &Cur, const syntax::Token &Next) {
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ // We require the selector name and : to be contiguous.
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+bool parseMessageExpression(llvm::ArrayRef Tokens,
+const SourceManager &SM, unsigned Index,
+unsigned Last, Selector Sel,
+std::vector &SelectorPieces) {
+
+ unsigned NumArgs = Sel.getNumArgs();
+ llvm::SmallVector Closes;
+ SelectorPieces.clear();
+ while (Index < Last) {
+const auto &Tok = Tokens[Index];
+
+if (Closes.empty()) {
+ auto PieceCount = SelectorPieces.size();
+ if (PieceCount < NumArgs &&
+ isMatchingSelectorName(Tok, Tokens[Index + 1], SM,
+ Sel.getNameForSlot(PieceCount))) {
+// If 'foo:' instead of ':' (empty selector), we need to skip the ':'
+// token after the name.
+if (!Sel.getNameForSlot(PieceCount).empty()) {
+ ++Index;
+}
+SelectorPieces.push_back(
+halfOpenToRange(SM, Tok.range(SM).toCharRange(SM)));
+continue;
+ }
+ // If we've found all pieces but the current token looks like another
+ // selector piece, it means the method being renamed is a strict prefix
of
+ // the selector we've found - should be skipped.
+ if (SelectorPieces.size() >= NumArgs &&
+ isSelectorLike(Tok, Tokens[Index + 1]))
+return false;
+}
+
+switch (Tok.kind()) {
+case tok::l_square:
+ Closes.push_back(']');
+ break;
+case tok::l_paren:
+ Closes.push_back(')');
+ break;
+case tok::l_brace:
+ Closes.push_back('}');
+ break;
+case tok::r_square:
+ if (Closes.empty())
+return SelectorPieces.size() == NumArgs;
+
+ if (Closes.back() != ']')
+return false;
+ Closes.pop_back();
+ break;
+case tok::r_paren:
+ if (Closes.empty() || Closes.back() != ')')
+return false;
+ Closes.pop_back();
+ break;
+case tok::r_brace:
+ if (Closes.empty() || Closes.back() != '}')
+return false;
+ Closes.pop_back();
+ break;
+case tok::semi:
+ // top level ; ends all statements.
+ if (Closes.empty())
+return false;
+ break;
+default:
+ break;
+}
+
+++Index;
+ }
+ return false;
+}
+
+/// Collects all ranges of the given identifier/selector in the source code.
+///
+/// If a selector is given, this does a full lex of the given source code in
+/// order to identify all selector fragments (e.g. in method exprs/decls) since
+/// they are non-contiguous.
+std::vector collectRenameIdentifierRanges(
+llvm::StringRef Identifier, llvm::StringRef Content,
+const LangOptions &LangOpts, std::optional Selector) {
+ std::vector Ranges;
+ if (!Selector) {
+auto IdentifierRanges =
+collectIdentifierRanges(Identifier, Content, LangOpts);
+for (const auto &R : IdentifierRanges)
+ Ranges.emplace_back(R);
+return Ranges;
+ }
+ // FIXME: InMemoryFileAdapter crashes unless the buffer is null terminated!
+ std::string NullTerminatedCode = Content.str();
+ SourceManagerForFile FileSM("mock_file_name.cpp", NullTerminatedCode);
+ auto &SM = FileSM.get();
+
+ // We track parens and braces to ensure that we don't accidentally try
parsing
+ // a method declaration or definition which isn't at the top level or similar
+ // looking expressions (e.g. an @selector() expression).
+ unsigned ParenCount = 0;
+ unsigned BraceCount = 0;
+ unsigned NumArgs = Selector->getNumArgs();
+
+ std::vector SelectorPieces;
+ auto Tokens = syntax::tokenize(SM.getMainFileID(), SM, LangOpts);
+ unsigned Last = Tokens.size() - 1;
+ for (unsigned Index = 0; Index < Last; ++Index) {
+const auto &Tok = Tokens[Index];
+
+if (BraceCou
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -538,11 +565,254 @@ std::optional checkName(const NamedDecl
&RenameDecl,
Conflict->getLocation().printToString(ASTCtx.getSourceManager())};
}
}
- if (Result)
+ if (Result) {
InvalidNameMetric.record(1, toString(Result->K));
+return makeError(*Result);
+ }
+ return std::nullopt;
+}
+
+bool isMatchingSelectorName(const syntax::Token &Cur, const syntax::Token
&Next,
+const SourceManager &SM,
+llvm::StringRef SelectorName) {
+ if (SelectorName.empty())
+return Cur.kind() == tok::colon;
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ Cur.text(SM) == SelectorName &&
+ // We require the selector name and : to be contiguous to avoid
+ // potential conflicts with ternary expression.
+ //
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+bool isSelectorLike(const syntax::Token &Cur, const syntax::Token &Next) {
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ // We require the selector name and : to be contiguous.
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+bool parseMessageExpression(llvm::ArrayRef Tokens,
ahoppen wrote:
I think a doc comment of what this method does on a high level would be very
helpful.
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -681,12 +957,22 @@ renameOutsideFile(const NamedDecl &RenameDecl,
llvm::StringRef MainFilePath,
ExpBuffer.getError().message());
continue;
}
+std::string RenameIdentifier = RenameDecl.getNameAsString();
+std::optional Selector = std::nullopt;
+llvm::SmallVector NewNames;
+if (const auto *MD = dyn_cast(&RenameDecl)) {
ahoppen wrote:
A high-level comment. Instead of dealing with a vector of `StringRef` for
`NewNames`, I think we should use `SymbolName`. It is already designed to be
able to represent Objective-C selectors and that way you could do the name
splitting in `SymbolName`.
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
ahoppen wrote: I think we could also include my tests from https://github.com/llvm/llvm-project/pull/78872/files#diff-26ff7c74af8a1d882abbad43625d3cd4bf8d024ef5c7064993f5ede3eec8752eR834 More tests never hurt. https://github.com/llvm/llvm-project/pull/76466 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -681,12 +957,22 @@ renameOutsideFile(const NamedDecl &RenameDecl, llvm::StringRef MainFilePath, ExpBuffer.getError().message()); continue; } +std::string RenameIdentifier = RenameDecl.getNameAsString(); ahoppen wrote: Technically `RenameIdentifier` isn’t correct here because this can be an Objective-C selector name, right? Which would be multiple identifiers and colons. https://github.com/llvm/llvm-project/pull/76466 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -508,24 +513,46 @@ static bool mayBeValidIdentifier(llvm::StringRef Ident) {
!isAsciiIdentifierStart(Ident.front(), AllowDollar))
return false;
for (char C : Ident) {
+if (AllowColon && C == ':')
+ continue;
if (llvm::isASCII(C) && !isAsciiIdentifierContinue(C, AllowDollar))
return false;
}
return true;
}
+std::string getName(const NamedDecl &RenameDecl) {
+ if (const auto *MD = dyn_cast(&RenameDecl))
+return MD->getSelector().getAsString();
+ if (const auto *ID = RenameDecl.getIdentifier())
+return ID->getName().str();
+ return "";
+}
+
// Check if we can rename the given RenameDecl into NewName.
// Return details if the rename would produce a conflict.
-std::optional checkName(const NamedDecl &RenameDecl,
- llvm::StringRef NewName) {
+std::optional checkName(const NamedDecl &RenameDecl,
+ llvm::StringRef NewName,
+ llvm::StringRef OldName) {
trace::Span Tracer("CheckName");
static constexpr trace::Metric InvalidNameMetric(
"rename_name_invalid", trace::Metric::Counter, "invalid_kind");
+
+ if (OldName == NewName)
+return makeError(ReasonToReject::SameName);
+
+ if (const auto *MD = dyn_cast(&RenameDecl)) {
+const auto Sel = MD->getSelector();
+if (Sel.getNumArgs() != NewName.count(':') &&
+NewName != "__clangd_rename_placeholder")
+ return makeError(InvalidName{InvalidName::BadIdentifier, NewName.str()});
ahoppen wrote:
This seems awfully ad-hoc to check for `__clangd_rename_placeholder`. I would
prefer to change the prepare rename request to just change one of the selector
pieces to `__clangd_rename_placeholder`.
I’ve been doing this here
https://github.com/llvm/llvm-project/pull/78872/files#diff-26ff7c74af8a1d882abbad43625d3cd4bf8d024ef5c7064993f5ede3eec8752eR817-R829
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -538,11 +565,254 @@ std::optional checkName(const NamedDecl
&RenameDecl,
Conflict->getLocation().printToString(ASTCtx.getSourceManager())};
}
}
- if (Result)
+ if (Result) {
InvalidNameMetric.record(1, toString(Result->K));
+return makeError(*Result);
+ }
+ return std::nullopt;
+}
+
+bool isMatchingSelectorName(const syntax::Token &Cur, const syntax::Token
&Next,
+const SourceManager &SM,
+llvm::StringRef SelectorName) {
+ if (SelectorName.empty())
+return Cur.kind() == tok::colon;
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ Cur.text(SM) == SelectorName &&
+ // We require the selector name and : to be contiguous to avoid
+ // potential conflicts with ternary expression.
+ //
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+bool isSelectorLike(const syntax::Token &Cur, const syntax::Token &Next) {
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ // We require the selector name and : to be contiguous.
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+bool parseMessageExpression(llvm::ArrayRef Tokens,
+const SourceManager &SM, unsigned Index,
+unsigned Last, Selector Sel,
+std::vector &SelectorPieces) {
+
+ unsigned NumArgs = Sel.getNumArgs();
+ llvm::SmallVector Closes;
+ SelectorPieces.clear();
+ while (Index < Last) {
+const auto &Tok = Tokens[Index];
+
+if (Closes.empty()) {
+ auto PieceCount = SelectorPieces.size();
+ if (PieceCount < NumArgs &&
+ isMatchingSelectorName(Tok, Tokens[Index + 1], SM,
+ Sel.getNameForSlot(PieceCount))) {
+// If 'foo:' instead of ':' (empty selector), we need to skip the ':'
+// token after the name.
+if (!Sel.getNameForSlot(PieceCount).empty()) {
+ ++Index;
+}
+SelectorPieces.push_back(
+halfOpenToRange(SM, Tok.range(SM).toCharRange(SM)));
+continue;
+ }
+ // If we've found all pieces but the current token looks like another
+ // selector piece, it means the method being renamed is a strict prefix
of
+ // the selector we've found - should be skipped.
+ if (SelectorPieces.size() >= NumArgs &&
+ isSelectorLike(Tok, Tokens[Index + 1]))
+return false;
+}
+
+switch (Tok.kind()) {
+case tok::l_square:
+ Closes.push_back(']');
+ break;
+case tok::l_paren:
+ Closes.push_back(')');
+ break;
+case tok::l_brace:
+ Closes.push_back('}');
+ break;
+case tok::r_square:
+ if (Closes.empty())
+return SelectorPieces.size() == NumArgs;
+
+ if (Closes.back() != ']')
+return false;
+ Closes.pop_back();
+ break;
+case tok::r_paren:
+ if (Closes.empty() || Closes.back() != ')')
+return false;
+ Closes.pop_back();
+ break;
+case tok::r_brace:
+ if (Closes.empty() || Closes.back() != '}')
+return false;
+ Closes.pop_back();
+ break;
+case tok::semi:
+ // top level ; ends all statements.
+ if (Closes.empty())
+return false;
+ break;
+default:
+ break;
+}
+
+++Index;
+ }
+ return false;
+}
+
+/// Collects all ranges of the given identifier/selector in the source code.
+///
+/// If a selector is given, this does a full lex of the given source code in
+/// order to identify all selector fragments (e.g. in method exprs/decls) since
+/// they are non-contiguous.
+std::vector collectRenameIdentifierRanges(
+llvm::StringRef Identifier, llvm::StringRef Content,
+const LangOptions &LangOpts, std::optional Selector) {
+ std::vector Ranges;
+ if (!Selector) {
+auto IdentifierRanges =
+collectIdentifierRanges(Identifier, Content, LangOpts);
+for (const auto &R : IdentifierRanges)
+ Ranges.emplace_back(R);
+return Ranges;
+ }
+ // FIXME: InMemoryFileAdapter crashes unless the buffer is null terminated!
+ std::string NullTerminatedCode = Content.str();
+ SourceManagerForFile FileSM("mock_file_name.cpp", NullTerminatedCode);
+ auto &SM = FileSM.get();
+
+ // We track parens and braces to ensure that we don't accidentally try
parsing
+ // a method declaration or definition which isn't at the top level or similar
+ // looking expressions (e.g. an @selector() expression).
+ unsigned ParenCount = 0;
+ unsigned BraceCount = 0;
+ unsigned NumArgs = Selector->getNumArgs();
+
+ std::vector SelectorPieces;
+ auto Tokens = syntax::tokenize(SM.getMainFileID(), SM, LangOpts);
ahoppen wrote:
When calling `collectRenameIdentifierRanges` from `renameObjCMethodWithinFile`,
we already have a `ParsedAST` and
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -508,24 +513,46 @@ static bool mayBeValidIdentifier(llvm::StringRef Ident) {
!isAsciiIdentifierStart(Ident.front(), AllowDollar))
return false;
for (char C : Ident) {
+if (AllowColon && C == ':')
+ continue;
if (llvm::isASCII(C) && !isAsciiIdentifierContinue(C, AllowDollar))
return false;
}
return true;
}
+std::string getName(const NamedDecl &RenameDecl) {
+ if (const auto *MD = dyn_cast(&RenameDecl))
+return MD->getSelector().getAsString();
+ if (const auto *ID = RenameDecl.getIdentifier())
+return ID->getName().str();
+ return "";
+}
+
// Check if we can rename the given RenameDecl into NewName.
// Return details if the rename would produce a conflict.
-std::optional checkName(const NamedDecl &RenameDecl,
- llvm::StringRef NewName) {
+std::optional checkName(const NamedDecl &RenameDecl,
+ llvm::StringRef NewName,
+ llvm::StringRef OldName) {
trace::Span Tracer("CheckName");
static constexpr trace::Metric InvalidNameMetric(
"rename_name_invalid", trace::Metric::Counter, "invalid_kind");
+
+ if (OldName == NewName)
+return makeError(ReasonToReject::SameName);
+
+ if (const auto *MD = dyn_cast(&RenameDecl)) {
+const auto Sel = MD->getSelector();
+if (Sel.getNumArgs() != NewName.count(':') &&
ahoppen wrote:
Instead of effectively doing a stripped-down selector parsing here, I would
make `checkName` take a `SymbolName`.
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -569,8 +840,13 @@ renameWithinFile(ParsedAST &AST, const NamedDecl &RenameDecl, // } if (!isInsideMainFile(RenameLoc, SM)) continue; +Locs.push_back(RenameLoc); + } + if (const auto *MD = dyn_cast(&RenameDecl)) +return renameObjCMethodWithinFile(AST, MD, NewName, std::move(Locs)); ahoppen wrote: If it is a zero-arg or one-arg Objective-C selector, I think we can still use the old, faster path that doesn’t need to scan the source file for Objective-C selectors using `renameObjCMethodWithinFile`. https://github.com/llvm/llvm-project/pull/76466 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -508,24 +513,46 @@ static bool mayBeValidIdentifier(llvm::StringRef Ident) {
!isAsciiIdentifierStart(Ident.front(), AllowDollar))
return false;
for (char C : Ident) {
+if (AllowColon && C == ':')
ahoppen wrote:
Should we also allow spaces here if you spell the new method name as
`doSomething: with:`?
---
Should we also check that the first selector piece is not empty? Ie. that `:
with:` is not a valid selector.
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -538,11 +565,254 @@ std::optional checkName(const NamedDecl
&RenameDecl,
Conflict->getLocation().printToString(ASTCtx.getSourceManager())};
}
}
- if (Result)
+ if (Result) {
InvalidNameMetric.record(1, toString(Result->K));
+return makeError(*Result);
+ }
+ return std::nullopt;
+}
+
+bool isMatchingSelectorName(const syntax::Token &Cur, const syntax::Token
&Next,
+const SourceManager &SM,
+llvm::StringRef SelectorName) {
+ if (SelectorName.empty())
+return Cur.kind() == tok::colon;
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ Cur.text(SM) == SelectorName &&
+ // We require the selector name and : to be contiguous to avoid
+ // potential conflicts with ternary expression.
+ //
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
ahoppen wrote:
Could this be simplified to
```suggestion
return isSelectorLike(Cur, Next) && Cur.text(SM) == SelectorName;
```
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -538,11 +565,254 @@ std::optional checkName(const NamedDecl
&RenameDecl,
Conflict->getLocation().printToString(ASTCtx.getSourceManager())};
}
}
- if (Result)
+ if (Result) {
InvalidNameMetric.record(1, toString(Result->K));
+return makeError(*Result);
+ }
+ return std::nullopt;
+}
+
+bool isMatchingSelectorName(const syntax::Token &Cur, const syntax::Token
&Next,
+const SourceManager &SM,
+llvm::StringRef SelectorName) {
+ if (SelectorName.empty())
+return Cur.kind() == tok::colon;
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ Cur.text(SM) == SelectorName &&
+ // We require the selector name and : to be contiguous to avoid
+ // potential conflicts with ternary expression.
+ //
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+bool isSelectorLike(const syntax::Token &Cur, const syntax::Token &Next) {
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ // We require the selector name and : to be contiguous.
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+bool parseMessageExpression(llvm::ArrayRef Tokens,
+const SourceManager &SM, unsigned Index,
+unsigned Last, Selector Sel,
+std::vector &SelectorPieces) {
+
+ unsigned NumArgs = Sel.getNumArgs();
+ llvm::SmallVector Closes;
ahoppen wrote:
Wouldn’t it be cleaner if `Closes` is a vector of token kinds? I think then you
could also just check something like the following instead of repeating it in
the `switch`.
```cpp
if (!Closes.empty() && Closes.back() == Tok.kind()) {
Closes.pop_back();
continue;
}
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -53,13 +55,34 @@ struct RenameInputs {
struct RenameResult {
// The range of the symbol that the user can attempt to rename.
Range Target;
+ // Placeholder text for the rename operation if non-empty.
+ std::string Placeholder;
// Rename occurrences for the current main file.
std::vector LocalChanges;
// Complete edits for the rename, including LocalChanges.
// If the full set of changes is unknown, this field is empty.
FileEdits GlobalChanges;
};
+/// Represents a symbol range where the symbol can potentially have multiple
+/// tokens.
+struct SymbolRange {
+ /// Ranges for the tokens that make up the symbol's name.
+ /// Usually a single range, but there can be multiple ranges if the tokens
for
+ /// the symbol are split, e.g. ObjC selectors.
+ std::vector Ranges;
ahoppen wrote:
I think it would be good to clarify if
- These ranges are only the selector piece names or also the colons
- If an empty selector piece is represented by an empty range in here
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
https://github.com/bnbarham commented: Cool stuff @DavidGoldman! Nice to see ObjC getting some love. It's a bummer we duplicated here, but seems like both you and @ahoppen took fairly similar approaches. The main difference looks to be the use of `SymbolName` in the other PR. https://github.com/llvm/llvm-project/pull/76466 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -508,24 +513,46 @@ static bool mayBeValidIdentifier(llvm::StringRef Ident) {
!isAsciiIdentifierStart(Ident.front(), AllowDollar))
return false;
for (char C : Ident) {
+if (AllowColon && C == ':')
+ continue;
if (llvm::isASCII(C) && !isAsciiIdentifierContinue(C, AllowDollar))
return false;
}
return true;
}
+std::string getName(const NamedDecl &RenameDecl) {
+ if (const auto *MD = dyn_cast(&RenameDecl))
+return MD->getSelector().getAsString();
+ if (const auto *ID = RenameDecl.getIdentifier())
+return ID->getName().str();
+ return "";
+}
+
// Check if we can rename the given RenameDecl into NewName.
// Return details if the rename would produce a conflict.
-std::optional checkName(const NamedDecl &RenameDecl,
- llvm::StringRef NewName) {
+std::optional checkName(const NamedDecl &RenameDecl,
DavidGoldman wrote:
Done
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -538,11 +565,254 @@ std::optional checkName(const NamedDecl
&RenameDecl,
Conflict->getLocation().printToString(ASTCtx.getSourceManager())};
}
}
- if (Result)
+ if (Result) {
InvalidNameMetric.record(1, toString(Result->K));
+return makeError(*Result);
+ }
+ return std::nullopt;
+}
+
+bool isMatchingSelectorName(const syntax::Token &Cur, const syntax::Token
&Next,
+const SourceManager &SM,
+llvm::StringRef SelectorName) {
+ if (SelectorName.empty())
+return Cur.kind() == tok::colon;
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ Cur.text(SM) == SelectorName &&
+ // We require the selector name and : to be contiguous to avoid
+ // potential conflicts with ternary expression.
+ //
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+bool isSelectorLike(const syntax::Token &Cur, const syntax::Token &Next) {
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ // We require the selector name and : to be contiguous.
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+bool parseMessageExpression(llvm::ArrayRef Tokens,
+const SourceManager &SM, unsigned Index,
+unsigned Last, Selector Sel,
+std::vector &SelectorPieces) {
+
+ unsigned NumArgs = Sel.getNumArgs();
+ llvm::SmallVector Closes;
+ SelectorPieces.clear();
+ while (Index < Last) {
+const auto &Tok = Tokens[Index];
+
+if (Closes.empty()) {
+ auto PieceCount = SelectorPieces.size();
+ if (PieceCount < NumArgs &&
+ isMatchingSelectorName(Tok, Tokens[Index + 1], SM,
+ Sel.getNameForSlot(PieceCount))) {
+// If 'foo:' instead of ':' (empty selector), we need to skip the ':'
+// token after the name.
+if (!Sel.getNameForSlot(PieceCount).empty()) {
+ ++Index;
+}
+SelectorPieces.push_back(
+halfOpenToRange(SM, Tok.range(SM).toCharRange(SM)));
+continue;
+ }
+ // If we've found all pieces but the current token looks like another
+ // selector piece, it means the method being renamed is a strict prefix
of
+ // the selector we've found - should be skipped.
+ if (SelectorPieces.size() >= NumArgs &&
+ isSelectorLike(Tok, Tokens[Index + 1]))
+return false;
+}
+
+switch (Tok.kind()) {
+case tok::l_square:
+ Closes.push_back(']');
+ break;
+case tok::l_paren:
+ Closes.push_back(')');
+ break;
+case tok::l_brace:
+ Closes.push_back('}');
+ break;
+case tok::r_square:
+ if (Closes.empty())
+return SelectorPieces.size() == NumArgs;
+
+ if (Closes.back() != ']')
+return false;
+ Closes.pop_back();
+ break;
+case tok::r_paren:
+ if (Closes.empty() || Closes.back() != ')')
+return false;
+ Closes.pop_back();
+ break;
+case tok::r_brace:
+ if (Closes.empty() || Closes.back() != '}')
+return false;
+ Closes.pop_back();
+ break;
+case tok::semi:
+ // top level ; ends all statements.
+ if (Closes.empty())
+return false;
+ break;
+default:
+ break;
+}
+
+++Index;
+ }
+ return false;
+}
+
+/// Collects all ranges of the given identifier/selector in the source code.
+///
+/// If a selector is given, this does a full lex of the given source code in
+/// order to identify all selector fragments (e.g. in method exprs/decls) since
+/// they are non-contiguous.
+std::vector collectRenameIdentifierRanges(
+llvm::StringRef Identifier, llvm::StringRef Content,
+const LangOptions &LangOpts, std::optional Selector) {
+ std::vector Ranges;
+ if (!Selector) {
+auto IdentifierRanges =
+collectIdentifierRanges(Identifier, Content, LangOpts);
+for (const auto &R : IdentifierRanges)
+ Ranges.emplace_back(R);
+return Ranges;
+ }
+ // FIXME: InMemoryFileAdapter crashes unless the buffer is null terminated!
+ std::string NullTerminatedCode = Content.str();
+ SourceManagerForFile FileSM("mock_file_name.cpp", NullTerminatedCode);
+ auto &SM = FileSM.get();
+
+ // We track parens and braces to ensure that we don't accidentally try
parsing
+ // a method declaration or definition which isn't at the top level or similar
+ // looking expressions (e.g. an @selector() expression).
+ unsigned ParenCount = 0;
+ unsigned BraceCount = 0;
+ unsigned NumArgs = Selector->getNumArgs();
+
+ std::vector SelectorPieces;
+ auto Tokens = syntax::tokenize(SM.getMainFileID(), SM, LangOpts);
+ unsigned Last = Tokens.size() - 1;
+ for (unsigned Index = 0; Index < Last; ++Index) {
+const auto &Tok = Tokens[Index];
+
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -538,11 +565,254 @@ std::optional checkName(const NamedDecl
&RenameDecl,
Conflict->getLocation().printToString(ASTCtx.getSourceManager())};
}
}
- if (Result)
+ if (Result) {
InvalidNameMetric.record(1, toString(Result->K));
+return makeError(*Result);
+ }
+ return std::nullopt;
+}
+
+bool isMatchingSelectorName(const syntax::Token &Cur, const syntax::Token
&Next,
+const SourceManager &SM,
+llvm::StringRef SelectorName) {
+ if (SelectorName.empty())
+return Cur.kind() == tok::colon;
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ Cur.text(SM) == SelectorName &&
+ // We require the selector name and : to be contiguous to avoid
+ // potential conflicts with ternary expression.
+ //
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+bool isSelectorLike(const syntax::Token &Cur, const syntax::Token &Next) {
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ // We require the selector name and : to be contiguous.
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+bool parseMessageExpression(llvm::ArrayRef Tokens,
+const SourceManager &SM, unsigned Index,
+unsigned Last, Selector Sel,
+std::vector &SelectorPieces) {
+
+ unsigned NumArgs = Sel.getNumArgs();
+ llvm::SmallVector Closes;
+ SelectorPieces.clear();
+ while (Index < Last) {
+const auto &Tok = Tokens[Index];
+
+if (Closes.empty()) {
+ auto PieceCount = SelectorPieces.size();
+ if (PieceCount < NumArgs &&
+ isMatchingSelectorName(Tok, Tokens[Index + 1], SM,
+ Sel.getNameForSlot(PieceCount))) {
+// If 'foo:' instead of ':' (empty selector), we need to skip the ':'
+// token after the name.
+if (!Sel.getNameForSlot(PieceCount).empty()) {
+ ++Index;
+}
+SelectorPieces.push_back(
+halfOpenToRange(SM, Tok.range(SM).toCharRange(SM)));
+continue;
+ }
+ // If we've found all pieces but the current token looks like another
+ // selector piece, it means the method being renamed is a strict prefix
of
+ // the selector we've found - should be skipped.
+ if (SelectorPieces.size() >= NumArgs &&
+ isSelectorLike(Tok, Tokens[Index + 1]))
+return false;
+}
+
+switch (Tok.kind()) {
+case tok::l_square:
+ Closes.push_back(']');
+ break;
+case tok::l_paren:
+ Closes.push_back(')');
+ break;
+case tok::l_brace:
+ Closes.push_back('}');
+ break;
+case tok::r_square:
+ if (Closes.empty())
+return SelectorPieces.size() == NumArgs;
+
+ if (Closes.back() != ']')
+return false;
+ Closes.pop_back();
+ break;
+case tok::r_paren:
+ if (Closes.empty() || Closes.back() != ')')
+return false;
+ Closes.pop_back();
+ break;
+case tok::r_brace:
+ if (Closes.empty() || Closes.back() != '}')
+return false;
+ Closes.pop_back();
+ break;
+case tok::semi:
+ // top level ; ends all statements.
+ if (Closes.empty())
+return false;
+ break;
+default:
+ break;
+}
+
+++Index;
+ }
+ return false;
+}
+
+/// Collects all ranges of the given identifier/selector in the source code.
+///
+/// If a selector is given, this does a full lex of the given source code in
+/// order to identify all selector fragments (e.g. in method exprs/decls) since
+/// they are non-contiguous.
+std::vector collectRenameIdentifierRanges(
+llvm::StringRef Identifier, llvm::StringRef Content,
+const LangOptions &LangOpts, std::optional Selector) {
+ std::vector Ranges;
+ if (!Selector) {
+auto IdentifierRanges =
+collectIdentifierRanges(Identifier, Content, LangOpts);
+for (const auto &R : IdentifierRanges)
+ Ranges.emplace_back(R);
+return Ranges;
+ }
+ // FIXME: InMemoryFileAdapter crashes unless the buffer is null terminated!
+ std::string NullTerminatedCode = Content.str();
+ SourceManagerForFile FileSM("mock_file_name.cpp", NullTerminatedCode);
+ auto &SM = FileSM.get();
+
+ // We track parens and braces to ensure that we don't accidentally try
parsing
+ // a method declaration or definition which isn't at the top level or similar
+ // looking expressions (e.g. an @selector() expression).
+ unsigned ParenCount = 0;
+ unsigned BraceCount = 0;
+ unsigned NumArgs = Selector->getNumArgs();
+
+ std::vector SelectorPieces;
+ auto Tokens = syntax::tokenize(SM.getMainFileID(), SM, LangOpts);
+ unsigned Last = Tokens.size() - 1;
+ for (unsigned Index = 0; Index < Last; ++Index) {
+const auto &Tok = Tokens[Index];
+
+if (BraceCou
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -538,11 +565,254 @@ std::optional checkName(const NamedDecl
&RenameDecl,
Conflict->getLocation().printToString(ASTCtx.getSourceManager())};
}
}
- if (Result)
+ if (Result) {
InvalidNameMetric.record(1, toString(Result->K));
+return makeError(*Result);
+ }
+ return std::nullopt;
+}
+
+bool isMatchingSelectorName(const syntax::Token &Cur, const syntax::Token
&Next,
+const SourceManager &SM,
+llvm::StringRef SelectorName) {
+ if (SelectorName.empty())
+return Cur.kind() == tok::colon;
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ Cur.text(SM) == SelectorName &&
+ // We require the selector name and : to be contiguous to avoid
+ // potential conflicts with ternary expression.
+ //
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+bool isSelectorLike(const syntax::Token &Cur, const syntax::Token &Next) {
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ // We require the selector name and : to be contiguous.
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+bool parseMessageExpression(llvm::ArrayRef Tokens,
+const SourceManager &SM, unsigned Index,
+unsigned Last, Selector Sel,
+std::vector &SelectorPieces) {
DavidGoldman wrote:
Done
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -508,24 +513,46 @@ static bool mayBeValidIdentifier(llvm::StringRef Ident) {
!isAsciiIdentifierStart(Ident.front(), AllowDollar))
return false;
for (char C : Ident) {
+if (AllowColon && C == ':')
DavidGoldman wrote:
This doesn't check the entire selector - only a fragment, so I don't think
there's a need to check spaces here. AFAIK selectors can have an empty first
fragment too - any idea where that restriction is from?
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -538,11 +565,254 @@ std::optional checkName(const NamedDecl
&RenameDecl,
Conflict->getLocation().printToString(ASTCtx.getSourceManager())};
}
}
- if (Result)
+ if (Result) {
InvalidNameMetric.record(1, toString(Result->K));
+return makeError(*Result);
+ }
+ return std::nullopt;
+}
+
+bool isMatchingSelectorName(const syntax::Token &Cur, const syntax::Token
&Next,
+const SourceManager &SM,
+llvm::StringRef SelectorName) {
+ if (SelectorName.empty())
+return Cur.kind() == tok::colon;
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ Cur.text(SM) == SelectorName &&
+ // We require the selector name and : to be contiguous to avoid
+ // potential conflicts with ternary expression.
+ //
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+bool isSelectorLike(const syntax::Token &Cur, const syntax::Token &Next) {
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ // We require the selector name and : to be contiguous.
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+bool parseMessageExpression(llvm::ArrayRef Tokens,
+const SourceManager &SM, unsigned Index,
+unsigned Last, Selector Sel,
+std::vector &SelectorPieces) {
+
+ unsigned NumArgs = Sel.getNumArgs();
+ llvm::SmallVector Closes;
+ SelectorPieces.clear();
+ while (Index < Last) {
+const auto &Tok = Tokens[Index];
+
+if (Closes.empty()) {
+ auto PieceCount = SelectorPieces.size();
+ if (PieceCount < NumArgs &&
+ isMatchingSelectorName(Tok, Tokens[Index + 1], SM,
+ Sel.getNameForSlot(PieceCount))) {
+// If 'foo:' instead of ':' (empty selector), we need to skip the ':'
+// token after the name.
+if (!Sel.getNameForSlot(PieceCount).empty()) {
+ ++Index;
+}
+SelectorPieces.push_back(
+halfOpenToRange(SM, Tok.range(SM).toCharRange(SM)));
+continue;
+ }
+ // If we've found all pieces but the current token looks like another
+ // selector piece, it means the method being renamed is a strict prefix
of
+ // the selector we've found - should be skipped.
+ if (SelectorPieces.size() >= NumArgs &&
+ isSelectorLike(Tok, Tokens[Index + 1]))
+return false;
+}
+
+switch (Tok.kind()) {
+case tok::l_square:
+ Closes.push_back(']');
+ break;
+case tok::l_paren:
+ Closes.push_back(')');
+ break;
+case tok::l_brace:
+ Closes.push_back('}');
+ break;
+case tok::r_square:
+ if (Closes.empty())
+return SelectorPieces.size() == NumArgs;
+
+ if (Closes.back() != ']')
+return false;
+ Closes.pop_back();
+ break;
+case tok::r_paren:
+ if (Closes.empty() || Closes.back() != ')')
+return false;
+ Closes.pop_back();
+ break;
+case tok::r_brace:
+ if (Closes.empty() || Closes.back() != '}')
+return false;
+ Closes.pop_back();
+ break;
+case tok::semi:
+ // top level ; ends all statements.
+ if (Closes.empty())
+return false;
+ break;
+default:
+ break;
+}
+
+++Index;
+ }
+ return false;
+}
+
+/// Collects all ranges of the given identifier/selector in the source code.
+///
+/// If a selector is given, this does a full lex of the given source code in
+/// order to identify all selector fragments (e.g. in method exprs/decls) since
+/// they are non-contiguous.
+std::vector collectRenameIdentifierRanges(
+llvm::StringRef Identifier, llvm::StringRef Content,
+const LangOptions &LangOpts, std::optional Selector) {
+ std::vector Ranges;
+ if (!Selector) {
+auto IdentifierRanges =
+collectIdentifierRanges(Identifier, Content, LangOpts);
+for (const auto &R : IdentifierRanges)
+ Ranges.emplace_back(R);
+return Ranges;
+ }
+ // FIXME: InMemoryFileAdapter crashes unless the buffer is null terminated!
+ std::string NullTerminatedCode = Content.str();
+ SourceManagerForFile FileSM("mock_file_name.cpp", NullTerminatedCode);
+ auto &SM = FileSM.get();
+
+ // We track parens and braces to ensure that we don't accidentally try
parsing
+ // a method declaration or definition which isn't at the top level or similar
+ // looking expressions (e.g. an @selector() expression).
+ unsigned ParenCount = 0;
+ unsigned BraceCount = 0;
+ unsigned NumArgs = Selector->getNumArgs();
+
+ std::vector SelectorPieces;
+ auto Tokens = syntax::tokenize(SM.getMainFileID(), SM, LangOpts);
DavidGoldman wrote:
That's true, although a bit annoying since we need a SM to fetch this and I
wouldn't want to force that into
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -538,11 +565,254 @@ std::optional checkName(const NamedDecl
&RenameDecl,
Conflict->getLocation().printToString(ASTCtx.getSourceManager())};
}
}
- if (Result)
+ if (Result) {
InvalidNameMetric.record(1, toString(Result->K));
+return makeError(*Result);
+ }
+ return std::nullopt;
+}
+
+bool isMatchingSelectorName(const syntax::Token &Cur, const syntax::Token
&Next,
+const SourceManager &SM,
+llvm::StringRef SelectorName) {
+ if (SelectorName.empty())
+return Cur.kind() == tok::colon;
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ Cur.text(SM) == SelectorName &&
+ // We require the selector name and : to be contiguous to avoid
+ // potential conflicts with ternary expression.
+ //
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+bool isSelectorLike(const syntax::Token &Cur, const syntax::Token &Next) {
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ // We require the selector name and : to be contiguous.
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+bool parseMessageExpression(llvm::ArrayRef Tokens,
+const SourceManager &SM, unsigned Index,
+unsigned Last, Selector Sel,
+std::vector &SelectorPieces) {
+
+ unsigned NumArgs = Sel.getNumArgs();
+ llvm::SmallVector Closes;
+ SelectorPieces.clear();
+ while (Index < Last) {
+const auto &Tok = Tokens[Index];
+
+if (Closes.empty()) {
+ auto PieceCount = SelectorPieces.size();
+ if (PieceCount < NumArgs &&
+ isMatchingSelectorName(Tok, Tokens[Index + 1], SM,
+ Sel.getNameForSlot(PieceCount))) {
+// If 'foo:' instead of ':' (empty selector), we need to skip the ':'
+// token after the name.
+if (!Sel.getNameForSlot(PieceCount).empty()) {
+ ++Index;
+}
+SelectorPieces.push_back(
+halfOpenToRange(SM, Tok.range(SM).toCharRange(SM)));
+continue;
+ }
+ // If we've found all pieces but the current token looks like another
+ // selector piece, it means the method being renamed is a strict prefix
of
+ // the selector we've found - should be skipped.
+ if (SelectorPieces.size() >= NumArgs &&
+ isSelectorLike(Tok, Tokens[Index + 1]))
+return false;
+}
+
+switch (Tok.kind()) {
+case tok::l_square:
+ Closes.push_back(']');
+ break;
+case tok::l_paren:
+ Closes.push_back(')');
+ break;
+case tok::l_brace:
+ Closes.push_back('}');
+ break;
+case tok::r_square:
+ if (Closes.empty())
+return SelectorPieces.size() == NumArgs;
+
+ if (Closes.back() != ']')
+return false;
+ Closes.pop_back();
+ break;
+case tok::r_paren:
+ if (Closes.empty() || Closes.back() != ')')
+return false;
+ Closes.pop_back();
+ break;
+case tok::r_brace:
+ if (Closes.empty() || Closes.back() != '}')
+return false;
+ Closes.pop_back();
+ break;
+case tok::semi:
+ // top level ; ends all statements.
+ if (Closes.empty())
+return false;
+ break;
+default:
+ break;
+}
+
+++Index;
+ }
+ return false;
+}
+
+/// Collects all ranges of the given identifier/selector in the source code.
+///
+/// If a selector is given, this does a full lex of the given source code in
+/// order to identify all selector fragments (e.g. in method exprs/decls) since
+/// they are non-contiguous.
+std::vector collectRenameIdentifierRanges(
+llvm::StringRef Identifier, llvm::StringRef Content,
+const LangOptions &LangOpts, std::optional Selector) {
+ std::vector Ranges;
+ if (!Selector) {
+auto IdentifierRanges =
+collectIdentifierRanges(Identifier, Content, LangOpts);
+for (const auto &R : IdentifierRanges)
+ Ranges.emplace_back(R);
+return Ranges;
+ }
+ // FIXME: InMemoryFileAdapter crashes unless the buffer is null terminated!
+ std::string NullTerminatedCode = Content.str();
+ SourceManagerForFile FileSM("mock_file_name.cpp", NullTerminatedCode);
+ auto &SM = FileSM.get();
+
+ // We track parens and braces to ensure that we don't accidentally try
parsing
+ // a method declaration or definition which isn't at the top level or similar
+ // looking expressions (e.g. an @selector() expression).
+ unsigned ParenCount = 0;
+ unsigned BraceCount = 0;
+ unsigned NumArgs = Selector->getNumArgs();
+
+ std::vector SelectorPieces;
+ auto Tokens = syntax::tokenize(SM.getMainFileID(), SM, LangOpts);
+ unsigned Last = Tokens.size() - 1;
+ for (unsigned Index = 0; Index < Last; ++Index) {
+const auto &Tok = Tokens[Index];
+
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -538,11 +565,254 @@ std::optional checkName(const NamedDecl
&RenameDecl,
Conflict->getLocation().printToString(ASTCtx.getSourceManager())};
}
}
- if (Result)
+ if (Result) {
InvalidNameMetric.record(1, toString(Result->K));
+return makeError(*Result);
+ }
+ return std::nullopt;
+}
+
+bool isMatchingSelectorName(const syntax::Token &Cur, const syntax::Token
&Next,
+const SourceManager &SM,
+llvm::StringRef SelectorName) {
+ if (SelectorName.empty())
+return Cur.kind() == tok::colon;
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ Cur.text(SM) == SelectorName &&
+ // We require the selector name and : to be contiguous to avoid
+ // potential conflicts with ternary expression.
+ //
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+bool isSelectorLike(const syntax::Token &Cur, const syntax::Token &Next) {
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ // We require the selector name and : to be contiguous.
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+bool parseMessageExpression(llvm::ArrayRef Tokens,
+const SourceManager &SM, unsigned Index,
+unsigned Last, Selector Sel,
+std::vector &SelectorPieces) {
+
+ unsigned NumArgs = Sel.getNumArgs();
+ llvm::SmallVector Closes;
+ SelectorPieces.clear();
+ while (Index < Last) {
+const auto &Tok = Tokens[Index];
+
+if (Closes.empty()) {
+ auto PieceCount = SelectorPieces.size();
+ if (PieceCount < NumArgs &&
+ isMatchingSelectorName(Tok, Tokens[Index + 1], SM,
+ Sel.getNameForSlot(PieceCount))) {
+// If 'foo:' instead of ':' (empty selector), we need to skip the ':'
+// token after the name.
+if (!Sel.getNameForSlot(PieceCount).empty()) {
+ ++Index;
+}
+SelectorPieces.push_back(
+halfOpenToRange(SM, Tok.range(SM).toCharRange(SM)));
+continue;
+ }
+ // If we've found all pieces but the current token looks like another
+ // selector piece, it means the method being renamed is a strict prefix
of
+ // the selector we've found - should be skipped.
+ if (SelectorPieces.size() >= NumArgs &&
+ isSelectorLike(Tok, Tokens[Index + 1]))
+return false;
+}
+
+switch (Tok.kind()) {
+case tok::l_square:
+ Closes.push_back(']');
+ break;
+case tok::l_paren:
+ Closes.push_back(')');
+ break;
+case tok::l_brace:
+ Closes.push_back('}');
+ break;
+case tok::r_square:
+ if (Closes.empty())
+return SelectorPieces.size() == NumArgs;
+
+ if (Closes.back() != ']')
+return false;
+ Closes.pop_back();
+ break;
+case tok::r_paren:
+ if (Closes.empty() || Closes.back() != ')')
+return false;
+ Closes.pop_back();
+ break;
+case tok::r_brace:
+ if (Closes.empty() || Closes.back() != '}')
+return false;
+ Closes.pop_back();
+ break;
+case tok::semi:
+ // top level ; ends all statements.
+ if (Closes.empty())
+return false;
+ break;
+default:
+ break;
+}
+
+++Index;
+ }
+ return false;
+}
+
+/// Collects all ranges of the given identifier/selector in the source code.
+///
+/// If a selector is given, this does a full lex of the given source code in
+/// order to identify all selector fragments (e.g. in method exprs/decls) since
+/// they are non-contiguous.
+std::vector collectRenameIdentifierRanges(
+llvm::StringRef Identifier, llvm::StringRef Content,
+const LangOptions &LangOpts, std::optional Selector) {
+ std::vector Ranges;
+ if (!Selector) {
+auto IdentifierRanges =
+collectIdentifierRanges(Identifier, Content, LangOpts);
+for (const auto &R : IdentifierRanges)
+ Ranges.emplace_back(R);
+return Ranges;
+ }
+ // FIXME: InMemoryFileAdapter crashes unless the buffer is null terminated!
+ std::string NullTerminatedCode = Content.str();
+ SourceManagerForFile FileSM("mock_file_name.cpp", NullTerminatedCode);
+ auto &SM = FileSM.get();
+
+ // We track parens and braces to ensure that we don't accidentally try
parsing
+ // a method declaration or definition which isn't at the top level or similar
+ // looking expressions (e.g. an @selector() expression).
+ unsigned ParenCount = 0;
+ unsigned BraceCount = 0;
+ unsigned NumArgs = Selector->getNumArgs();
+
+ std::vector SelectorPieces;
+ auto Tokens = syntax::tokenize(SM.getMainFileID(), SM, LangOpts);
+ unsigned Last = Tokens.size() - 1;
+ for (unsigned Index = 0; Index < Last; ++Index) {
+const auto &Tok = Tokens[Index];
+
+if (BraceCou
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -508,24 +513,46 @@ static bool mayBeValidIdentifier(llvm::StringRef Ident) {
!isAsciiIdentifierStart(Ident.front(), AllowDollar))
return false;
for (char C : Ident) {
+if (AllowColon && C == ':')
ahoppen wrote:
Oh, I just assumed that the first selector argument had to be named. I never
checked if it can be unnamed.
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -538,11 +565,254 @@ std::optional checkName(const NamedDecl
&RenameDecl,
Conflict->getLocation().printToString(ASTCtx.getSourceManager())};
}
}
- if (Result)
+ if (Result) {
InvalidNameMetric.record(1, toString(Result->K));
+return makeError(*Result);
+ }
+ return std::nullopt;
+}
+
+bool isMatchingSelectorName(const syntax::Token &Cur, const syntax::Token
&Next,
+const SourceManager &SM,
+llvm::StringRef SelectorName) {
+ if (SelectorName.empty())
+return Cur.kind() == tok::colon;
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ Cur.text(SM) == SelectorName &&
+ // We require the selector name and : to be contiguous to avoid
+ // potential conflicts with ternary expression.
+ //
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+bool isSelectorLike(const syntax::Token &Cur, const syntax::Token &Next) {
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ // We require the selector name and : to be contiguous.
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+bool parseMessageExpression(llvm::ArrayRef Tokens,
+const SourceManager &SM, unsigned Index,
+unsigned Last, Selector Sel,
+std::vector &SelectorPieces) {
+
+ unsigned NumArgs = Sel.getNumArgs();
+ llvm::SmallVector Closes;
+ SelectorPieces.clear();
+ while (Index < Last) {
+const auto &Tok = Tokens[Index];
+
+if (Closes.empty()) {
+ auto PieceCount = SelectorPieces.size();
+ if (PieceCount < NumArgs &&
+ isMatchingSelectorName(Tok, Tokens[Index + 1], SM,
+ Sel.getNameForSlot(PieceCount))) {
+// If 'foo:' instead of ':' (empty selector), we need to skip the ':'
+// token after the name.
+if (!Sel.getNameForSlot(PieceCount).empty()) {
+ ++Index;
+}
+SelectorPieces.push_back(
+halfOpenToRange(SM, Tok.range(SM).toCharRange(SM)));
+continue;
+ }
+ // If we've found all pieces but the current token looks like another
+ // selector piece, it means the method being renamed is a strict prefix
of
+ // the selector we've found - should be skipped.
+ if (SelectorPieces.size() >= NumArgs &&
+ isSelectorLike(Tok, Tokens[Index + 1]))
+return false;
+}
+
+switch (Tok.kind()) {
+case tok::l_square:
+ Closes.push_back(']');
+ break;
+case tok::l_paren:
+ Closes.push_back(')');
+ break;
+case tok::l_brace:
+ Closes.push_back('}');
+ break;
+case tok::r_square:
+ if (Closes.empty())
+return SelectorPieces.size() == NumArgs;
+
+ if (Closes.back() != ']')
+return false;
+ Closes.pop_back();
+ break;
+case tok::r_paren:
+ if (Closes.empty() || Closes.back() != ')')
+return false;
+ Closes.pop_back();
+ break;
+case tok::r_brace:
+ if (Closes.empty() || Closes.back() != '}')
+return false;
+ Closes.pop_back();
+ break;
+case tok::semi:
+ // top level ; ends all statements.
+ if (Closes.empty())
+return false;
+ break;
+default:
+ break;
+}
+
+++Index;
+ }
+ return false;
+}
+
+/// Collects all ranges of the given identifier/selector in the source code.
+///
+/// If a selector is given, this does a full lex of the given source code in
+/// order to identify all selector fragments (e.g. in method exprs/decls) since
+/// they are non-contiguous.
+std::vector collectRenameIdentifierRanges(
+llvm::StringRef Identifier, llvm::StringRef Content,
+const LangOptions &LangOpts, std::optional Selector) {
+ std::vector Ranges;
+ if (!Selector) {
+auto IdentifierRanges =
+collectIdentifierRanges(Identifier, Content, LangOpts);
+for (const auto &R : IdentifierRanges)
+ Ranges.emplace_back(R);
+return Ranges;
+ }
+ // FIXME: InMemoryFileAdapter crashes unless the buffer is null terminated!
+ std::string NullTerminatedCode = Content.str();
+ SourceManagerForFile FileSM("mock_file_name.cpp", NullTerminatedCode);
+ auto &SM = FileSM.get();
+
+ // We track parens and braces to ensure that we don't accidentally try
parsing
+ // a method declaration or definition which isn't at the top level or similar
+ // looking expressions (e.g. an @selector() expression).
+ unsigned ParenCount = 0;
+ unsigned BraceCount = 0;
+ unsigned NumArgs = Selector->getNumArgs();
+
+ std::vector SelectorPieces;
+ auto Tokens = syntax::tokenize(SM.getMainFileID(), SM, LangOpts);
ahoppen wrote:
I don’t think it’s too bad to force that onto the caller. AFAICT we have two
calls to `collectRenameIdentifierRang
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -538,11 +565,254 @@ std::optional checkName(const NamedDecl
&RenameDecl,
Conflict->getLocation().printToString(ASTCtx.getSourceManager())};
}
}
- if (Result)
+ if (Result) {
InvalidNameMetric.record(1, toString(Result->K));
+return makeError(*Result);
+ }
+ return std::nullopt;
+}
+
+bool isMatchingSelectorName(const syntax::Token &Cur, const syntax::Token
&Next,
+const SourceManager &SM,
+llvm::StringRef SelectorName) {
+ if (SelectorName.empty())
+return Cur.kind() == tok::colon;
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ Cur.text(SM) == SelectorName &&
+ // We require the selector name and : to be contiguous to avoid
+ // potential conflicts with ternary expression.
+ //
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
DavidGoldman wrote:
Done
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
https://github.com/DavidGoldman updated
https://github.com/llvm/llvm-project/pull/76466
>From 4caf5b3c779bf18236b4b0be5bc7147d10339f2b Mon Sep 17 00:00:00 2001
From: David Goldman
Date: Tue, 26 Dec 2023 15:59:01 -0500
Subject: [PATCH 1/8] [clangd][SymbolCollector] Treat ObjC methods as spelled
We'll treat multi-arg methods as spelled once we have full rename
support for them.
---
.../clangd/index/SymbolCollector.cpp | 6 ++-
.../clangd/unittests/SymbolCollectorTests.cpp | 42 +++
2 files changed, 47 insertions(+), 1 deletion(-)
diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp
b/clang-tools-extra/clangd/index/SymbolCollector.cpp
index 7ef4b15febad22..336bc3506bb360 100644
--- a/clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -174,7 +174,9 @@ bool isSpelled(SourceLocation Loc, const NamedDecl &ND) {
auto Name = ND.getDeclName();
const auto NameKind = Name.getNameKind();
if (NameKind != DeclarationName::Identifier &&
- NameKind != DeclarationName::CXXConstructorName)
+ NameKind != DeclarationName::CXXConstructorName &&
+ NameKind != DeclarationName::ObjCZeroArgSelector &&
+ NameKind != DeclarationName::ObjCOneArgSelector)
return false;
const auto &AST = ND.getASTContext();
const auto &SM = AST.getSourceManager();
@@ -183,6 +185,8 @@ bool isSpelled(SourceLocation Loc, const NamedDecl &ND) {
if (clang::Lexer::getRawToken(Loc, Tok, SM, LO))
return false;
auto StrName = Name.getAsString();
+ if (const auto *MD = dyn_cast(&ND))
+StrName = MD->getSelector().getNameForSlot(0).str();
return clang::Lexer::getSpelling(Tok, SM, LO) == StrName;
}
} // namespace
diff --git a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
index 9cdc57ec01f327..1d4e1c1d75ea23 100644
--- a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -105,6 +105,9 @@ MATCHER(refRange, "") {
MATCHER_P2(OverriddenBy, Subject, Object, "") {
return arg == Relation{Subject.ID, RelationKind::OverriddenBy, Object.ID};
}
+MATCHER(isSpelled, "") {
+ return static_cast(arg.Kind & RefKind::Spelled);
+}
::testing::Matcher &>
haveRanges(const std::vector Ranges) {
return ::testing::UnorderedPointwise(refRange(), Ranges);
@@ -524,6 +527,45 @@ TEST_F(SymbolCollectorTest, templateArgs) {
forCodeCompletion(false);
}
+TEST_F(SymbolCollectorTest, ObjCRefs) {
+ Annotations Header(R"(
+ @interface Person
+ - (void)$talk[[talk]];
+ - (void)$say[[say]]:(id)something;
+ @end
+ @interface Person (Category)
+ - (void)categoryMethod;
+ - (void)multiArg:(id)a method:(id)b;
+ @end
+ )");
+ Annotations Main(R"(
+ @implementation Person
+ - (void)$talk[[talk]] {}
+ - (void)$say[[say]]:(id)something {}
+ @end
+
+ void fff(Person *p) {
+[p $talk[[talk]]];
+[p $say[[say]]:0];
+[p categoryMethod];
+[p multiArg:0 method:0];
+ }
+ )");
+ CollectorOpts.RefFilter = RefKind::All;
+ CollectorOpts.CollectMainFileRefs = true;
+ TestFileName = testPath("test.m");
+ runSymbolCollector(Header.code(), Main.code(),
+ {"-fblocks", "-xobjective-c++", "-Wno-objc-root-class"});
+ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Person::talk").ID,
+ haveRanges(Main.ranges("talk");
+ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Person::say:").ID,
+ haveRanges(Main.ranges("say");
+ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols,
"Person::categoryMethod").ID,
+ ElementsAre(isSpelled();
+ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols,
"Person::multiArg:method:").ID,
+ ElementsAre(Not(isSpelled());
+}
+
TEST_F(SymbolCollectorTest, ObjCSymbols) {
const std::string Header = R"(
@interface Person
>From 1b6a09464ff5c7b1988fcb479d0a4ff876f696e6 Mon Sep 17 00:00:00 2001
From: David Goldman
Date: Tue, 26 Dec 2023 16:12:03 -0500
Subject: [PATCH 2/8] Run clang-format
---
.../clangd/unittests/SymbolCollectorTests.cpp | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
index 1d4e1c1d75ea23..5c20b950e4eac0 100644
--- a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -560,10 +560,12 @@ TEST_F(SymbolCollectorTest, ObjCRefs) {
haveRanges(Main.ranges("talk");
EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Person::say:").ID,
haveRanges(Main.ranges("say");
- EXPECT_THAT(Refs, Contains(Pair(findSymbol
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
https://github.com/DavidGoldman updated
https://github.com/llvm/llvm-project/pull/76466
>From 4caf5b3c779bf18236b4b0be5bc7147d10339f2b Mon Sep 17 00:00:00 2001
From: David Goldman
Date: Tue, 26 Dec 2023 15:59:01 -0500
Subject: [PATCH 1/9] [clangd][SymbolCollector] Treat ObjC methods as spelled
We'll treat multi-arg methods as spelled once we have full rename
support for them.
---
.../clangd/index/SymbolCollector.cpp | 6 ++-
.../clangd/unittests/SymbolCollectorTests.cpp | 42 +++
2 files changed, 47 insertions(+), 1 deletion(-)
diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp
b/clang-tools-extra/clangd/index/SymbolCollector.cpp
index 7ef4b15febad2..336bc3506bb36 100644
--- a/clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -174,7 +174,9 @@ bool isSpelled(SourceLocation Loc, const NamedDecl &ND) {
auto Name = ND.getDeclName();
const auto NameKind = Name.getNameKind();
if (NameKind != DeclarationName::Identifier &&
- NameKind != DeclarationName::CXXConstructorName)
+ NameKind != DeclarationName::CXXConstructorName &&
+ NameKind != DeclarationName::ObjCZeroArgSelector &&
+ NameKind != DeclarationName::ObjCOneArgSelector)
return false;
const auto &AST = ND.getASTContext();
const auto &SM = AST.getSourceManager();
@@ -183,6 +185,8 @@ bool isSpelled(SourceLocation Loc, const NamedDecl &ND) {
if (clang::Lexer::getRawToken(Loc, Tok, SM, LO))
return false;
auto StrName = Name.getAsString();
+ if (const auto *MD = dyn_cast(&ND))
+StrName = MD->getSelector().getNameForSlot(0).str();
return clang::Lexer::getSpelling(Tok, SM, LO) == StrName;
}
} // namespace
diff --git a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
index 9cdc57ec01f32..1d4e1c1d75ea2 100644
--- a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -105,6 +105,9 @@ MATCHER(refRange, "") {
MATCHER_P2(OverriddenBy, Subject, Object, "") {
return arg == Relation{Subject.ID, RelationKind::OverriddenBy, Object.ID};
}
+MATCHER(isSpelled, "") {
+ return static_cast(arg.Kind & RefKind::Spelled);
+}
::testing::Matcher &>
haveRanges(const std::vector Ranges) {
return ::testing::UnorderedPointwise(refRange(), Ranges);
@@ -524,6 +527,45 @@ TEST_F(SymbolCollectorTest, templateArgs) {
forCodeCompletion(false);
}
+TEST_F(SymbolCollectorTest, ObjCRefs) {
+ Annotations Header(R"(
+ @interface Person
+ - (void)$talk[[talk]];
+ - (void)$say[[say]]:(id)something;
+ @end
+ @interface Person (Category)
+ - (void)categoryMethod;
+ - (void)multiArg:(id)a method:(id)b;
+ @end
+ )");
+ Annotations Main(R"(
+ @implementation Person
+ - (void)$talk[[talk]] {}
+ - (void)$say[[say]]:(id)something {}
+ @end
+
+ void fff(Person *p) {
+[p $talk[[talk]]];
+[p $say[[say]]:0];
+[p categoryMethod];
+[p multiArg:0 method:0];
+ }
+ )");
+ CollectorOpts.RefFilter = RefKind::All;
+ CollectorOpts.CollectMainFileRefs = true;
+ TestFileName = testPath("test.m");
+ runSymbolCollector(Header.code(), Main.code(),
+ {"-fblocks", "-xobjective-c++", "-Wno-objc-root-class"});
+ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Person::talk").ID,
+ haveRanges(Main.ranges("talk");
+ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Person::say:").ID,
+ haveRanges(Main.ranges("say");
+ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols,
"Person::categoryMethod").ID,
+ ElementsAre(isSpelled();
+ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols,
"Person::multiArg:method:").ID,
+ ElementsAre(Not(isSpelled());
+}
+
TEST_F(SymbolCollectorTest, ObjCSymbols) {
const std::string Header = R"(
@interface Person
>From 1b6a09464ff5c7b1988fcb479d0a4ff876f696e6 Mon Sep 17 00:00:00 2001
From: David Goldman
Date: Tue, 26 Dec 2023 16:12:03 -0500
Subject: [PATCH 2/9] Run clang-format
---
.../clangd/unittests/SymbolCollectorTests.cpp | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
index 1d4e1c1d75ea2..5c20b950e4eac 100644
--- a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -560,10 +560,12 @@ TEST_F(SymbolCollectorTest, ObjCRefs) {
haveRanges(Main.ranges("talk");
EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Person::say:").ID,
haveRanges(Main.ranges("say");
- EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbo
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
DavidGoldman wrote: Done https://github.com/llvm/llvm-project/pull/76466 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -538,11 +565,254 @@ std::optional checkName(const NamedDecl
&RenameDecl,
Conflict->getLocation().printToString(ASTCtx.getSourceManager())};
}
}
- if (Result)
+ if (Result) {
InvalidNameMetric.record(1, toString(Result->K));
+return makeError(*Result);
+ }
+ return std::nullopt;
+}
+
+bool isMatchingSelectorName(const syntax::Token &Cur, const syntax::Token
&Next,
+const SourceManager &SM,
+llvm::StringRef SelectorName) {
+ if (SelectorName.empty())
+return Cur.kind() == tok::colon;
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ Cur.text(SM) == SelectorName &&
+ // We require the selector name and : to be contiguous to avoid
+ // potential conflicts with ternary expression.
+ //
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+bool isSelectorLike(const syntax::Token &Cur, const syntax::Token &Next) {
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ // We require the selector name and : to be contiguous.
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+bool parseMessageExpression(llvm::ArrayRef Tokens,
+const SourceManager &SM, unsigned Index,
+unsigned Last, Selector Sel,
+std::vector &SelectorPieces) {
+
+ unsigned NumArgs = Sel.getNumArgs();
+ llvm::SmallVector Closes;
DavidGoldman wrote:
Done
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -538,11 +565,254 @@ std::optional checkName(const NamedDecl
&RenameDecl,
Conflict->getLocation().printToString(ASTCtx.getSourceManager())};
}
}
- if (Result)
+ if (Result) {
InvalidNameMetric.record(1, toString(Result->K));
+return makeError(*Result);
+ }
+ return std::nullopt;
+}
+
+bool isMatchingSelectorName(const syntax::Token &Cur, const syntax::Token
&Next,
+const SourceManager &SM,
+llvm::StringRef SelectorName) {
+ if (SelectorName.empty())
+return Cur.kind() == tok::colon;
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ Cur.text(SM) == SelectorName &&
+ // We require the selector name and : to be contiguous to avoid
+ // potential conflicts with ternary expression.
+ //
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+bool isSelectorLike(const syntax::Token &Cur, const syntax::Token &Next) {
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ // We require the selector name and : to be contiguous.
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+bool parseMessageExpression(llvm::ArrayRef Tokens,
+const SourceManager &SM, unsigned Index,
+unsigned Last, Selector Sel,
+std::vector &SelectorPieces) {
+
+ unsigned NumArgs = Sel.getNumArgs();
+ llvm::SmallVector Closes;
+ SelectorPieces.clear();
+ while (Index < Last) {
+const auto &Tok = Tokens[Index];
+
+if (Closes.empty()) {
+ auto PieceCount = SelectorPieces.size();
+ if (PieceCount < NumArgs &&
+ isMatchingSelectorName(Tok, Tokens[Index + 1], SM,
+ Sel.getNameForSlot(PieceCount))) {
+// If 'foo:' instead of ':' (empty selector), we need to skip the ':'
+// token after the name.
+if (!Sel.getNameForSlot(PieceCount).empty()) {
+ ++Index;
+}
+SelectorPieces.push_back(
+halfOpenToRange(SM, Tok.range(SM).toCharRange(SM)));
+continue;
+ }
+ // If we've found all pieces but the current token looks like another
+ // selector piece, it means the method being renamed is a strict prefix
of
+ // the selector we've found - should be skipped.
+ if (SelectorPieces.size() >= NumArgs &&
+ isSelectorLike(Tok, Tokens[Index + 1]))
+return false;
+}
+
+switch (Tok.kind()) {
+case tok::l_square:
+ Closes.push_back(']');
+ break;
+case tok::l_paren:
+ Closes.push_back(')');
+ break;
+case tok::l_brace:
+ Closes.push_back('}');
+ break;
+case tok::r_square:
+ if (Closes.empty())
+return SelectorPieces.size() == NumArgs;
+
+ if (Closes.back() != ']')
+return false;
+ Closes.pop_back();
+ break;
+case tok::r_paren:
+ if (Closes.empty() || Closes.back() != ')')
+return false;
+ Closes.pop_back();
+ break;
+case tok::r_brace:
+ if (Closes.empty() || Closes.back() != '}')
+return false;
+ Closes.pop_back();
+ break;
+case tok::semi:
+ // top level ; ends all statements.
+ if (Closes.empty())
+return false;
+ break;
+default:
+ break;
+}
+
+++Index;
+ }
+ return false;
+}
+
+/// Collects all ranges of the given identifier/selector in the source code.
+///
+/// If a selector is given, this does a full lex of the given source code in
+/// order to identify all selector fragments (e.g. in method exprs/decls) since
+/// they are non-contiguous.
+std::vector collectRenameIdentifierRanges(
+llvm::StringRef Identifier, llvm::StringRef Content,
+const LangOptions &LangOpts, std::optional Selector) {
+ std::vector Ranges;
+ if (!Selector) {
+auto IdentifierRanges =
+collectIdentifierRanges(Identifier, Content, LangOpts);
+for (const auto &R : IdentifierRanges)
+ Ranges.emplace_back(R);
+return Ranges;
+ }
+ // FIXME: InMemoryFileAdapter crashes unless the buffer is null terminated!
+ std::string NullTerminatedCode = Content.str();
+ SourceManagerForFile FileSM("mock_file_name.cpp", NullTerminatedCode);
+ auto &SM = FileSM.get();
+
+ // We track parens and braces to ensure that we don't accidentally try
parsing
+ // a method declaration or definition which isn't at the top level or similar
+ // looking expressions (e.g. an @selector() expression).
+ unsigned ParenCount = 0;
+ unsigned BraceCount = 0;
+ unsigned NumArgs = Selector->getNumArgs();
+
+ std::vector SelectorPieces;
+ auto Tokens = syntax::tokenize(SM.getMainFileID(), SM, LangOpts);
+ unsigned Last = Tokens.size() - 1;
+ for (unsigned Index = 0; Index < Last; ++Index) {
+const auto &Tok = Tokens[Index];
+
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -538,11 +564,205 @@ std::optional checkName(const NamedDecl
&RenameDecl,
Conflict->getLocation().printToString(ASTCtx.getSourceManager())};
}
}
- if (Result)
+ if (Result) {
InvalidNameMetric.record(1, toString(Result->K));
+return makeError(*Result);
+ }
+ return llvm::Error::success();
+}
+
+bool isSelectorLike(const syntax::Token &Cur, const syntax::Token &Next) {
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ // We require the selector name and : to be contiguous.
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+bool isMatchingSelectorName(const syntax::Token &Cur, const syntax::Token
&Next,
+const SourceManager &SM,
+llvm::StringRef SelectorName) {
+ if (SelectorName.empty())
+return Cur.kind() == tok::colon;
+ return isSelectorLike(Cur, Next) && Cur.text(SM) == SelectorName;
+}
+
+std::vector findAllSelectorPieces(llvm::ArrayRef Tokens,
kadircet wrote:
can we just return a `std::optional` here?
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
https://github.com/kadircet commented: thanks a lot for bearing with me @DavidGoldman ! https://github.com/llvm/llvm-project/pull/76466 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -538,11 +564,205 @@ std::optional checkName(const NamedDecl
&RenameDecl,
Conflict->getLocation().printToString(ASTCtx.getSourceManager())};
}
}
- if (Result)
+ if (Result) {
InvalidNameMetric.record(1, toString(Result->K));
+return makeError(*Result);
+ }
+ return llvm::Error::success();
+}
+
+bool isSelectorLike(const syntax::Token &Cur, const syntax::Token &Next) {
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ // We require the selector name and : to be contiguous.
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+bool isMatchingSelectorName(const syntax::Token &Cur, const syntax::Token
&Next,
+const SourceManager &SM,
+llvm::StringRef SelectorName) {
+ if (SelectorName.empty())
+return Cur.kind() == tok::colon;
+ return isSelectorLike(Cur, Next) && Cur.text(SM) == SelectorName;
+}
+
+std::vector findAllSelectorPieces(llvm::ArrayRef Tokens,
+ const SourceManager &SM, Selector Sel,
+ tok::TokenKind Terminator) {
+
+ unsigned NumArgs = Sel.getNumArgs();
+ llvm::SmallVector Closes;
+ std::vector SelectorPieces;
+ unsigned Last = Tokens.size() - 1;
+ for (unsigned Index = 0; Index < Last; ++Index) {
+const auto &Tok = Tokens[Index];
+
+if (Closes.empty()) {
+ auto PieceCount = SelectorPieces.size();
+ if (PieceCount < NumArgs &&
+ isMatchingSelectorName(Tok, Tokens[Index + 1], SM,
+ Sel.getNameForSlot(PieceCount))) {
+// If 'foo:' instead of ':' (empty selector), we need to skip the ':'
+// token after the name.
+if (!Sel.getNameForSlot(PieceCount).empty()) {
kadircet wrote:
nit: braces
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -538,11 +564,205 @@ std::optional checkName(const NamedDecl
&RenameDecl,
Conflict->getLocation().printToString(ASTCtx.getSourceManager())};
}
}
- if (Result)
+ if (Result) {
InvalidNameMetric.record(1, toString(Result->K));
+return makeError(*Result);
+ }
+ return llvm::Error::success();
+}
+
+bool isSelectorLike(const syntax::Token &Cur, const syntax::Token &Next) {
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ // We require the selector name and : to be contiguous.
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+bool isMatchingSelectorName(const syntax::Token &Cur, const syntax::Token
&Next,
+const SourceManager &SM,
+llvm::StringRef SelectorName) {
+ if (SelectorName.empty())
+return Cur.kind() == tok::colon;
+ return isSelectorLike(Cur, Next) && Cur.text(SM) == SelectorName;
+}
+
+std::vector findAllSelectorPieces(llvm::ArrayRef Tokens,
+ const SourceManager &SM, Selector Sel,
+ tok::TokenKind Terminator) {
+
+ unsigned NumArgs = Sel.getNumArgs();
+ llvm::SmallVector Closes;
+ std::vector SelectorPieces;
+ unsigned Last = Tokens.size() - 1;
+ for (unsigned Index = 0; Index < Last; ++Index) {
+const auto &Tok = Tokens[Index];
+
+if (Closes.empty()) {
+ auto PieceCount = SelectorPieces.size();
+ if (PieceCount < NumArgs &&
+ isMatchingSelectorName(Tok, Tokens[Index + 1], SM,
+ Sel.getNameForSlot(PieceCount))) {
+// If 'foo:' instead of ':' (empty selector), we need to skip the ':'
+// token after the name.
+if (!Sel.getNameForSlot(PieceCount).empty()) {
+ ++Index;
+}
+SelectorPieces.push_back(
+halfOpenToRange(SM, Tok.range(SM).toCharRange(SM)));
+continue;
+ }
+ // If we've found all pieces but the current token looks like another
+ // selector piece, it means the method being renamed is a strict prefix
of
+ // the selector we've found - should be skipped.
+ if (SelectorPieces.size() >= NumArgs &&
+ isSelectorLike(Tok, Tokens[Index + 1]))
+return std::vector();
+}
+
+if (Tok.kind() == Terminator)
kadircet wrote:
shouldn't this also be checked only when `Closes.empty()`?
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
https://github.com/kadircet edited https://github.com/llvm/llvm-project/pull/76466 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -538,11 +564,205 @@ std::optional checkName(const NamedDecl
&RenameDecl,
Conflict->getLocation().printToString(ASTCtx.getSourceManager())};
}
}
- if (Result)
+ if (Result) {
InvalidNameMetric.record(1, toString(Result->K));
+return makeError(*Result);
+ }
+ return llvm::Error::success();
+}
+
+bool isSelectorLike(const syntax::Token &Cur, const syntax::Token &Next) {
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ // We require the selector name and : to be contiguous.
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+bool isMatchingSelectorName(const syntax::Token &Cur, const syntax::Token
&Next,
+const SourceManager &SM,
+llvm::StringRef SelectorName) {
+ if (SelectorName.empty())
+return Cur.kind() == tok::colon;
+ return isSelectorLike(Cur, Next) && Cur.text(SM) == SelectorName;
+}
+
+std::vector findAllSelectorPieces(llvm::ArrayRef Tokens,
+ const SourceManager &SM, Selector Sel,
+ tok::TokenKind Terminator) {
+
+ unsigned NumArgs = Sel.getNumArgs();
+ llvm::SmallVector Closes;
+ std::vector SelectorPieces;
+ unsigned Last = Tokens.size() - 1;
+ for (unsigned Index = 0; Index < Last; ++Index) {
+const auto &Tok = Tokens[Index];
+
+if (Closes.empty()) {
+ auto PieceCount = SelectorPieces.size();
+ if (PieceCount < NumArgs &&
+ isMatchingSelectorName(Tok, Tokens[Index + 1], SM,
+ Sel.getNameForSlot(PieceCount))) {
+// If 'foo:' instead of ':' (empty selector), we need to skip the ':'
+// token after the name.
+if (!Sel.getNameForSlot(PieceCount).empty()) {
+ ++Index;
+}
+SelectorPieces.push_back(
+halfOpenToRange(SM, Tok.range(SM).toCharRange(SM)));
+continue;
+ }
+ // If we've found all pieces but the current token looks like another
+ // selector piece, it means the method being renamed is a strict prefix
of
+ // the selector we've found - should be skipped.
+ if (SelectorPieces.size() >= NumArgs &&
+ isSelectorLike(Tok, Tokens[Index + 1]))
+return std::vector();
+}
+
+if (Tok.kind() == Terminator)
+ return SelectorPieces.size() == NumArgs ? SelectorPieces
+ : std::vector();
+
+switch (Tok.kind()) {
+case tok::l_square:
+ Closes.push_back(tok::r_square);
+ break;
+case tok::l_paren:
+ Closes.push_back(tok::r_paren);
+ break;
+case tok::l_brace:
+ Closes.push_back(tok::r_brace);
+ break;
+case tok::r_square:
+case tok::r_paren:
+case tok::r_brace:
+ if (Closes.empty() || Closes.back() != Tok.kind())
+return std::vector();
+ Closes.pop_back();
+ break;
+case tok::semi:
+ // top level ; terminates all statements.
+ if (Closes.empty())
+return SelectorPieces.size() == NumArgs ? SelectorPieces
+: std::vector();
+ break;
+default:
+ break;
+}
+ }
+ return std::vector();
+}
+
+/// Collects all ranges of the given identifier/selector in the source code.
+///
+/// If a selector is given, this does a full lex of the given source code in
+/// order to identify all selector fragments (e.g. in method exprs/decls) since
+/// they are non-contiguous.
+std::vector collectRenameIdentifierRanges(
+llvm::StringRef Identifier, llvm::StringRef Content,
+const LangOptions &LangOpts, std::optional Selector) {
+ std::vector Ranges;
+ if (!Selector) {
+auto IdentifierRanges =
+collectIdentifierRanges(Identifier, Content, LangOpts);
+for (const auto &R : IdentifierRanges)
+ Ranges.emplace_back(R);
+return Ranges;
+ }
+ // FIXME: InMemoryFileAdapter crashes unless the buffer is null terminated!
+ std::string NullTerminatedCode = Content.str();
+ SourceManagerForFile FileSM("mock_file_name.cpp", NullTerminatedCode);
+ auto &SM = FileSM.get();
+
+ // We track parens and brackets to ensure that we don't accidentally try
+ // parsing a method declaration or definition which isn't at the top level or
+ // similar looking expressions (e.g. an @selector() expression).
+ llvm::SmallVector Closes;
+ llvm::StringRef FirstSelPiece = Selector->getNameForSlot(0);
+
+ std::vector SelectorPieces;
+ std::vector MsgExprPieces;
+ auto Tokens = syntax::tokenize(SM.getMainFileID(), SM, LangOpts);
+ unsigned Last = Tokens.size() - 1;
+ for (unsigned Index = 0; Index < Last; ++Index) {
+const auto &Tok = Tokens[Index];
+
+// Search for the first selector piece to begin a match, but make sure
we're
+// not in () to avoid the @selector(foo:bar:) case.
+if ((Closes.empty() || Closes.back() == tok::r_square)
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -696,7 +982,7 @@ renameOutsideFile(const NamedDecl &RenameDecl, llvm::StringRef MainFilePath, FilePath); } auto RenameEdit = -buildRenameEdit(FilePath, AffectedFileCode, *RenameRanges, NewName); +buildRenameEdit(FilePath, AffectedFileCode, *RenameRanges, NewNames); kadircet wrote: up https://github.com/llvm/llvm-project/pull/76466 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -508,24 +513,46 @@ static bool mayBeValidIdentifier(llvm::StringRef Ident) {
!isAsciiIdentifierStart(Ident.front(), AllowDollar))
return false;
for (char C : Ident) {
+if (AllowColon && C == ':')
+ continue;
if (llvm::isASCII(C) && !isAsciiIdentifierContinue(C, AllowDollar))
return false;
}
return true;
}
+std::string getName(const NamedDecl &RenameDecl) {
+ if (const auto *MD = dyn_cast(&RenameDecl))
+return MD->getSelector().getAsString();
+ if (const auto *ID = RenameDecl.getIdentifier())
+return ID->getName().str();
+ return "";
+}
+
// Check if we can rename the given RenameDecl into NewName.
// Return details if the rename would produce a conflict.
-std::optional checkName(const NamedDecl &RenameDecl,
- llvm::StringRef NewName) {
+std::optional checkName(const NamedDecl &RenameDecl,
+ llvm::StringRef NewName,
+ llvm::StringRef OldName) {
trace::Span Tracer("CheckName");
static constexpr trace::Metric InvalidNameMetric(
"rename_name_invalid", trace::Metric::Counter, "invalid_kind");
+
+ if (OldName == NewName)
+return makeError(ReasonToReject::SameName);
+
+ if (const auto *MD = dyn_cast(&RenameDecl)) {
+const auto Sel = MD->getSelector();
+if (Sel.getNumArgs() != NewName.count(':') &&
+NewName != "__clangd_rename_placeholder")
+ return makeError(InvalidName{InvalidName::BadIdentifier, NewName.str()});
kadircet wrote:
if we want a proper solution, i think it's better to signal that this is a
prepare-rename request via `RenameInputs`. (but this change is already hard to
review, so i'd rather see that as a follow up)
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -538,11 +564,205 @@ std::optional checkName(const NamedDecl
&RenameDecl,
Conflict->getLocation().printToString(ASTCtx.getSourceManager())};
}
}
- if (Result)
+ if (Result) {
InvalidNameMetric.record(1, toString(Result->K));
+return makeError(*Result);
+ }
+ return llvm::Error::success();
+}
+
+bool isSelectorLike(const syntax::Token &Cur, const syntax::Token &Next) {
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ // We require the selector name and : to be contiguous.
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+bool isMatchingSelectorName(const syntax::Token &Cur, const syntax::Token
&Next,
+const SourceManager &SM,
+llvm::StringRef SelectorName) {
+ if (SelectorName.empty())
+return Cur.kind() == tok::colon;
+ return isSelectorLike(Cur, Next) && Cur.text(SM) == SelectorName;
+}
+
+std::vector findAllSelectorPieces(llvm::ArrayRef Tokens,
+ const SourceManager &SM, Selector Sel,
+ tok::TokenKind Terminator) {
+
+ unsigned NumArgs = Sel.getNumArgs();
+ llvm::SmallVector Closes;
+ std::vector SelectorPieces;
+ unsigned Last = Tokens.size() - 1;
+ for (unsigned Index = 0; Index < Last; ++Index) {
+const auto &Tok = Tokens[Index];
+
+if (Closes.empty()) {
+ auto PieceCount = SelectorPieces.size();
+ if (PieceCount < NumArgs &&
+ isMatchingSelectorName(Tok, Tokens[Index + 1], SM,
+ Sel.getNameForSlot(PieceCount))) {
+// If 'foo:' instead of ':' (empty selector), we need to skip the ':'
+// token after the name.
+if (!Sel.getNameForSlot(PieceCount).empty()) {
+ ++Index;
+}
+SelectorPieces.push_back(
+halfOpenToRange(SM, Tok.range(SM).toCharRange(SM)));
+continue;
+ }
+ // If we've found all pieces but the current token looks like another
+ // selector piece, it means the method being renamed is a strict prefix
of
+ // the selector we've found - should be skipped.
+ if (SelectorPieces.size() >= NumArgs &&
+ isSelectorLike(Tok, Tokens[Index + 1]))
+return std::vector();
+}
+
+if (Tok.kind() == Terminator)
+ return SelectorPieces.size() == NumArgs ? SelectorPieces
+ : std::vector();
+
+switch (Tok.kind()) {
+case tok::l_square:
+ Closes.push_back(tok::r_square);
+ break;
+case tok::l_paren:
+ Closes.push_back(tok::r_paren);
+ break;
+case tok::l_brace:
+ Closes.push_back(tok::r_brace);
+ break;
+case tok::r_square:
+case tok::r_paren:
+case tok::r_brace:
+ if (Closes.empty() || Closes.back() != Tok.kind())
+return std::vector();
+ Closes.pop_back();
+ break;
+case tok::semi:
+ // top level ; terminates all statements.
+ if (Closes.empty())
+return SelectorPieces.size() == NumArgs ? SelectorPieces
+: std::vector();
+ break;
+default:
+ break;
+}
+ }
+ return std::vector();
+}
+
+/// Collects all ranges of the given identifier/selector in the source code.
+///
+/// If a selector is given, this does a full lex of the given source code in
+/// order to identify all selector fragments (e.g. in method exprs/decls) since
+/// they are non-contiguous.
+std::vector collectRenameIdentifierRanges(
+llvm::StringRef Identifier, llvm::StringRef Content,
+const LangOptions &LangOpts, std::optional Selector) {
+ std::vector Ranges;
+ if (!Selector) {
+auto IdentifierRanges =
+collectIdentifierRanges(Identifier, Content, LangOpts);
+for (const auto &R : IdentifierRanges)
+ Ranges.emplace_back(R);
+return Ranges;
+ }
+ // FIXME: InMemoryFileAdapter crashes unless the buffer is null terminated!
+ std::string NullTerminatedCode = Content.str();
+ SourceManagerForFile FileSM("mock_file_name.cpp", NullTerminatedCode);
+ auto &SM = FileSM.get();
+
+ // We track parens and brackets to ensure that we don't accidentally try
+ // parsing a method declaration or definition which isn't at the top level or
+ // similar looking expressions (e.g. an @selector() expression).
+ llvm::SmallVector Closes;
+ llvm::StringRef FirstSelPiece = Selector->getNameForSlot(0);
+
+ std::vector SelectorPieces;
+ std::vector MsgExprPieces;
kadircet wrote:
this is not used, right?
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -681,12 +957,22 @@ renameOutsideFile(const NamedDecl &RenameDecl,
llvm::StringRef MainFilePath,
ExpBuffer.getError().message());
continue;
}
+std::string RenameIdentifier = RenameDecl.getNameAsString();
+std::optional Selector = std::nullopt;
+llvm::SmallVector NewNames;
+if (const auto *MD = dyn_cast(&RenameDecl)) {
kadircet wrote:
i'd rather leave that to a follow up change so that we can close the loop here,
if that's OK.
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -538,11 +564,205 @@ std::optional checkName(const NamedDecl
&RenameDecl,
Conflict->getLocation().printToString(ASTCtx.getSourceManager())};
}
}
- if (Result)
+ if (Result) {
InvalidNameMetric.record(1, toString(Result->K));
+return makeError(*Result);
+ }
+ return llvm::Error::success();
+}
+
+bool isSelectorLike(const syntax::Token &Cur, const syntax::Token &Next) {
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ // We require the selector name and : to be contiguous.
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+bool isMatchingSelectorName(const syntax::Token &Cur, const syntax::Token
&Next,
+const SourceManager &SM,
+llvm::StringRef SelectorName) {
+ if (SelectorName.empty())
+return Cur.kind() == tok::colon;
+ return isSelectorLike(Cur, Next) && Cur.text(SM) == SelectorName;
+}
+
+std::vector findAllSelectorPieces(llvm::ArrayRef Tokens,
+ const SourceManager &SM, Selector Sel,
+ tok::TokenKind Terminator) {
+
+ unsigned NumArgs = Sel.getNumArgs();
+ llvm::SmallVector Closes;
+ std::vector SelectorPieces;
+ unsigned Last = Tokens.size() - 1;
+ for (unsigned Index = 0; Index < Last; ++Index) {
+const auto &Tok = Tokens[Index];
+
+if (Closes.empty()) {
+ auto PieceCount = SelectorPieces.size();
+ if (PieceCount < NumArgs &&
+ isMatchingSelectorName(Tok, Tokens[Index + 1], SM,
+ Sel.getNameForSlot(PieceCount))) {
+// If 'foo:' instead of ':' (empty selector), we need to skip the ':'
+// token after the name.
+if (!Sel.getNameForSlot(PieceCount).empty()) {
+ ++Index;
+}
+SelectorPieces.push_back(
+halfOpenToRange(SM, Tok.range(SM).toCharRange(SM)));
+continue;
+ }
+ // If we've found all pieces but the current token looks like another
+ // selector piece, it means the method being renamed is a strict prefix
of
+ // the selector we've found - should be skipped.
+ if (SelectorPieces.size() >= NumArgs &&
+ isSelectorLike(Tok, Tokens[Index + 1]))
+return std::vector();
+}
+
+if (Tok.kind() == Terminator)
+ return SelectorPieces.size() == NumArgs ? SelectorPieces
+ : std::vector();
+
+switch (Tok.kind()) {
+case tok::l_square:
+ Closes.push_back(tok::r_square);
+ break;
+case tok::l_paren:
+ Closes.push_back(tok::r_paren);
+ break;
+case tok::l_brace:
+ Closes.push_back(tok::r_brace);
+ break;
+case tok::r_square:
+case tok::r_paren:
+case tok::r_brace:
+ if (Closes.empty() || Closes.back() != Tok.kind())
+return std::vector();
+ Closes.pop_back();
+ break;
+case tok::semi:
+ // top level ; terminates all statements.
+ if (Closes.empty())
+return SelectorPieces.size() == NumArgs ? SelectorPieces
+: std::vector();
+ break;
+default:
+ break;
+}
+ }
+ return std::vector();
+}
+
+/// Collects all ranges of the given identifier/selector in the source code.
+///
+/// If a selector is given, this does a full lex of the given source code in
+/// order to identify all selector fragments (e.g. in method exprs/decls) since
+/// they are non-contiguous.
+std::vector collectRenameIdentifierRanges(
+llvm::StringRef Identifier, llvm::StringRef Content,
+const LangOptions &LangOpts, std::optional Selector) {
+ std::vector Ranges;
+ if (!Selector) {
+auto IdentifierRanges =
+collectIdentifierRanges(Identifier, Content, LangOpts);
+for (const auto &R : IdentifierRanges)
+ Ranges.emplace_back(R);
+return Ranges;
+ }
+ // FIXME: InMemoryFileAdapter crashes unless the buffer is null terminated!
+ std::string NullTerminatedCode = Content.str();
+ SourceManagerForFile FileSM("mock_file_name.cpp", NullTerminatedCode);
+ auto &SM = FileSM.get();
+
+ // We track parens and brackets to ensure that we don't accidentally try
+ // parsing a method declaration or definition which isn't at the top level or
+ // similar looking expressions (e.g. an @selector() expression).
+ llvm::SmallVector Closes;
+ llvm::StringRef FirstSelPiece = Selector->getNameForSlot(0);
+
+ std::vector SelectorPieces;
+ std::vector MsgExprPieces;
+ auto Tokens = syntax::tokenize(SM.getMainFileID(), SM, LangOpts);
+ unsigned Last = Tokens.size() - 1;
+ for (unsigned Index = 0; Index < Last; ++Index) {
+const auto &Tok = Tokens[Index];
+
+// Search for the first selector piece to begin a match, but make sure
we're
+// not in () to avoid the @selector(foo:bar:) case.
+if ((Closes.empty() || Closes.back() == tok::r_square)
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -538,11 +564,205 @@ std::optional checkName(const NamedDecl
&RenameDecl,
Conflict->getLocation().printToString(ASTCtx.getSourceManager())};
}
}
- if (Result)
+ if (Result) {
InvalidNameMetric.record(1, toString(Result->K));
+return makeError(*Result);
+ }
+ return llvm::Error::success();
+}
+
+bool isSelectorLike(const syntax::Token &Cur, const syntax::Token &Next) {
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ // We require the selector name and : to be contiguous.
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+bool isMatchingSelectorName(const syntax::Token &Cur, const syntax::Token
&Next,
+const SourceManager &SM,
+llvm::StringRef SelectorName) {
+ if (SelectorName.empty())
+return Cur.kind() == tok::colon;
+ return isSelectorLike(Cur, Next) && Cur.text(SM) == SelectorName;
+}
+
+std::vector findAllSelectorPieces(llvm::ArrayRef Tokens,
kadircet wrote:
can we just return a `std::optional` here?
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -538,11 +564,205 @@ std::optional checkName(const NamedDecl
&RenameDecl,
Conflict->getLocation().printToString(ASTCtx.getSourceManager())};
}
}
- if (Result)
+ if (Result) {
InvalidNameMetric.record(1, toString(Result->K));
+return makeError(*Result);
+ }
+ return llvm::Error::success();
+}
+
+bool isSelectorLike(const syntax::Token &Cur, const syntax::Token &Next) {
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ // We require the selector name and : to be contiguous.
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+bool isMatchingSelectorName(const syntax::Token &Cur, const syntax::Token
&Next,
+const SourceManager &SM,
+llvm::StringRef SelectorName) {
+ if (SelectorName.empty())
+return Cur.kind() == tok::colon;
+ return isSelectorLike(Cur, Next) && Cur.text(SM) == SelectorName;
+}
+
+std::vector findAllSelectorPieces(llvm::ArrayRef Tokens,
kadircet wrote:
can you add some comments describing parameters and what the function does?
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -538,11 +564,205 @@ std::optional checkName(const NamedDecl
&RenameDecl,
Conflict->getLocation().printToString(ASTCtx.getSourceManager())};
}
}
- if (Result)
+ if (Result) {
InvalidNameMetric.record(1, toString(Result->K));
+return makeError(*Result);
+ }
+ return llvm::Error::success();
+}
+
+bool isSelectorLike(const syntax::Token &Cur, const syntax::Token &Next) {
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ // We require the selector name and : to be contiguous.
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+bool isMatchingSelectorName(const syntax::Token &Cur, const syntax::Token
&Next,
+const SourceManager &SM,
+llvm::StringRef SelectorName) {
+ if (SelectorName.empty())
+return Cur.kind() == tok::colon;
+ return isSelectorLike(Cur, Next) && Cur.text(SM) == SelectorName;
+}
+
+std::vector findAllSelectorPieces(llvm::ArrayRef Tokens,
DavidGoldman wrote:
Done
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -53,13 +55,34 @@ struct RenameInputs {
struct RenameResult {
// The range of the symbol that the user can attempt to rename.
Range Target;
+ // Placeholder text for the rename operation if non-empty.
+ std::string Placeholder;
// Rename occurrences for the current main file.
std::vector LocalChanges;
// Complete edits for the rename, including LocalChanges.
// If the full set of changes is unknown, this field is empty.
FileEdits GlobalChanges;
};
+/// Represents a symbol range where the symbol can potentially have multiple
+/// tokens.
+struct SymbolRange {
+ /// Ranges for the tokens that make up the symbol's name.
+ /// Usually a single range, but there can be multiple ranges if the tokens
for
+ /// the symbol are split, e.g. ObjC selectors.
+ std::vector Ranges;
DavidGoldman wrote:
That makes sense, I think we might need to improve the empty selector case -
I'll do that in a follow up.
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -569,8 +840,13 @@ renameWithinFile(ParsedAST &AST, const NamedDecl &RenameDecl, // } if (!isInsideMainFile(RenameLoc, SM)) continue; +Locs.push_back(RenameLoc); + } + if (const auto *MD = dyn_cast(&RenameDecl)) +return renameObjCMethodWithinFile(AST, MD, NewName, std::move(Locs)); DavidGoldman wrote: SGTM, I'll look into this in a follow up (+ add some tests for that). https://github.com/llvm/llvm-project/pull/76466 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
DavidGoldman wrote: Thanks, PTAL, I'll save the remaining comments for follow ups. https://github.com/llvm/llvm-project/pull/76466 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
https://github.com/DavidGoldman updated
https://github.com/llvm/llvm-project/pull/76466
>From 4caf5b3c779bf18236b4b0be5bc7147d10339f2b Mon Sep 17 00:00:00 2001
From: David Goldman
Date: Tue, 26 Dec 2023 15:59:01 -0500
Subject: [PATCH 1/4] [clangd][SymbolCollector] Treat ObjC methods as spelled
We'll treat multi-arg methods as spelled once we have full rename
support for them.
---
.../clangd/index/SymbolCollector.cpp | 6 ++-
.../clangd/unittests/SymbolCollectorTests.cpp | 42 +++
2 files changed, 47 insertions(+), 1 deletion(-)
diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp
b/clang-tools-extra/clangd/index/SymbolCollector.cpp
index 7ef4b15febad22f..336bc3506bb3608 100644
--- a/clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -174,7 +174,9 @@ bool isSpelled(SourceLocation Loc, const NamedDecl &ND) {
auto Name = ND.getDeclName();
const auto NameKind = Name.getNameKind();
if (NameKind != DeclarationName::Identifier &&
- NameKind != DeclarationName::CXXConstructorName)
+ NameKind != DeclarationName::CXXConstructorName &&
+ NameKind != DeclarationName::ObjCZeroArgSelector &&
+ NameKind != DeclarationName::ObjCOneArgSelector)
return false;
const auto &AST = ND.getASTContext();
const auto &SM = AST.getSourceManager();
@@ -183,6 +185,8 @@ bool isSpelled(SourceLocation Loc, const NamedDecl &ND) {
if (clang::Lexer::getRawToken(Loc, Tok, SM, LO))
return false;
auto StrName = Name.getAsString();
+ if (const auto *MD = dyn_cast(&ND))
+StrName = MD->getSelector().getNameForSlot(0).str();
return clang::Lexer::getSpelling(Tok, SM, LO) == StrName;
}
} // namespace
diff --git a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
index 9cdc57ec01f3276..1d4e1c1d75ea230 100644
--- a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -105,6 +105,9 @@ MATCHER(refRange, "") {
MATCHER_P2(OverriddenBy, Subject, Object, "") {
return arg == Relation{Subject.ID, RelationKind::OverriddenBy, Object.ID};
}
+MATCHER(isSpelled, "") {
+ return static_cast(arg.Kind & RefKind::Spelled);
+}
::testing::Matcher &>
haveRanges(const std::vector Ranges) {
return ::testing::UnorderedPointwise(refRange(), Ranges);
@@ -524,6 +527,45 @@ TEST_F(SymbolCollectorTest, templateArgs) {
forCodeCompletion(false);
}
+TEST_F(SymbolCollectorTest, ObjCRefs) {
+ Annotations Header(R"(
+ @interface Person
+ - (void)$talk[[talk]];
+ - (void)$say[[say]]:(id)something;
+ @end
+ @interface Person (Category)
+ - (void)categoryMethod;
+ - (void)multiArg:(id)a method:(id)b;
+ @end
+ )");
+ Annotations Main(R"(
+ @implementation Person
+ - (void)$talk[[talk]] {}
+ - (void)$say[[say]]:(id)something {}
+ @end
+
+ void fff(Person *p) {
+[p $talk[[talk]]];
+[p $say[[say]]:0];
+[p categoryMethod];
+[p multiArg:0 method:0];
+ }
+ )");
+ CollectorOpts.RefFilter = RefKind::All;
+ CollectorOpts.CollectMainFileRefs = true;
+ TestFileName = testPath("test.m");
+ runSymbolCollector(Header.code(), Main.code(),
+ {"-fblocks", "-xobjective-c++", "-Wno-objc-root-class"});
+ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Person::talk").ID,
+ haveRanges(Main.ranges("talk");
+ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Person::say:").ID,
+ haveRanges(Main.ranges("say");
+ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols,
"Person::categoryMethod").ID,
+ ElementsAre(isSpelled();
+ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols,
"Person::multiArg:method:").ID,
+ ElementsAre(Not(isSpelled());
+}
+
TEST_F(SymbolCollectorTest, ObjCSymbols) {
const std::string Header = R"(
@interface Person
>From 1b6a09464ff5c7b1988fcb479d0a4ff876f696e6 Mon Sep 17 00:00:00 2001
From: David Goldman
Date: Tue, 26 Dec 2023 16:12:03 -0500
Subject: [PATCH 2/4] Run clang-format
---
.../clangd/unittests/SymbolCollectorTests.cpp | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
index 1d4e1c1d75ea230..5c20b950e4eac0d 100644
--- a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -560,10 +560,12 @@ TEST_F(SymbolCollectorTest, ObjCRefs) {
haveRanges(Main.ranges("talk");
EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Person::say:").ID,
haveRanges(Main.ranges("say");
- EXPECT_THAT(Refs, Contains(Pair(find
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -893,22 +964,36 @@ llvm::Expected buildRenameEdit(llvm::StringRef
AbsFilePath,
return LastOffset;
};
- std::vector> OccurrencesOffsets;
- for (const auto &R : Occurrences) {
-auto StartOffset = Offset(R.start);
-if (!StartOffset)
- return StartOffset.takeError();
-auto EndOffset = Offset(R.end);
-if (!EndOffset)
- return EndOffset.takeError();
-OccurrencesOffsets.push_back({*StartOffset, *EndOffset});
+ struct OccurrenceOffset {
+size_t Start;
+size_t End;
+llvm::StringRef NewName;
+
+OccurrenceOffset(size_t Start, size_t End, llvm::StringRef NewName) :
+ Start(Start), End(End), NewName(NewName) {}
+ };
+
+ std::vector OccurrencesOffsets;
+ for (const auto &SR : Occurrences) {
+for (auto It = SR.Ranges.begin(); It != SR.Ranges.end(); ++It) {
DavidGoldman wrote:
Done
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -681,12 +718,26 @@ renameOutsideFile(const NamedDecl &RenameDecl,
llvm::StringRef MainFilePath,
ExpBuffer.getError().message());
continue;
}
+std::string RenameIdentifier = RenameDecl.getNameAsString();
+std::optional Selector = std::nullopt;
+llvm::SmallVector NewNames;
+if (const auto *MD = dyn_cast(&RenameDecl)) {
+ if (MD->getSelector().getNumArgs() > 1) {
+RenameIdentifier = MD->getSelector().getNameForSlot(0).str();
+Selector = MD->getSelector();
+NewName.split(NewNames, ":");
kadircet wrote:
you can perform the split unconditionally ,rather than pushing to `NewNames`
below.
also is it possible to have "empty" selector segments? (e.g. is a NewName like
`foo::bar` valid for a selector with 3 segments? if not we should probably
raise an error when validating)
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -543,6 +550,45 @@ std::optional checkName(const NamedDecl
&RenameDecl,
return Result;
}
+clangd::Range tokenRangeForLoc(ParsedAST &AST, SourceLocation TokLoc,
+ const SourceManager &SM,
+ const LangOptions &LangOpts) {
+ const auto *Token = AST.getTokens().spelledTokenAt(TokLoc);
+ assert(Token && "got inclusion at wrong offset");
+ clangd::Range Result;
+ Result.start = sourceLocToPosition(SM, Token->location());
+ Result.end = sourceLocToPosition(SM, Token->endLocation());
+ return Result;
+}
+
+// AST-based ObjC method rename, it renames all occurrences in the main file
+// even for selectors which may have multiple tokens.
+llvm::Expected
+renameObjCMethodWithinFile(ParsedAST &AST, const ObjCMethodDecl *MD,
+ llvm::StringRef NewName,
+ std::vector Locs) {
+ const SourceManager &SM = AST.getSourceManager();
+ auto Code = SM.getBufferData(SM.getMainFileID());
+ auto RenameIdentifier = MD->getSelector().getNameForSlot(0).str();
+ llvm::SmallVector NewNames;
+ NewName.split(NewNames, ":");
+ if (NewNames.empty())
+NewNames.push_back(NewName);
+
+ std::vector Ranges;
+ const auto &LangOpts = MD->getASTContext().getLangOpts();
+ for (const auto &Loc : Locs)
+Ranges.push_back(tokenRangeForLoc(AST, Loc, SM, LangOpts));
+ auto FilePath = AST.tuPath();
+ auto RenameRanges = collectRenameIdentifierRanges(
kadircet wrote:
hmm we don't re-use the `Ranges`/`Locs` here at all. let's strip it all (or use
it to guide lexing, instead of searching for selector identifier, just check if
selectors at these locations are what we need ?)
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -817,7 +939,8 @@ llvm::Expected rename(const RenameInputs
&RInputs) {
return StartOffset.takeError();
if (!EndOffset)
return EndOffset.takeError();
- if (llvm::none_of(
+ if (RenamingCurToken &&
kadircet wrote:
as mentioned above, let's just check with
`shouldRenameTriggeringLocation(RenameDecl)`
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -778,12 +868,44 @@ llvm::Expected rename(const RenameInputs
&RInputs) {
return makeError(ReasonToReject::NoSymbolFound);
if (DeclsUnderCursor.size() > 1)
return makeError(ReasonToReject::AmbiguousSymbol);
+ std::string Placeholder;
+ // We expect the token under the cursor to be changed unless the user is
+ // renaming an Objective-C selector with multiple pieces and only renames
+ // some of the selector piece(s).
+ bool RenamingCurToken = true;
const auto &RenameDecl = **DeclsUnderCursor.begin();
- const auto *ID = RenameDecl.getIdentifier();
- if (!ID)
-return makeError(ReasonToReject::UnsupportedSymbol);
- if (ID->getName() == RInputs.NewName)
-return makeError(ReasonToReject::SameName);
+ if (const auto *MD = dyn_cast(&RenameDecl)) {
+const auto Sel = MD->getSelector();
+if (Sel.getAsString() == RInputs.NewName)
+ return makeError(ReasonToReject::SameName);
+if (Sel.getNumArgs() != RInputs.NewName.count(':') &&
+RInputs.NewName != "__clangd_rename_placeholder")
+ return makeError(
+ InvalidName{InvalidName::BadIdentifier, RInputs.NewName.str()});
+if (Sel.getNumArgs() > 1)
+ Placeholder = Sel.getAsString();
+
+// See if the token under the cursor should actually be renamed.
+if (RInputs.NewName != "__clangd_rename_placeholder") {
+ llvm::StringRef NewName = RInputs.NewName;
+ llvm::SmallVector NewNames;
+ NewName.split(NewNames, ":");
+
+ unsigned NumSelectorLocs = MD->getNumSelectorLocs();
+ for (unsigned I = 0; I < NumSelectorLocs; ++I) {
+if (MD->getSelectorLoc(I) == IdentifierToken->location()) {
+ RenamingCurToken = Sel.getNameForSlot(I) != NewNames[I];
kadircet wrote:
i don't think it's worth all this trouble to leave things in a semi-working
state. we might rely on `CurrentIdentifier` or `IdentifierRange` in other
places, and until we change this logic to actually amend the identifer, it's
easier to assume that it'll always be invalid for objc.
let's leave a TODO to figure out the relevant selector range using
`ObjCMessageExpr` under the cursor and change the safety check to not happen
for ObjCMethodDecls for now?
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -1017,9 +1159,10 @@ size_t renameRangeAdjustmentCost(ArrayRef
Indexed, ArrayRef Lexed,
int LastDLine = 0, LastDColumn = 0;
int Cost = 0;
for (size_t I = 0; I < Indexed.size(); ++I) {
-int DLine = Indexed[I].start.line - Lexed[MappedIndex[I]].start.line;
-int DColumn =
-Indexed[I].start.character - Lexed[MappedIndex[I]].start.character;
+int DLine =
+Indexed[I].start.line - Lexed[MappedIndex[I]].range().start.line;
+int DColumn = Indexed[I].start.character -
+ Lexed[MappedIndex[I]].range().start.character;
kadircet wrote:
looks like just formatting change, can you revert?
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -746,6 +812,30 @@ void findNearMiss(
} // namespace
+SymbolRange::SymbolRange(Range R) : Ranges({R}) {}
+
+SymbolRange::SymbolRange(std::vector Ranges)
+: Ranges(std::move(Ranges)) {}
+
+Range SymbolRange::range() const { return Ranges.front(); }
+
+bool operator==(const SymbolRange &LHS, const SymbolRange &RHS) {
+ return LHS.Ranges == RHS.Ranges;
+}
+bool operator!=(const SymbolRange &LHS, const SymbolRange &RHS) {
+ return !(LHS == RHS);
+}
+bool operator<(const SymbolRange &LHS, const SymbolRange &RHS) {
+ return LHS.range() < RHS.range();
+}
+
+std::vector symbolRanges(const std::vector Ranges) {
kadircet wrote:
can we take in `llvm::ArrayRef Ranges`, also move it to renametests, i
don't think it's used elswhere
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
