[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)
@@ -538,11 +564,222 @@ std::optional checkName(const NamedDecl
,
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 , const syntax::Token ) {
+ 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 , const syntax::Token
,
+const SourceManager ,
+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 , 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)
@@ -538,11 +564,222 @@ std::optional checkName(const NamedDecl
,
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 , const syntax::Token ) {
+ 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 , const syntax::Token
,
+const SourceManager ,
+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 , 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 = 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)
@@ -696,7 +982,7 @@ renameOutsideFile(const NamedDecl , 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
,
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 , const syntax::Token ) {
+ 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 , const syntax::Token
,
+const SourceManager ,
+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 , 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)
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
,
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 , const syntax::Token ) {
+ 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 , const syntax::Token
,
+const SourceManager ,
+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 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
,
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 , const syntax::Token ) {
+ 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)
@@ -538,11 +564,222 @@ std::optional checkName(const NamedDecl
,
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 , const syntax::Token ) {
+ 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 , const syntax::Token
,
+const SourceManager ,
+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 , 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 = 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 , std::optional Selector) {
+ std::vector Ranges;
+ if (!Selector) {
+auto IdentifierRanges =
+collectIdentifierRanges(Identifier, Content, LangOpts);
+for (const auto : 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
,
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 , const syntax::Token ) {
+ 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 , const syntax::Token
,
+const SourceManager ,
+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)
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)
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)
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)
@@ -569,8 +840,13 @@ renameWithinFile(ParsedAST , const NamedDecl , // } if (!isInsideMainFile(RenameLoc, SM)) continue; +Locs.push_back(RenameLoc); + } + if (const auto *MD = dyn_cast()) +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)
@@ -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)
@@ -538,11 +564,205 @@ std::optional checkName(const NamedDecl
,
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 , const syntax::Token ) {
+ 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 , const syntax::Token
,
+const SourceManager ,
+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)
@@ -538,11 +564,205 @@ std::optional checkName(const NamedDecl
,
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 , const syntax::Token ) {
+ 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 , const syntax::Token
,
+const SourceManager ,
+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
,
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 , const syntax::Token ) {
+ 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 , const syntax::Token
,
+const SourceManager ,
+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
,
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 , const syntax::Token ) {
+ 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 , const syntax::Token
,
+const SourceManager ,
+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 , 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 = 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 , std::optional Selector) {
+ std::vector Ranges;
+ if (!Selector) {
+auto IdentifierRanges =
+collectIdentifierRanges(Identifier, Content, LangOpts);
+for (const auto : 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 = 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 = 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) &&
+isMatchingSelectorName(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 ,
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()) {
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
,
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 , const syntax::Token ) {
+ 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 , const syntax::Token
,
+const SourceManager ,
+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 , 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 = 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 , std::optional Selector) {
+ std::vector Ranges;
+ if (!Selector) {
+auto IdentifierRanges =
+collectIdentifierRanges(Identifier, Content, LangOpts);
+for (const auto : 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 = 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)
@@ -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 ) {
+ if (const auto *MD = dyn_cast())
+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 ,
- llvm::StringRef NewName) {
+std::optional checkName(const NamedDecl ,
+ 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()) {
+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)
@@ -696,7 +982,7 @@ renameOutsideFile(const NamedDecl , 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)
@@ -538,11 +564,205 @@ std::optional checkName(const NamedDecl
,
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 , const syntax::Token ) {
+ 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 , const syntax::Token
,
+const SourceManager ,
+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 , 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 = 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 , std::optional Selector) {
+ std::vector Ranges;
+ if (!Selector) {
+auto IdentifierRanges =
+collectIdentifierRanges(Identifier, Content, LangOpts);
+for (const auto : 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 = 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 = 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) &&
+isMatchingSelectorName(Tok, Tokens[Index +
[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
,
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 , const syntax::Token ) {
+ 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 , const syntax::Token
,
+const SourceManager ,
+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 , 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 = 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)
@@ -538,11 +564,205 @@ std::optional checkName(const NamedDecl
,
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 , const syntax::Token ) {
+ 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 , const syntax::Token
,
+const SourceManager ,
+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 , 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 = 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)
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
,
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 , const syntax::Token ) {
+ 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 , const syntax::Token
,
+const SourceManager ,
+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 +565,254 @@ std::optional checkName(const NamedDecl
,
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 , const syntax::Token
,
+const SourceManager ,
+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 , const syntax::Token ) {
+ 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 , unsigned Index,
+unsigned Last, Selector Sel,
+std::vector ) {
+
+ unsigned NumArgs = Sel.getNumArgs();
+ llvm::SmallVector Closes;
+ SelectorPieces.clear();
+ while (Index < Last) {
+const auto = 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 , std::optional Selector) {
+ std::vector Ranges;
+ if (!Selector) {
+auto IdentifierRanges =
+collectIdentifierRanges(Identifier, Content, LangOpts);
+for (const auto : 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 = 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 = Tokens[Index];
+
DavidGoldman wrote:
Done. This lexer logic runs independent of those
[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
,
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 , const syntax::Token
,
+const SourceManager ,
+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 , const syntax::Token ) {
+ 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 , unsigned Index,
+unsigned Last, Selector Sel,
+std::vector ) {
+
+ 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)
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/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 ) {
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 = ND.getASTContext();
const auto = AST.getSourceManager();
@@ -183,6 +185,8 @@ bool isSpelled(SourceLocation Loc, const NamedDecl ) {
if (clang::Lexer::getRawToken(Loc, Tok, SM, LO))
return false;
auto StrName = Name.getAsString();
+ if (const auto *MD = dyn_cast())
+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(Symbols,
[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 ) {
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 = ND.getASTContext();
const auto = AST.getSourceManager();
@@ -183,6 +185,8 @@ bool isSpelled(SourceLocation Loc, const NamedDecl ) {
if (clang::Lexer::getRawToken(Loc, Tok, SM, LO))
return false;
auto StrName = Name.getAsString();
+ if (const auto *MD = dyn_cast())
+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(Symbols,
[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
,
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 , const syntax::Token
,
+const SourceManager ,
+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)
@@ -538,11 +565,254 @@ std::optional checkName(const NamedDecl
,
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 , const syntax::Token
,
+const SourceManager ,
+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 , const syntax::Token ) {
+ 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 , unsigned Index,
+unsigned Last, Selector Sel,
+std::vector ) {
+
+ unsigned NumArgs = Sel.getNumArgs();
+ llvm::SmallVector Closes;
+ SelectorPieces.clear();
+ while (Index < Last) {
+const auto = 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 , std::optional Selector) {
+ std::vector Ranges;
+ if (!Selector) {
+auto IdentifierRanges =
+collectIdentifierRanges(Identifier, Content, LangOpts);
+for (const auto : 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 = 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 `collectRenameIdentifierRanges` at the moment, one of which already
has the tokens and one
[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
,
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 , const syntax::Token
,
+const SourceManager ,
+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 , const syntax::Token ) {
+ 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 , unsigned Index,
+unsigned Last, Selector Sel,
+std::vector ) {
+
+ unsigned NumArgs = Sel.getNumArgs();
+ llvm::SmallVector Closes;
+ SelectorPieces.clear();
+ while (Index < Last) {
+const auto = 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 , std::optional Selector) {
+ std::vector Ranges;
+ if (!Selector) {
+auto IdentifierRanges =
+collectIdentifierRanges(Identifier, Content, LangOpts);
+for (const auto : 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 = 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 = Tokens[Index];
+
+if (BraceCount == 0 && ParenCount == 0) {
+ auto PieceCount =
[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
,
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 , const syntax::Token
,
+const SourceManager ,
+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 , const syntax::Token ) {
+ 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 , unsigned Index,
+unsigned Last, Selector Sel,
+std::vector ) {
+
+ unsigned NumArgs = Sel.getNumArgs();
+ llvm::SmallVector Closes;
+ SelectorPieces.clear();
+ while (Index < Last) {
+const auto = 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 , std::optional Selector) {
+ std::vector Ranges;
+ if (!Selector) {
+auto IdentifierRanges =
+collectIdentifierRanges(Identifier, Content, LangOpts);
+for (const auto : 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 = 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 = Tokens[Index];
+
ahoppen wrote:
But shouldn’t we know based on the index data that `-
[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
,
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 , const syntax::Token
,
+const SourceManager ,
+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 , const syntax::Token ) {
+ 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 , unsigned Index,
+unsigned Last, Selector Sel,
+std::vector ) {
+
+ unsigned NumArgs = Sel.getNumArgs();
+ llvm::SmallVector Closes;
+ SelectorPieces.clear();
+ while (Index < Last) {
+const auto = 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 , std::optional Selector) {
+ std::vector Ranges;
+ if (!Selector) {
+auto IdentifierRanges =
+collectIdentifierRanges(Identifier, Content, LangOpts);
+for (const auto : 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 = 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 the caller. I could pass an optional Array ref
of tokens to use to
[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
,
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 , const syntax::Token
,
+const SourceManager ,
+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 , const syntax::Token ) {
+ 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 , unsigned Index,
+unsigned Last, Selector Sel,
+std::vector ) {
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
,
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 , const syntax::Token
,
+const SourceManager ,
+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 , const syntax::Token ) {
+ 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 , unsigned Index,
+unsigned Last, Selector Sel,
+std::vector ) {
+
+ unsigned NumArgs = Sel.getNumArgs();
+ llvm::SmallVector Closes;
+ SelectorPieces.clear();
+ while (Index < Last) {
+const auto = 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 , std::optional Selector) {
+ std::vector Ranges;
+ if (!Selector) {
+auto IdentifierRanges =
+collectIdentifierRanges(Identifier, Content, LangOpts);
+for (const auto : 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 = 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 = Tokens[Index];
+
+if (BraceCount == 0 && ParenCount == 0) {
+ auto PieceCount =
[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
,
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 , const syntax::Token
,
+const SourceManager ,
+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 , const syntax::Token ) {
+ 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 , unsigned Index,
+unsigned Last, Selector Sel,
+std::vector ) {
+
+ unsigned NumArgs = Sel.getNumArgs();
+ llvm::SmallVector Closes;
+ SelectorPieces.clear();
+ while (Index < Last) {
+const auto = 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 , std::optional Selector) {
+ std::vector Ranges;
+ if (!Selector) {
+auto IdentifierRanges =
+collectIdentifierRanges(Identifier, Content, LangOpts);
+for (const auto : 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 = 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 = Tokens[Index];
+
DavidGoldman wrote:
The main edge case I mentioned is having one
[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 ) {
+ if (const auto *MD = dyn_cast())
+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 ,
- llvm::StringRef NewName) {
+std::optional checkName(const NamedDecl ,
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/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)
@@ -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)
@@ -538,11 +565,254 @@ std::optional checkName(const NamedDecl
,
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 , const syntax::Token
,
+const SourceManager ,
+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 , const syntax::Token ) {
+ 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 , unsigned Index,
+unsigned Last, Selector Sel,
+std::vector ) {
+
+ 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)
@@ -538,11 +565,254 @@ std::optional checkName(const NamedDecl
,
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 , const syntax::Token
,
+const SourceManager ,
+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)
@@ -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)
@@ -569,8 +840,13 @@ renameWithinFile(ParsedAST , const NamedDecl , // } if (!isInsideMainFile(RenameLoc, SM)) continue; +Locs.push_back(RenameLoc); + } + if (const auto *MD = dyn_cast()) +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 == ':')
+ continue;
if (llvm::isASCII(C) && !isAsciiIdentifierContinue(C, AllowDollar))
return false;
}
return true;
}
+std::string getName(const NamedDecl ) {
+ if (const auto *MD = dyn_cast())
+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 ,
- llvm::StringRef NewName) {
+std::optional checkName(const NamedDecl ,
+ 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()) {
+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)
@@ -538,11 +565,254 @@ std::optional checkName(const NamedDecl
,
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 , const syntax::Token
,
+const SourceManager ,
+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 , const syntax::Token ) {
+ 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 , unsigned Index,
+unsigned Last, Selector Sel,
+std::vector ) {
+
+ unsigned NumArgs = Sel.getNumArgs();
+ llvm::SmallVector Closes;
+ SelectorPieces.clear();
+ while (Index < Last) {
+const auto = 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 , std::optional Selector) {
+ std::vector Ranges;
+ if (!Selector) {
+auto IdentifierRanges =
+collectIdentifierRanges(Identifier, Content, LangOpts);
+for (const auto : 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 = 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 we shouldn’t need to re-parse the file.
[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 ) {
+ if (const auto *MD = dyn_cast())
+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 ,
- llvm::StringRef NewName) {
+std::optional checkName(const NamedDecl ,
+ 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()) {
+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)
@@ -681,12 +957,22 @@ renameOutsideFile(const NamedDecl , 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)
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 ,
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()) {
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)
@@ -538,11 +565,254 @@ std::optional checkName(const NamedDecl
,
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 , const syntax::Token
,
+const SourceManager ,
+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 , const syntax::Token ) {
+ 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)
@@ -538,11 +565,254 @@ std::optional checkName(const NamedDecl
,
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 , const syntax::Token
,
+const SourceManager ,
+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 , const syntax::Token ) {
+ 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 , unsigned Index,
+unsigned Last, Selector Sel,
+std::vector ) {
+
+ unsigned NumArgs = Sel.getNumArgs();
+ llvm::SmallVector Closes;
+ SelectorPieces.clear();
+ while (Index < Last) {
+const auto = 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 , std::optional Selector) {
+ std::vector Ranges;
+ if (!Selector) {
+auto IdentifierRanges =
+collectIdentifierRanges(Identifier, Content, LangOpts);
+for (const auto : 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 = 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 = Tokens[Index];
+
+if (BraceCount == 0 && ParenCount == 0) {
+ auto PieceCount =
[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
,
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 , const syntax::Token
,
+const SourceManager ,
+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 , const syntax::Token ) {
+ 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 , unsigned Index,
+unsigned Last, Selector Sel,
+std::vector ) {
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)
@@ -696,7 +982,7 @@ renameOutsideFile(const NamedDecl , 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)
@@ -543,6 +550,45 @@ std::optional checkName(const NamedDecl
,
return Result;
}
+clangd::Range tokenRangeForLoc(ParsedAST , SourceLocation TokLoc,
+ const SourceManager ,
+ const LangOptions ) {
+ 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 , const ObjCMethodDecl *MD,
+ llvm::StringRef NewName,
+ std::vector Locs) {
+ const SourceManager = 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 = MD->getASTContext().getLangOpts();
+ for (const auto : 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)
@@ -538,11 +565,254 @@ std::optional checkName(const NamedDecl
,
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 , const syntax::Token
,
+const SourceManager ,
+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 , const syntax::Token ) {
+ 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 , unsigned Index,
+unsigned Last, Selector Sel,
+std::vector ) {
+
+ unsigned NumArgs = Sel.getNumArgs();
+ llvm::SmallVector Closes;
+ SelectorPieces.clear();
+ while (Index < Last) {
+const auto = 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 , std::optional Selector) {
+ std::vector Ranges;
+ if (!Selector) {
+auto IdentifierRanges =
+collectIdentifierRanges(Identifier, Content, LangOpts);
+for (const auto : 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 = 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 = Tokens[Index];
+
kadircet wrote:
Sorry, i am still having a lot of trouble following
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -681,12 +957,22 @@ renameOutsideFile(const NamedDecl ,
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()) {
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
,
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 , const syntax::Token
,
+const SourceManager ,
+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 , const syntax::Token ) {
+ 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 , unsigned Index,
+unsigned Last, Selector Sel,
+std::vector ) {
+
+ unsigned NumArgs = Sel.getNumArgs();
+ llvm::SmallVector Closes;
+ SelectorPieces.clear();
+ while (Index < Last) {
+const auto = 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 , std::optional Selector) {
+ std::vector Ranges;
+ if (!Selector) {
+auto IdentifierRanges =
+collectIdentifierRanges(Identifier, Content, LangOpts);
+for (const auto : 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 = 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 = Tokens[Index];
+
kadircet wrote:
this might be missing your concerns around having
[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 ) {
+ if (const auto *MD = dyn_cast())
+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 ,
- llvm::StringRef NewName) {
+std::optional checkName(const NamedDecl ,
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)
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 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 ) {
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 = ND.getASTContext();
const auto = AST.getSourceManager();
@@ -183,6 +185,8 @@ bool isSpelled(SourceLocation Loc, const NamedDecl ) {
if (clang::Lexer::getRawToken(Loc, Tok, SM, LO))
return false;
auto StrName = Name.getAsString();
+ if (const auto *MD = dyn_cast())
+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/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 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(findSymbol(Symbols,
[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 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 ) {
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 = ND.getASTContext();
const auto = AST.getSourceManager();
@@ -183,6 +185,8 @@ bool isSpelled(SourceLocation Loc, const NamedDecl ) {
if (clang::Lexer::getRawToken(Loc, Tok, SM, LO))
return false;
auto StrName = Name.getAsString();
+ if (const auto *MD = dyn_cast())
+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/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 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(findSymbol(Symbols,
[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 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 ) {
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 = ND.getASTContext();
const auto = AST.getSourceManager();
@@ -183,6 +185,8 @@ bool isSpelled(SourceLocation Loc, const NamedDecl ) {
if (clang::Lexer::getRawToken(Loc, Tok, SM, LO))
return false;
auto StrName = Name.getAsString();
+ if (const auto *MD = dyn_cast())
+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/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 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(findSymbol(Symbols,
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -38,6 +38,11 @@
namespace clang {
namespace clangd {
+
+std::vector collectRenameIdentifierRanges(
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)
@@ -1029,5 +1172,151 @@ size_t renameRangeAdjustmentCost(ArrayRef
Indexed, ArrayRef Lexed,
return Cost;
}
+static bool isMatchingSelectorName(const syntax::Token ,
+ const syntax::Token ,
+ const SourceManager ,
+ 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.
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+static bool isSelectorLike(const syntax::Token ,
+ const syntax::Token ) {
+ 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();
+}
+
+static void
+lex(llvm::StringRef Code, const LangOptions ,
+llvm::function_ref
+Action) {
+ // FIXME: InMemoryFileAdapter crashes unless the buffer is null terminated!
+ std::string NullTerminatedCode = Code.str();
+ SourceManagerForFile FileSM("mock_file_name.cpp", NullTerminatedCode);
+ auto = FileSM.get();
+ for (const auto : syntax::tokenize(SM.getMainFileID(), SM, LangOpts))
+Action(Tok, SM);
+}
+
+std::vector collectRenameIdentifierRanges(
+llvm::StringRef Identifier, llvm::StringRef Content,
+const LangOptions , std::optional Selector) {
+ std::vector Ranges;
+ if (!Selector) {
+lex(Content, LangOpts,
+[&](const syntax::Token , const SourceManager ) {
+ if (Tok.kind() != tok::identifier || Tok.text(SM) != Identifier)
+return;
+ Ranges.emplace_back(
+ halfOpenToRange(SM, Tok.range(SM).toCharRange(SM)));
+});
+return Ranges;
+ }
+ // FIXME: InMemoryFileAdapter crashes unless the buffer is null terminated!
+ std::string NullTerminatedCode = Content.str();
+ SourceManagerForFile FileSM("mock_file_name.cpp", NullTerminatedCode);
+ auto = FileSM.get();
+
+ auto Tokens = syntax::tokenize(SM.getMainFileID(), SM, LangOpts);
+ unsigned Last = Tokens.size() - 1;
+
+ // One parser state for top level and each `[]` pair, can be nested.
+ // Technically we should have a state or recursion for each ()/{} as well,
+ // but since we're expecting well formed code it shouldn't matter in
practice.
+ struct ParserState {
+unsigned ParenCount = 0;
+unsigned BraceCount = 0;
+std::vector Pieces;
+ };
+
+ // We have to track square brackets, parens and braces as we want to skip the
+ // tokens inside them. This ensures that we don't use identical selector
+ // pieces in inner message sends, blocks, lambdas and @selector expressions.
+ std::vector States = {ParserState()};
+ unsigned NumPieces = Selector->getNumArgs();
+
+ for (unsigned Index = 0; Index < Last; ++Index) {
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)
@@ -1029,5 +1172,151 @@ size_t renameRangeAdjustmentCost(ArrayRef
Indexed, ArrayRef Lexed,
return Cost;
}
+static bool isMatchingSelectorName(const syntax::Token ,
+ const syntax::Token ,
+ const SourceManager ,
+ 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.
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+static bool isSelectorLike(const syntax::Token ,
+ const syntax::Token ) {
+ 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();
+}
+
+static void
+lex(llvm::StringRef Code, const LangOptions ,
+llvm::function_ref
+Action) {
+ // FIXME: InMemoryFileAdapter crashes unless the buffer is null terminated!
+ std::string NullTerminatedCode = Code.str();
+ SourceManagerForFile FileSM("mock_file_name.cpp", NullTerminatedCode);
+ auto = FileSM.get();
+ for (const auto : syntax::tokenize(SM.getMainFileID(), SM, LangOpts))
+Action(Tok, SM);
+}
+
+std::vector collectRenameIdentifierRanges(
+llvm::StringRef Identifier, llvm::StringRef Content,
+const LangOptions , std::optional Selector) {
+ std::vector Ranges;
+ if (!Selector) {
+lex(Content, LangOpts,
+[&](const syntax::Token , const SourceManager ) {
+ if (Tok.kind() != tok::identifier || Tok.text(SM) != Identifier)
+return;
+ Ranges.emplace_back(
+ halfOpenToRange(SM, Tok.range(SM).toCharRange(SM)));
+});
+return Ranges;
+ }
+ // FIXME: InMemoryFileAdapter crashes unless the buffer is null terminated!
+ std::string NullTerminatedCode = Content.str();
+ SourceManagerForFile FileSM("mock_file_name.cpp", NullTerminatedCode);
+ auto = FileSM.get();
+
+ auto Tokens = syntax::tokenize(SM.getMainFileID(), SM, LangOpts);
+ unsigned Last = Tokens.size() - 1;
+
+ // One parser state for top level and each `[]` pair, can be nested.
+ // Technically we should have a state or recursion for each ()/{} as well,
+ // but since we're expecting well formed code it shouldn't matter in
practice.
DavidGoldman wrote:
Removed
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)
@@ -1029,5 +1172,151 @@ size_t renameRangeAdjustmentCost(ArrayRef
Indexed, ArrayRef Lexed,
return Cost;
}
+static bool isMatchingSelectorName(const syntax::Token ,
+ const syntax::Token ,
+ const SourceManager ,
+ 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.
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+static bool isSelectorLike(const syntax::Token ,
+ const syntax::Token ) {
+ 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();
+}
+
+static void
+lex(llvm::StringRef Code, const LangOptions ,
+llvm::function_ref
+Action) {
+ // FIXME: InMemoryFileAdapter crashes unless the buffer is null terminated!
+ std::string NullTerminatedCode = Code.str();
+ SourceManagerForFile FileSM("mock_file_name.cpp", NullTerminatedCode);
+ auto = FileSM.get();
+ for (const auto : syntax::tokenize(SM.getMainFileID(), SM, LangOpts))
+Action(Tok, SM);
+}
+
+std::vector collectRenameIdentifierRanges(
+llvm::StringRef Identifier, llvm::StringRef Content,
+const LangOptions , std::optional Selector) {
+ std::vector Ranges;
+ if (!Selector) {
+lex(Content, LangOpts,
+[&](const syntax::Token , const SourceManager ) {
+ if (Tok.kind() != tok::identifier || Tok.text(SM) != Identifier)
+return;
+ Ranges.emplace_back(
+ halfOpenToRange(SM, Tok.range(SM).toCharRange(SM)));
+});
+return Ranges;
+ }
+ // FIXME: InMemoryFileAdapter crashes unless the buffer is null terminated!
+ std::string NullTerminatedCode = Content.str();
+ SourceManagerForFile FileSM("mock_file_name.cpp", NullTerminatedCode);
+ auto = FileSM.get();
+
+ auto Tokens = syntax::tokenize(SM.getMainFileID(), SM, LangOpts);
+ unsigned Last = Tokens.size() - 1;
+
+ // One parser state for top level and each `[]` pair, can be nested.
+ // Technically we should have a state or recursion for each ()/{} as well,
+ // but since we're expecting well formed code it shouldn't matter in
practice.
+ struct ParserState {
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)
@@ -1029,5 +1172,151 @@ size_t renameRangeAdjustmentCost(ArrayRef
Indexed, ArrayRef Lexed,
return Cost;
}
+static bool isMatchingSelectorName(const syntax::Token ,
+ const syntax::Token ,
+ const SourceManager ,
+ 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.
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+static bool isSelectorLike(const syntax::Token ,
+ const syntax::Token ) {
+ 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();
+}
+
+static void
+lex(llvm::StringRef Code, const LangOptions ,
+llvm::function_ref
+Action) {
+ // FIXME: InMemoryFileAdapter crashes unless the buffer is null terminated!
+ std::string NullTerminatedCode = Code.str();
+ SourceManagerForFile FileSM("mock_file_name.cpp", NullTerminatedCode);
+ auto = FileSM.get();
+ for (const auto : syntax::tokenize(SM.getMainFileID(), SM, LangOpts))
+Action(Tok, SM);
+}
+
+std::vector collectRenameIdentifierRanges(
+llvm::StringRef Identifier, llvm::StringRef Content,
+const LangOptions , std::optional Selector) {
+ std::vector Ranges;
+ if (!Selector) {
+lex(Content, LangOpts,
+[&](const syntax::Token , const SourceManager ) {
+ if (Tok.kind() != tok::identifier || Tok.text(SM) != Identifier)
+return;
+ Ranges.emplace_back(
+ halfOpenToRange(SM, Tok.range(SM).toCharRange(SM)));
+});
+return Ranges;
+ }
+ // FIXME: InMemoryFileAdapter crashes unless the buffer is null terminated!
+ std::string NullTerminatedCode = Content.str();
+ SourceManagerForFile FileSM("mock_file_name.cpp", NullTerminatedCode);
+ auto = FileSM.get();
+
+ auto Tokens = syntax::tokenize(SM.getMainFileID(), SM, LangOpts);
+ unsigned Last = Tokens.size() - 1;
+
+ // One parser state for top level and each `[]` pair, can be nested.
+ // Technically we should have a state or recursion for each ()/{} as well,
+ // but since we're expecting well formed code it shouldn't matter in
practice.
+ struct ParserState {
+unsigned ParenCount = 0;
+unsigned BraceCount = 0;
+std::vector Pieces;
+ };
+
+ // We have to track square brackets, parens and braces as we want to skip the
+ // tokens inside them. This ensures that we don't use identical selector
+ // pieces in inner message sends, blocks, lambdas and @selector expressions.
+ std::vector States = {ParserState()};
+ unsigned NumPieces = Selector->getNumArgs();
+
+ for (unsigned Index = 0; Index < Last; ++Index) {
+auto = States.back();
+auto = State.Pieces;
+const auto = Tokens[Index];
+const auto Kind = Tok.kind();
+auto PieceCount = Pieces.size();
+
+if (State.ParenCount == 0) {
+ // Check for matches until we find all selector pieces.
+ if (PieceCount < NumPieces &&
+ isMatchingSelectorName(Tok, Tokens[Index + 1], SM,
+ Selector->getNameForSlot(PieceCount))) {
+if (!Selector->getNameForSlot(PieceCount).empty()) {
+ // Skip the ':' after the name. This ensures that it won't match a
+ // follow-up selector piece with an empty name.
+ ++Index;
+}
+Pieces.push_back(halfOpenToRange(SM, Tok.range(SM).toCharRange(SM)));
+continue;
+ }
+ // If we've found all pieces, we still need to try to consume more pieces
+ // as it's possible the selector being renamed is a prefix of this method
+ // name.
DavidGoldman wrote:
Updated, parseMessageExpression can return early but the top level parse can't.
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)
@@ -1029,5 +1172,151 @@ size_t renameRangeAdjustmentCost(ArrayRef
Indexed, ArrayRef Lexed,
return Cost;
}
+static bool isMatchingSelectorName(const syntax::Token ,
+ const syntax::Token ,
+ const SourceManager ,
+ 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.
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+static bool isSelectorLike(const syntax::Token ,
+ const syntax::Token ) {
+ 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();
+}
+
+static void
+lex(llvm::StringRef Code, const LangOptions ,
+llvm::function_ref
+Action) {
+ // FIXME: InMemoryFileAdapter crashes unless the buffer is null terminated!
+ std::string NullTerminatedCode = Code.str();
+ SourceManagerForFile FileSM("mock_file_name.cpp", NullTerminatedCode);
+ auto = FileSM.get();
+ for (const auto : syntax::tokenize(SM.getMainFileID(), SM, LangOpts))
+Action(Tok, SM);
+}
+
+std::vector collectRenameIdentifierRanges(
+llvm::StringRef Identifier, llvm::StringRef Content,
+const LangOptions , std::optional Selector) {
+ std::vector Ranges;
+ if (!Selector) {
+lex(Content, LangOpts,
+[&](const syntax::Token , const SourceManager ) {
+ if (Tok.kind() != tok::identifier || Tok.text(SM) != Identifier)
+return;
+ Ranges.emplace_back(
+ halfOpenToRange(SM, Tok.range(SM).toCharRange(SM)));
+});
+return Ranges;
+ }
+ // FIXME: InMemoryFileAdapter crashes unless the buffer is null terminated!
+ std::string NullTerminatedCode = Content.str();
+ SourceManagerForFile FileSM("mock_file_name.cpp", NullTerminatedCode);
+ auto = FileSM.get();
+
+ auto Tokens = syntax::tokenize(SM.getMainFileID(), SM, LangOpts);
+ unsigned Last = Tokens.size() - 1;
+
+ // One parser state for top level and each `[]` pair, can be nested.
+ // Technically we should have a state or recursion for each ()/{} as well,
+ // but since we're expecting well formed code it shouldn't matter in
practice.
+ struct ParserState {
+unsigned ParenCount = 0;
+unsigned BraceCount = 0;
+std::vector Pieces;
+ };
+
+ // We have to track square brackets, parens and braces as we want to skip the
+ // tokens inside them. This ensures that we don't use identical selector
+ // pieces in inner message sends, blocks, lambdas and @selector expressions.
+ std::vector States = {ParserState()};
+ unsigned NumPieces = Selector->getNumArgs();
+
+ for (unsigned Index = 0; Index < Last; ++Index) {
+auto = States.back();
+auto = State.Pieces;
+const auto = Tokens[Index];
+const auto Kind = Tok.kind();
+auto PieceCount = Pieces.size();
+
+if (State.ParenCount == 0) {
+ // Check for matches until we find all selector pieces.
+ if (PieceCount < NumPieces &&
+ isMatchingSelectorName(Tok, Tokens[Index + 1], SM,
+ Selector->getNameForSlot(PieceCount))) {
+if (!Selector->getNameForSlot(PieceCount).empty()) {
DavidGoldman wrote:
Invalid or empty selector piece (foo::)
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)
@@ -1029,5 +1172,151 @@ size_t renameRangeAdjustmentCost(ArrayRef
Indexed, ArrayRef Lexed,
return Cost;
}
+static bool isMatchingSelectorName(const syntax::Token ,
+ const syntax::Token ,
+ const SourceManager ,
+ 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.
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+static bool isSelectorLike(const syntax::Token ,
+ const syntax::Token ) {
+ 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();
+}
+
+static void
+lex(llvm::StringRef Code, const LangOptions ,
+llvm::function_ref
+Action) {
+ // FIXME: InMemoryFileAdapter crashes unless the buffer is null terminated!
+ std::string NullTerminatedCode = Code.str();
+ SourceManagerForFile FileSM("mock_file_name.cpp", NullTerminatedCode);
+ auto = FileSM.get();
+ for (const auto : syntax::tokenize(SM.getMainFileID(), SM, LangOpts))
+Action(Tok, SM);
+}
+
+std::vector collectRenameIdentifierRanges(
+llvm::StringRef Identifier, llvm::StringRef Content,
+const LangOptions , std::optional Selector) {
+ std::vector Ranges;
+ if (!Selector) {
+lex(Content, LangOpts,
+[&](const syntax::Token , const SourceManager ) {
+ if (Tok.kind() != tok::identifier || Tok.text(SM) != Identifier)
+return;
+ Ranges.emplace_back(
+ halfOpenToRange(SM, Tok.range(SM).toCharRange(SM)));
+});
+return Ranges;
+ }
+ // FIXME: InMemoryFileAdapter crashes unless the buffer is null terminated!
+ std::string NullTerminatedCode = Content.str();
+ SourceManagerForFile FileSM("mock_file_name.cpp", NullTerminatedCode);
+ auto = FileSM.get();
+
+ auto Tokens = syntax::tokenize(SM.getMainFileID(), SM, LangOpts);
+ unsigned Last = Tokens.size() - 1;
+
+ // One parser state for top level and each `[]` pair, can be nested.
+ // Technically we should have a state or recursion for each ()/{} as well,
+ // but since we're expecting well formed code it shouldn't matter in
practice.
+ struct ParserState {
+unsigned ParenCount = 0;
+unsigned BraceCount = 0;
+std::vector Pieces;
+ };
+
+ // We have to track square brackets, parens and braces as we want to skip the
+ // tokens inside them. This ensures that we don't use identical selector
+ // pieces in inner message sends, blocks, lambdas and @selector expressions.
+ std::vector States = {ParserState()};
+ unsigned NumPieces = Selector->getNumArgs();
+
+ for (unsigned Index = 0; Index < Last; ++Index) {
+auto = States.back();
+auto = State.Pieces;
+const auto = Tokens[Index];
+const auto Kind = Tok.kind();
+auto PieceCount = Pieces.size();
+
+if (State.ParenCount == 0) {
+ // Check for matches until we find all selector pieces.
+ if (PieceCount < NumPieces &&
+ isMatchingSelectorName(Tok, Tokens[Index + 1], SM,
DavidGoldman wrote:
It's not, Last = Len -1 and we go until < Last, so Len - 2.
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)
@@ -1029,5 +1172,151 @@ size_t renameRangeAdjustmentCost(ArrayRef
Indexed, ArrayRef Lexed,
return Cost;
}
+static bool isMatchingSelectorName(const syntax::Token ,
+ const syntax::Token ,
+ const SourceManager ,
+ 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.
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+static bool isSelectorLike(const syntax::Token ,
+ const syntax::Token ) {
+ 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();
+}
+
+static void
+lex(llvm::StringRef Code, const LangOptions ,
+llvm::function_ref
+Action) {
+ // FIXME: InMemoryFileAdapter crashes unless the buffer is null terminated!
+ std::string NullTerminatedCode = Code.str();
+ SourceManagerForFile FileSM("mock_file_name.cpp", NullTerminatedCode);
+ auto = FileSM.get();
+ for (const auto : syntax::tokenize(SM.getMainFileID(), SM, LangOpts))
+Action(Tok, SM);
+}
+
+std::vector collectRenameIdentifierRanges(
+llvm::StringRef Identifier, llvm::StringRef Content,
+const LangOptions , std::optional Selector) {
+ std::vector Ranges;
+ if (!Selector) {
+lex(Content, LangOpts,
+[&](const syntax::Token , const SourceManager ) {
+ if (Tok.kind() != tok::identifier || Tok.text(SM) != Identifier)
+return;
+ Ranges.emplace_back(
+ halfOpenToRange(SM, Tok.range(SM).toCharRange(SM)));
+});
+return Ranges;
+ }
+ // FIXME: InMemoryFileAdapter crashes unless the buffer is null terminated!
+ std::string NullTerminatedCode = Content.str();
+ SourceManagerForFile FileSM("mock_file_name.cpp", NullTerminatedCode);
+ auto = FileSM.get();
+
+ auto Tokens = syntax::tokenize(SM.getMainFileID(), SM, LangOpts);
+ unsigned Last = Tokens.size() - 1;
+
+ // One parser state for top level and each `[]` pair, can be nested.
+ // Technically we should have a state or recursion for each ()/{} as well,
+ // but since we're expecting well formed code it shouldn't matter in
practice.
+ struct ParserState {
+unsigned ParenCount = 0;
+unsigned BraceCount = 0;
+std::vector Pieces;
DavidGoldman wrote:
Updated with a comment, selector pieces/ranges as well as ( and { count
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)
@@ -1029,5 +1172,151 @@ size_t renameRangeAdjustmentCost(ArrayRef Indexed, ArrayRef Lexed, return Cost; } +static bool isMatchingSelectorName(const syntax::Token , 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)
@@ -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;
DavidGoldman wrote:
This was formatted because it was changed, .start -> .range().start
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)
@@ -1007,7 +1148,8 @@ std::optional> getMappedRanges(ArrayRef Indexed, // diff[0]: line + 1 <- insert a line before edit 0. // diff[1]: column + 1 <- remove a line between edits 0 and 1, and insert a // character on edit 1. -size_t renameRangeAdjustmentCost(ArrayRef Indexed, ArrayRef Lexed, +size_t renameRangeAdjustmentCost(ArrayRef Indexed, + ArrayRef Lexed, DavidGoldman wrote: These are formatted now because Range -> SymbolRange 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)
@@ -1029,5 +1172,151 @@ size_t renameRangeAdjustmentCost(ArrayRef
Indexed, ArrayRef Lexed,
return Cost;
}
+static bool isMatchingSelectorName(const syntax::Token ,
+ const syntax::Token ,
+ const SourceManager ,
+ 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.
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+static bool isSelectorLike(const syntax::Token ,
+ const syntax::Token ) {
+ 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();
+}
+
+static void
+lex(llvm::StringRef Code, const LangOptions ,
+llvm::function_ref
+Action) {
+ // FIXME: InMemoryFileAdapter crashes unless the buffer is null terminated!
+ std::string NullTerminatedCode = Code.str();
+ SourceManagerForFile FileSM("mock_file_name.cpp", NullTerminatedCode);
+ auto = FileSM.get();
+ for (const auto : syntax::tokenize(SM.getMainFileID(), SM, LangOpts))
+Action(Tok, SM);
+}
+
+std::vector collectRenameIdentifierRanges(
+llvm::StringRef Identifier, llvm::StringRef Content,
+const LangOptions , std::optional Selector) {
+ std::vector Ranges;
+ if (!Selector) {
+lex(Content, LangOpts,
+[&](const syntax::Token , const SourceManager ) {
+ if (Tok.kind() != tok::identifier || Tok.text(SM) != Identifier)
+return;
+ Ranges.emplace_back(
+ halfOpenToRange(SM, Tok.range(SM).toCharRange(SM)));
+});
+return Ranges;
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)
@@ -1029,5 +1172,151 @@ size_t renameRangeAdjustmentCost(ArrayRef
Indexed, ArrayRef Lexed,
return Cost;
}
+static bool isMatchingSelectorName(const syntax::Token ,
+ const syntax::Token ,
+ const SourceManager ,
+ 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.
+ // e.g. support `foo:` but not `foo :`.
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)
@@ -778,12 +868,44 @@ llvm::Expected rename(const RenameInputs
) {
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 = **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()) {
+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();
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)
@@ -778,12 +868,44 @@ llvm::Expected rename(const RenameInputs
) {
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 = **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()) {
+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];
+ break;
+}
+ }
+}
+ } else {
+const auto *ID = RenameDecl.getIdentifier();
+if (!ID)
+ return makeError(ReasonToReject::UnsupportedSymbol);
+if (ID->getName() == RInputs.NewName)
+ return makeError(ReasonToReject::SameName);
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)
@@ -778,12 +868,44 @@ llvm::Expected rename(const RenameInputs
) {
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 = **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()) {
+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];
+ break;
+}
+ }
+}
+ } else {
+const auto *ID = RenameDecl.getIdentifier();
+if (!ID)
+ return makeError(ReasonToReject::UnsupportedSymbol);
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)
@@ -778,12 +868,44 @@ llvm::Expected rename(const RenameInputs
) {
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 = **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()) {
+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()});
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)
@@ -778,12 +868,44 @@ llvm::Expected rename(const RenameInputs
) {
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 = **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()) {
+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()});
DavidGoldman wrote:
Never mind, didn't see your comments below
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
) {
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 = **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()) {
+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()});
DavidGoldman wrote:
Are you sure? I kept this separate since the else statement below performs
similar logic outside of checkName. Should I move both into checkName?
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 , const SymbolRange ) {
+ return LHS.Ranges == RHS.Ranges;
+}
+bool operator!=(const SymbolRange , const SymbolRange ) {
+ return !(LHS == RHS);
+}
+bool operator<(const SymbolRange , const SymbolRange ) {
+ return LHS.range() < RHS.range();
+}
+
+std::vector symbolRanges(const std::vector Ranges) {
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)
@@ -543,6 +550,45 @@ std::optional checkName(const NamedDecl
,
return Result;
}
+clangd::Range tokenRangeForLoc(ParsedAST , SourceLocation TokLoc,
+ const SourceManager ,
+ const LangOptions ) {
+ 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 , const ObjCMethodDecl *MD,
+ llvm::StringRef NewName,
+ std::vector Locs) {
+ const SourceManager = 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 = MD->getASTContext().getLangOpts();
+ for (const auto : Locs)
+Ranges.push_back(tokenRangeForLoc(AST, Loc, SM, LangOpts));
+ auto FilePath = AST.tuPath();
+ auto RenameRanges = collectRenameIdentifierRanges(
DavidGoldman wrote:
Hmm, what do you think? we could call adjustRenameRanges to make sure what
we've lexed matches. It's a bit more complex to guide lexing since we need to
differentiate between method decls/exprs but maybe that could work. Just not
sure it's worth adding much on top of this logic - we originally had a purely
AST-visitor based approach but I'd prefer not to add more complexity 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)
@@ -543,6 +550,45 @@ std::optional checkName(const NamedDecl
,
return Result;
}
+clangd::Range tokenRangeForLoc(ParsedAST , SourceLocation TokLoc,
+ const SourceManager ,
+ const LangOptions ) {
+ 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.
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)
@@ -543,6 +550,45 @@ std::optional checkName(const NamedDecl
,
return Result;
}
+clangd::Range tokenRangeForLoc(ParsedAST , SourceLocation TokLoc,
+ const SourceManager ,
+ const LangOptions ) {
+ 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 , const ObjCMethodDecl *MD,
+ llvm::StringRef NewName,
+ std::vector Locs) {
+ const SourceManager = AST.getSourceManager();
+ auto Code = SM.getBufferData(SM.getMainFileID());
+ auto RenameIdentifier = MD->getSelector().getNameForSlot(0).str();
+ llvm::SmallVector NewNames;
+ NewName.split(NewNames, ":");
+ if (NewNames.empty())
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)
@@ -543,6 +550,45 @@ std::optional checkName(const NamedDecl
,
return Result;
}
+clangd::Range tokenRangeForLoc(ParsedAST , SourceLocation TokLoc,
+ const SourceManager ,
+ const LangOptions ) {
+ const auto *Token = AST.getTokens().spelledTokenAt(TokLoc);
+ assert(Token && "got inclusion at wrong offset");
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 ,
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()) {
+ if (MD->getSelector().getNumArgs() > 1) {
+RenameIdentifier = MD->getSelector().getNameForSlot(0).str();
+Selector = MD->getSelector();
+NewName.split(NewNames, ":");
DavidGoldman wrote:
Yep, you can have empty selector fragments.
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)
@@ -1029,5 +1172,151 @@ size_t renameRangeAdjustmentCost(ArrayRef
Indexed, ArrayRef Lexed,
return Cost;
}
+static bool isMatchingSelectorName(const syntax::Token ,
+ const syntax::Token ,
+ const SourceManager ,
+ 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.
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+static bool isSelectorLike(const syntax::Token ,
+ const syntax::Token ) {
+ 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();
+}
+
+static void
+lex(llvm::StringRef Code, const LangOptions ,
+llvm::function_ref
+Action) {
+ // FIXME: InMemoryFileAdapter crashes unless the buffer is null terminated!
+ std::string NullTerminatedCode = Code.str();
+ SourceManagerForFile FileSM("mock_file_name.cpp", NullTerminatedCode);
+ auto = FileSM.get();
+ for (const auto : syntax::tokenize(SM.getMainFileID(), SM, LangOpts))
+Action(Tok, SM);
+}
+
+std::vector collectRenameIdentifierRanges(
+llvm::StringRef Identifier, llvm::StringRef Content,
+const LangOptions , std::optional Selector) {
+ std::vector Ranges;
+ if (!Selector) {
+lex(Content, LangOpts,
+[&](const syntax::Token , const SourceManager ) {
+ if (Tok.kind() != tok::identifier || Tok.text(SM) != Identifier)
+return;
+ Ranges.emplace_back(
+ halfOpenToRange(SM, Tok.range(SM).toCharRange(SM)));
+});
+return Ranges;
+ }
+ // FIXME: InMemoryFileAdapter crashes unless the buffer is null terminated!
+ std::string NullTerminatedCode = Content.str();
+ SourceManagerForFile FileSM("mock_file_name.cpp", NullTerminatedCode);
+ auto = FileSM.get();
+
+ auto Tokens = syntax::tokenize(SM.getMainFileID(), SM, LangOpts);
+ unsigned Last = Tokens.size() - 1;
+
+ // One parser state for top level and each `[]` pair, can be nested.
+ // Technically we should have a state or recursion for each ()/{} as well,
+ // but since we're expecting well formed code it shouldn't matter in
practice.
+ struct ParserState {
+unsigned ParenCount = 0;
+unsigned BraceCount = 0;
+std::vector Pieces;
+ };
+
+ // We have to track square brackets, parens and braces as we want to skip the
+ // tokens inside them. This ensures that we don't use identical selector
+ // pieces in inner message sends, blocks, lambdas and @selector expressions.
+ std::vector States = {ParserState()};
+ unsigned NumPieces = Selector->getNumArgs();
+
+ for (unsigned Index = 0; Index < Last; ++Index) {
+auto = States.back();
+auto = State.Pieces;
+const auto = Tokens[Index];
+const auto Kind = Tok.kind();
+auto PieceCount = Pieces.size();
+
+if (State.ParenCount == 0) {
+ // Check for matches until we find all selector pieces.
+ if (PieceCount < NumPieces &&
+ isMatchingSelectorName(Tok, Tokens[Index + 1], SM,
+ Selector->getNameForSlot(PieceCount))) {
+if (!Selector->getNameForSlot(PieceCount).empty()) {
+ // Skip the ':' after the name. This ensures that it won't match a
+ // follow-up selector piece with an empty name.
+ ++Index;
+}
+Pieces.push_back(halfOpenToRange(SM, Tok.range(SM).toCharRange(SM)));
+continue;
+ }
+ // If we've found all pieces, we still need to try to consume more pieces
+ // as it's possible the selector being renamed is a prefix of this method
+ // name.
kadircet wrote:
shouldn't we just fail in that case?
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
,
return Result;
}
+clangd::Range tokenRangeForLoc(ParsedAST , SourceLocation TokLoc,
+ const SourceManager ,
+ const LangOptions ) {
+ 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.
kadircet wrote:
can we also define what `Locs` are, with a better name (like
`SelectorOccurences`)?
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)
@@ -1029,5 +1172,151 @@ size_t renameRangeAdjustmentCost(ArrayRef Indexed, ArrayRef Lexed, return Cost; } +static bool isMatchingSelectorName(const syntax::Token , kadircet wrote: can you move all of these into anon namespace instead? 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)
@@ -38,6 +38,11 @@
namespace clang {
namespace clangd {
+
+std::vector collectRenameIdentifierRanges(
kadircet wrote:
let's move this into anonymous namespace, and add some comments explaining what
it 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)
@@ -1029,5 +1172,151 @@ size_t renameRangeAdjustmentCost(ArrayRef
Indexed, ArrayRef Lexed,
return Cost;
}
+static bool isMatchingSelectorName(const syntax::Token ,
+ const syntax::Token ,
+ const SourceManager ,
+ 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.
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+static bool isSelectorLike(const syntax::Token ,
+ const syntax::Token ) {
+ 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();
+}
+
+static void
+lex(llvm::StringRef Code, const LangOptions ,
+llvm::function_ref
+Action) {
+ // FIXME: InMemoryFileAdapter crashes unless the buffer is null terminated!
+ std::string NullTerminatedCode = Code.str();
+ SourceManagerForFile FileSM("mock_file_name.cpp", NullTerminatedCode);
+ auto = FileSM.get();
+ for (const auto : syntax::tokenize(SM.getMainFileID(), SM, LangOpts))
+Action(Tok, SM);
+}
+
+std::vector collectRenameIdentifierRanges(
+llvm::StringRef Identifier, llvm::StringRef Content,
+const LangOptions , std::optional Selector) {
+ std::vector Ranges;
+ if (!Selector) {
+lex(Content, LangOpts,
+[&](const syntax::Token , const SourceManager ) {
+ if (Tok.kind() != tok::identifier || Tok.text(SM) != Identifier)
+return;
+ Ranges.emplace_back(
+ halfOpenToRange(SM, Tok.range(SM).toCharRange(SM)));
+});
+return Ranges;
+ }
+ // FIXME: InMemoryFileAdapter crashes unless the buffer is null terminated!
+ std::string NullTerminatedCode = Content.str();
+ SourceManagerForFile FileSM("mock_file_name.cpp", NullTerminatedCode);
+ auto = FileSM.get();
+
+ auto Tokens = syntax::tokenize(SM.getMainFileID(), SM, LangOpts);
+ unsigned Last = Tokens.size() - 1;
+
+ // One parser state for top level and each `[]` pair, can be nested.
+ // Technically we should have a state or recursion for each ()/{} as well,
+ // but since we're expecting well formed code it shouldn't matter in
practice.
+ struct ParserState {
+unsigned ParenCount = 0;
+unsigned BraceCount = 0;
+std::vector Pieces;
+ };
+
+ // We have to track square brackets, parens and braces as we want to skip the
+ // tokens inside them. This ensures that we don't use identical selector
+ // pieces in inner message sends, blocks, lambdas and @selector expressions.
+ std::vector States = {ParserState()};
+ unsigned NumPieces = Selector->getNumArgs();
+
+ for (unsigned Index = 0; Index < Last; ++Index) {
+auto = States.back();
+auto = State.Pieces;
+const auto = Tokens[Index];
+const auto Kind = Tok.kind();
+auto PieceCount = Pieces.size();
+
+if (State.ParenCount == 0) {
+ // Check for matches until we find all selector pieces.
+ if (PieceCount < NumPieces &&
+ isMatchingSelectorName(Tok, Tokens[Index + 1], SM,
+ Selector->getNameForSlot(PieceCount))) {
+if (!Selector->getNameForSlot(PieceCount).empty()) {
kadircet wrote:
what does it mean for this selector slot to be 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
