================
@@ -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());
}
- unsigned Offset = Sel.AST->getSourceManager()
- .getDecomposedLoc(It->endLocation())
- .second;
- return InsertionPoint{Klass->getEnclosingNamespaceContext(), Offset};
+ });
+ if (!CandidateLoc)
+ return;
+
+ // If our definition is constrained to the same file, ignore
+ // definitions that are not located there.
+ // If our definition is not constrained to the same file, but
+ // our anchor definition is in the same file, then we also put our
+ // definition there, because that appears to be the user preference.
+ // Exception: If the existing definition is a template, then the
+ // location is likely due to technical necessity rather than preference,
+ // so ignore that definition.
+ bool CandidateSameFile = TuURI == CandidateLoc->second;
+ if (SameFile && !CandidateSameFile)
+ return;
+ if (!SameFile && CandidateSameFile) {
+ if (Candidate->isTemplateDecl())
+ return;
+ SameFile = true;
}
- return error(
- "failed to determine insertion location: no end of class found");
+ Anchor = *CandidateLoc;
+ };
+
+ // Try to find adjacent function declarations.
+ // Determine the closest one by alternatingly going "up" and "down"
+ // from our function in increasing steps.
+ const DeclContext *ParentContext = Source->getParent();
+ const auto SourceIt = llvm::find_if(
+ ParentContext->decls(), [this](const Decl *D) { return D == Source; });
+ if (SourceIt == ParentContext->decls_end())
+ return {};
+ const int Preceding = std::distance(ParentContext->decls_begin(),
SourceIt);
+ const int Following =
+ std::distance(SourceIt, ParentContext->decls_end()) - 1;
+ for (int Offset = 1; Offset <= Preceding || Offset <= Following; ++Offset)
{
+ if (Offset <= Preceding)
+ CheckCandidate(
+ *std::next(ParentContext->decls_begin(), Preceding - Offset));
+ if (Anchor)
+ return std::make_tuple(Anchor->first, Anchor->second,
+ RelativeInsertPos::After);
+ if (Offset <= Following)
+ CheckCandidate(*std::next(SourceIt, Offset));
+ if (Anchor)
+ return std::make_tuple(Anchor->first, Anchor->second,
+ RelativeInsertPos::Before);
}
+ return {};
+ }
- auto Region = getEligiblePoints(
- Contents, Source->getQualifiedNameAsString(), Sel.AST->getLangOpts());
+ // We don't know the actual start or end of the definition, only the position
+ // of the name. Therefore, we heuristically try to locate the last token
+ // before or in this function, respectively. Adapt as required by user code.
+ llvm::Expected<Position> getInsertionPointFromExistingDefinition(
+ const llvm::MemoryBuffer &Buffer, const SymbolLocation &Loc,
+ RelativeInsertPos RelInsertPos, ParsedAST *AST) {
+ auto LspPos = indexToLSPLocation(Loc, AST->tuPath());
+ if (!LspPos)
+ return LspPos.takeError();
+ auto StartOffset =
+ positionToOffset(Buffer.getBuffer(), LspPos->range.start);
+ if (!StartOffset)
+ return LspPos.takeError();
+ SourceLocation InsertionLoc;
+ SourceManager &SM = AST->getSourceManager();
+ FileID F = Buffer.getBufferIdentifier() == AST->tuPath()
+ ? SM.getMainFileID()
+ : SM.createFileID(Buffer);
----------------
HighCommander4 wrote:
I'm not sure that it's safe to do this with `AST->getSourceManager()`. The
[documentation](https://searchfox.org/llvm/rev/bea31dd373e3f053f0b3f1862c6b106831e1f25d/clang/include/clang/Basic/SourceManager.h#935-936)
of this method says "The memory buffer must outlive the SourceManager", which
is not the case here.
Instead, there's a utility called
[`SourceManagerForFile`](https://searchfox.org/llvm/rev/bea31dd373e3f053f0b3f1862c6b106831e1f25d/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp#514)
which we use later in `apply()` to create a temporary `SourceManager` for
`CCFile`. We can probably create that earlier in `apply()`, and use its
`SourceManager` for this purpose. (Note that there will no need to call
`createFileID()`, as for this `SourceManager`, `getMainFileID()` will be
`CCFile`.)
https://github.com/llvm/llvm-project/pull/128164
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
