kadircet updated this revision to Diff 230439.
kadircet added a comment.
- Rebase
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70535/new/
https://reviews.llvm.org/D70535
Files:
clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
clang-tools-extra/clangd/unittests/TweakTests.cpp
Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -1775,6 +1775,56 @@
}
}
+TEST_F(DefineOutlineTest, QualifyReturnValue) {
+ FileName = "Test.hpp";
+ ExtraFiles["Test.cpp"] = "";
+
+ struct {
+ llvm::StringRef Test;
+ llvm::StringRef ExpectedHeader;
+ llvm::StringRef ExpectedSource;
+ } Cases[] = {
+ {R"cpp(
+ namespace a { class Foo; }
+ using namespace a;
+ Foo fo^o() { return; })cpp",
+ R"cpp(
+ namespace a { class Foo; }
+ using namespace a;
+ Foo foo() ;)cpp",
+ "a::Foo foo() { return; }"},
+ {R"cpp(
+ namespace a {
+ class Foo {
+ class Bar {};
+ Bar fo^o() { return {}; }
+ }
+ })cpp",
+ R"cpp(
+ namespace a {
+ class Foo {
+ class Bar {};
+ Bar foo() ;
+ }
+ })cpp",
+ "a::Foo::Bar foo() { return {}; }\n"},
+ {R"cpp(
+ class Foo;
+ Foo fo^o() { return; })cpp",
+ R"cpp(
+ class Foo;
+ Foo foo() ;)cpp",
+ "Foo foo() { return; }"},
+ };
+ llvm::StringMap<std::string> EditedFiles;
+ for (auto &Case : Cases) {
+ apply(Case.Test, &EditedFiles);
+ EXPECT_EQ(apply(Case.Test, &EditedFiles), Case.ExpectedHeader);
+ llvm::errs() << EditedFiles.begin()->second << '\n';
+ EXPECT_THAT(EditedFiles, testing::ElementsAre(FileWithContents(
+ testPath("Test.cpp"), Case.ExpectedSource)));
+ }
+}
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -6,13 +6,17 @@
//
//===----------------------------------------------------------------------===//
+#include "AST.h"
+#include "FindTarget.h"
#include "HeaderSourceSwitch.h"
+#include "Logger.h"
#include "Path.h"
#include "Selection.h"
#include "SourceCode.h"
#include "refactor/Tweak.h"
#include "clang/AST/ASTTypeTraits.h"
#include "clang/AST/Decl.h"
+#include "clang/AST/DeclBase.h"
#include "clang/AST/DeclTemplate.h"
#include "clang/AST/Stmt.h"
#include "clang/Basic/SourceLocation.h"
@@ -22,8 +26,10 @@
#include "clang/Tooling/Core/Replacement.h"
#include "llvm/ADT/Optional.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Casting.h"
#include "llvm/Support/Error.h"
#include <cstddef>
+#include <string>
namespace clang {
namespace clangd {
@@ -70,27 +76,89 @@
// Creates a modified version of function definition that can be inserted at a
// different location. Contains both function signature and body.
-llvm::Optional<llvm::StringRef> getFunctionSourceCode(const FunctionDecl *FD) {
+llvm::Expected<std::string>
+getFunctionSourceCode(const FunctionDecl *FD, llvm::StringRef TargetNamespace) {
auto &SM = FD->getASTContext().getSourceManager();
- auto CharRange = toHalfOpenFileRange(SM, FD->getASTContext().getLangOpts(),
- FD->getSourceRange());
- if (!CharRange)
- return llvm::None;
- CharRange->setBegin(getBeginLoc(FD));
+ const DeclContext *TargetContext = FD->getLexicalDeclContext();
+ while (!TargetContext->isTranslationUnit()) {
+ if (TargetContext->isNamespace()) {
+ auto *NSD = llvm::dyn_cast<NamespaceDecl>(TargetContext);
+ if (NSD->getNameAsString() == TargetNamespace)
+ break;
+ }
+ TargetContext = TargetContext->getLexicalParent();
+ }
+
+ bool HadErrors = false;
+ tooling::Replacements Replacements;
+ findExplicitReferences(FD, [&](ReferenceLoc Ref) {
+ // It is enough to qualify the first qualifier, so skip references with a
+ // qualifier. Also we can't do much if there are no targets or name is
+ // inside a macro body.
+ if (Ref.Qualifier || Ref.Targets.empty() || Ref.NameLoc.isMacroID())
+ return;
+ // Qualify return type
+ if (Ref.NameLoc != FD->getReturnTypeSourceRange().getBegin())
+ return;
+
+ for (const NamedDecl *ND : Ref.Targets) {
+ if (ND->getDeclContext() != Ref.Targets.front()->getDeclContext()) {
+ elog("Targets from multiple contexts: {0}, {1}",
+ printQualifiedName(*Ref.Targets.front()), printQualifiedName(*ND));
+ return;
+ }
+ }
+ const NamedDecl *ND = Ref.Targets.front();
+ const std::string Qualifier =
+ getQualification(FD->getASTContext(), TargetContext,
+ SM.getLocForStartOfFile(SM.getMainFileID()), ND);
+ if (auto Err = Replacements.add(
+ tooling::Replacement(SM, Ref.NameLoc, 0, Qualifier))) {
+ elog("define outline: Failed to add quals: {0}", std::move(Err));
+ HadErrors = true;
+ }
+ });
+
+ if (HadErrors) {
+ return llvm::createStringError(
+ llvm::inconvertibleErrorCode(),
+ "define outline: Failed to compute qualifiers see logs for details.");
+ }
+
+ // Get new begin and end positions for the qualified body.
+ auto OrigFuncRange = toHalfOpenFileRange(
+ SM, FD->getASTContext().getLangOpts(), FD->getSourceRange());
+ if (!OrigFuncRange)
+ return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "Couldn't get range for function.");
+ OrigFuncRange->setBegin(getBeginLoc(FD));
+
+ unsigned FuncBegin = SM.getFileOffset(OrigFuncRange->getBegin());
+ unsigned FuncEnd = Replacements.getShiftedCodePosition(
+ SM.getFileOffset(OrigFuncRange->getEnd()));
+
+ // Trim the result to function body.
+ auto QualifiedFunc = tooling::applyAllReplacements(
+ SM.getBufferData(SM.getMainFileID()), Replacements);
+ if (!QualifiedFunc)
+ return QualifiedFunc.takeError();
+ return QualifiedFunc->substr(FuncBegin, FuncEnd - FuncBegin + 1);
- // FIXME: Qualify return type.
// FIXME: Qualify function name depending on the target context.
- return toSourceCode(SM, *CharRange);
}
+struct InsertionPoint {
+ std::string EnclosingNamespace;
+ size_t Offset;
+};
// 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.
-llvm::Expected<size_t> getInsertionOffset(llvm::StringRef Contents,
- llvm::StringRef QualifiedName,
- const format::FormatStyle &Style) {
+llvm::Expected<InsertionPoint>
+getInsertionPoint(llvm::StringRef Contents, llvm::StringRef QualifiedName,
+ const format::FormatStyle &Style) {
auto Region = getEligiblePoints(Contents, QualifiedName, Style);
assert(!Region.EligiblePoints.empty());
@@ -98,8 +166,10 @@
// locations for adjacent decls to Source. Unfortunately psudeo parsing in
// getEligibleRegions only knows about namespace begin/end events so we
// can't match function start/end positions yet.
- auto InsertionPoint = Region.EligiblePoints.back();
- return positionToOffset(Contents, InsertionPoint);
+ auto Offset = positionToOffset(Contents, Region.EligiblePoints.back());
+ if (!Offset)
+ return Offset.takeError();
+ return InsertionPoint{Region.EnclosingNamespace, *Offset};
}
/// Moves definition of a function/method to an appropriate implementation file.
@@ -171,21 +241,22 @@
return llvm::createStringError(Buffer.getError(),
Buffer.getError().message());
auto Contents = Buffer->get()->getBuffer();
- auto InsertionOffset =
- getInsertionOffset(Contents, Source->getQualifiedNameAsString(),
- getFormatStyleForFile(*CCFile, Contents, &FS));
- if (!InsertionOffset)
- return InsertionOffset.takeError();
+ auto InsertionPoint =
+ getInsertionPoint(Contents, Source->getQualifiedNameAsString(),
+ getFormatStyleForFile(*CCFile, Contents, &FS));
+ if (!InsertionPoint)
+ return InsertionPoint.takeError();
- auto FuncDef = getFunctionSourceCode(Source);
+ auto FuncDef =
+ getFunctionSourceCode(Source, InsertionPoint->EnclosingNamespace);
if (!FuncDef)
return llvm::createStringError(
llvm::inconvertibleErrorCode(),
"Couldn't get full source for function definition.");
SourceManagerForFile SMFF(*CCFile, Contents);
- const tooling::Replacement InsertFunctionDef(*CCFile, *InsertionOffset, 0,
- *FuncDef);
+ const tooling::Replacement InsertFunctionDef(
+ *CCFile, InsertionPoint->Offset, 0, *FuncDef);
auto Effect = Effect::mainFileEdit(
SMFF.get(), tooling::Replacements(InsertFunctionDef));
if (!Effect)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
