================
@@ -548,59 +601,193 @@ class DefineOutline : public Tweak {
return std::move(*Effect);
}
- // Returns the most natural insertion point for \p QualifiedName in \p
- // Contents. This currently cares about only the namespace proximity, but in
- // feature it should also try to follow ordering of declarations. For
example,
- // if decls come in order `foo, bar, baz` then this function should return
- // some point between foo and baz for inserting bar.
- // FIXME: The selection can be made smarter by looking at the definition
- // locations for adjacent decls to Source. Unfortunately pseudo parsing in
- // getEligibleRegions only knows about namespace begin/end events so we
- // can't match function start/end positions yet.
- llvm::Expected<InsertionPoint> getInsertionPoint(llvm::StringRef Contents,
- const Selection &Sel) {
- // If the definition goes to the same file and there is a namespace,
- // we should (and, in the case of anonymous namespaces, need to)
- // put the definition into the original namespace block.
- if (SameFile) {
- auto *Klass = Source->getDeclContext()->getOuterLexicalRecordContext();
- if (!Klass)
- return error("moving to same file not supported for free functions");
- const SourceLocation EndLoc = Klass->getBraceRange().getEnd();
- const auto &TokBuf = Sel.AST->getTokens();
- auto Tokens = TokBuf.expandedTokens();
- auto It = llvm::lower_bound(
- Tokens, EndLoc, [](const syntax::Token &Tok, SourceLocation EndLoc) {
- return Tok.location() < EndLoc;
- });
- while (It != Tokens.end()) {
- if (It->kind() != tok::semi) {
- ++It;
- continue;
+ enum class RelativeInsertPos { Before, After };
+ std::optional<std::tuple<SymbolLocation, Path, RelativeInsertPos>>
+ getDefinitionOfAdjacentDecl(const Selection &Sel) {
+ if (!Sel.Index)
+ return {};
+ std::optional<std::pair<SymbolLocation, Path>> Anchor;
+ std::string TuURI = URI::createFile(Sel.AST->tuPath()).toString();
+ auto CheckCandidate = [&](Decl *Candidate) {
+ assert(Candidate != Source);
+ if (auto Func = llvm::dyn_cast_or_null<FunctionDecl>(Candidate);
+ !Func || Func->isThisDeclarationADefinition()) {
+ return;
+ }
+ std::optional<std::pair<SymbolLocation, Path>> CandidateLoc;
+ Sel.Index->lookup({{getSymbolID(Candidate)}}, [&](const Symbol &S) {
+ if (S.Definition) {
+ CandidateLoc = std::make_pair(S.Definition,
+ StringRef(S.Definition.FileURI).str());
----------------
HighCommander4 wrote:
This is a bit subtle from a lifetime point of view: you are right to make a
copy of `FileURI` here (we can't rely on the storage it points to persisting
beyond the callback), but we're still exposing that dangling `FileURI` in the
returned `SymbolLocation` (which you're later careful to fix up with
`std::get<0>(*Anchor).FileURI = std::get<1>(*Anchor).c_str();`.)
I think it would be a bit more robust to convert to `clangd::Location` right
here in the callback. Then the local variable `Anchor` can just be a
`std::optional<Location>`, and the struct the function returns can just have
two fields (`Location` and `RelativeInsertPos`).
https://github.com/llvm/llvm-project/pull/128164
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
