[clang] [clang-tools-extra] [clangd] Use `SymbolName` to represent Objective-C selectors (PR #82061)

Sam McCall via cfe-commits cfe-commits at lists.llvm.org
Fri Feb 23 14:50:56 PST 2024


sam-mccall wrote:

> For some context: When I’m talking about * finding the ranges to rename based on an index that’s not clangd’s built-in index* I meant a request like [apple#7973](https://github.com/apple/llvm-project/pull/7973).

I see. That makes sense, it's just a bit non-obvious because we don't usually design these pieces as libraries to be consumed outside clangd (either by code calling into clangd's internals, or a modified version of clangd).

I think we usually won't support major design changes to accommodate these, but small ones like this look totally fine.
(There are exceptions, e.g. adding XPC support required significant changes. After these, XPC lives in llvm-project but could just as easily be downstream).

> Functionality-wise, I would be fine with using a `optional<vector<string>>` instead of `Selector` in `collectRenameIdentifierRanges`. FWIW I disagree that using a `vector<string>` is cleaner than using a dedicated type for it because there’s no type-level information about what the `vector<string>` represents

Yeah, "clean" can mean various things, e.g. "clearly communicating intent" here is in tension with "fewer moving parts".
I'd be fine with `struct SelectorName { vector<StringRef> Chunks; }` or `using SelectorName = vector<string>`, more than that is (for my taste) more noise than signal here. I think it should go in `clangd/refactor/Rename.h` though - it's just a part of that API.



https://github.com/llvm/llvm-project/pull/82061


More information about the cfe-commits mailing list