llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clangd Author: None (ckandeler) <details> <summary>Changes</summary> --- Full diff: https://github.com/llvm/llvm-project/pull/71950.diff 2 Files Affected: - (modified) clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp (+50-11) - (modified) clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp (+26-6) ``````````diff diff --git a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp index b84ae04072f2c19..9e715b7bf21175a 100644 --- a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp +++ b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp @@ -128,7 +128,16 @@ getFunctionSourceAfterReplacements(const FunctionDecl *FD, SM.getBufferData(SM.getMainFileID()), Replacements); if (!QualifiedFunc) return QualifiedFunc.takeError(); - return QualifiedFunc->substr(FuncBegin, FuncEnd - FuncBegin + 1); + auto QualifiedFuncString = + QualifiedFunc->substr(FuncBegin, FuncEnd - FuncBegin + 1); + if (!FD->hasBody() && !FD->isExplicitlyDefaulted()) { + QualifiedFuncString.pop_back(); // The semicolon finishing the declaration. + QualifiedFuncString += " {"; + if (!FD->getReturnType()->isVoidType()) + QualifiedFuncString += " return {}; "; + QualifiedFuncString += "}"; + } + return QualifiedFuncString; } // Returns replacements to delete tokens with kind `Kind` in the range @@ -348,6 +357,30 @@ llvm::Expected<InsertionPoint> getInsertionPoint(llvm::StringRef Contents, // initializers. SourceRange getDeletionRange(const FunctionDecl *FD, const syntax::TokenBuffer &TokBuf) { + if (!FD->hasBody()) { + if (!FD->isExplicitlyDefaulted()) + return {}; + + auto tokens = TokBuf.expandedTokens(FD->getSourceRange()); + for (auto It = std::rbegin(tokens); It != std::rend(tokens); ++It) { + if (It->kind() == tok::kw_default) { + const auto nextIt = std::next(It); + if (nextIt == std::rend(tokens)) + return {}; + const auto &ExpandedEquals = *nextIt; + if (ExpandedEquals.kind() != tok::equal) + return {}; + auto SpelledEquals = + TokBuf.spelledForExpanded(llvm::ArrayRef(ExpandedEquals)); + if (!SpelledEquals) + return {}; + return {SpelledEquals->front().location(), + FD->getDefaultLoc().getLocWithOffset(1)}; + } + } + return {}; + } + auto DeletionRange = FD->getBody()->getSourceRange(); if (auto *CD = llvm::dyn_cast<CXXConstructorDecl>(FD)) { // AST doesn't contain the location for ":" in ctor initializers. Therefore @@ -405,7 +438,9 @@ class DefineOutline : public Tweak { return CodeAction::REFACTOR_KIND; } std::string title() const override { - return "Move function body to out-of-line"; + if (Source->doesThisDeclarationHaveABody()) + return "Move function body to out-of-line"; + return "Create function body out-of-line"; } bool prepare(const Selection &Sel) override { @@ -417,9 +452,8 @@ class DefineOutline : public Tweak { return false; Source = getSelectedFunction(Sel.ASTSelection.commonAncestor()); - // Bail out if the selection is not a in-line function definition. - if (!Source || !Source->doesThisDeclarationHaveABody() || - Source->isOutOfLine()) + // Bail out if the selection is not a function declaration. + if (!Source || Source->isDeleted() || Source->isOutOfLine()) return false; // Bail out if this is a function template or specialization, as their @@ -483,12 +517,17 @@ class DefineOutline : public Tweak { if (!Effect) return Effect.takeError(); - tooling::Replacements HeaderUpdates(tooling::Replacement( - Sel.AST->getSourceManager(), - CharSourceRange::getTokenRange(*toHalfOpenFileRange( - SM, Sel.AST->getLangOpts(), - getDeletionRange(Source, Sel.AST->getTokens()))), - ";")); + tooling::Replacements HeaderUpdates; + auto DeletionRange = getDeletionRange(Source, Sel.AST->getTokens()); + if (DeletionRange.isValid()) { + if (auto Error = HeaderUpdates.add(tooling::Replacement( + Sel.AST->getSourceManager(), + CharSourceRange::getTokenRange(*toHalfOpenFileRange( + SM, Sel.AST->getLangOpts(), DeletionRange)), + ";"))) { + return Error; + } + } if (Source->isInlineSpecified()) { auto DelInline = diff --git a/clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp b/clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp index d1e60b070f20e95..dede5f7c3c25e8b 100644 --- a/clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp +++ b/clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp @@ -28,9 +28,6 @@ TEST_F(DefineOutlineTest, TriggersOnFunctionDecl) { FileName = "Test.hpp"; // Not available unless function name or fully body is selected. EXPECT_UNAVAILABLE(R"cpp( - // Not a definition - vo^i[[d^ ^f]]^oo(); - [[vo^id ]]foo[[()]] {[[ [[(void)(5+3); return;]] @@ -63,10 +60,9 @@ TEST_F(DefineOutlineTest, TriggersOnFunctionDecl) { return; }]]]])cpp"); - // Not available on defaulted/deleted members. + // Not available on deleted members. EXPECT_UNAVAILABLE(R"cpp( class Foo { - Fo^o() = default; F^oo(const Foo&) = delete; };)cpp"); @@ -128,12 +124,24 @@ TEST_F(DefineOutlineTest, ApplyTest) { llvm::StringRef ExpectedHeader; llvm::StringRef ExpectedSource; } Cases[] = { - // Simple check + // Simple check: Move { "void fo^o() { return; }", "void foo() ;", "void foo() { return; }", }, + // Simple check: Create + { + "void fo^o(int i = 5);", + "void foo(int i = 5);", + "void foo(int i ) {}", + }, + // Simple check: Create with return value + { + "int fo^o();", + "int foo();", + "int foo() { return {}; }", + }, // Inline specifier. { "inline void fo^o() { return; }", @@ -205,6 +213,18 @@ TEST_F(DefineOutlineTest, ApplyTest) { };)cpp", "Foo::Foo(int z) __attribute__((weak)) : bar(2){}\n", }, + // Default ctor + { + R"cpp( + class Foo { + F^oo() = default; + };)cpp", + R"cpp( + class Foo { + Foo() ; + };)cpp", + "Foo::Foo() = default;", + }, // Virt specifiers. { R"cpp( `````````` </details> https://github.com/llvm/llvm-project/pull/71950 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
