[clang-tools-extra] 9f251ee - [clangd] Define out-of-line availability checks
Kadir Cetinkaya via cfe-commits
cfe-commits at lists.llvm.org
Tue Dec 3 23:28:27 PST 2019
Author: Kadir Cetinkaya
Date: 2019-12-04T08:21:09+01:00
New Revision: 9f251eece46675ba0095baddebe90512abde6337
URL: https://github.com/llvm/llvm-project/commit/9f251eece46675ba0095baddebe90512abde6337
DIFF: https://github.com/llvm/llvm-project/commit/9f251eece46675ba0095baddebe90512abde6337.diff
LOG: [clangd] Define out-of-line availability checks
Summary:
Initial availability checks for performing define out-of-line code
action, which is a refactoring that will help users move function/method
definitions from headers to implementation files.
Proposed implementation only checks whether we have an interesting selection,
namely function name or full function definition/body.
Reviewers: hokein
Subscribers: mgorny, ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D69266
Added:
clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
Modified:
clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
clang-tools-extra/clangd/unittests/TweakTesting.cpp
clang-tools-extra/clangd/unittests/TweakTesting.h
clang-tools-extra/clangd/unittests/TweakTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt b/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
index ddf10a2ca2ba..cd68d7afe6ab 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
+++ b/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
@@ -15,6 +15,7 @@ add_clang_library(clangDaemonTweaks OBJECT
AnnotateHighlightings.cpp
DumpAST.cpp
DefineInline.cpp
+ DefineOutline.cpp
ExpandAutoType.cpp
ExpandMacro.cpp
ExtractFunction.cpp
diff --git a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
new file mode 100644
index 000000000000..c06945a632cc
--- /dev/null
+++ b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -0,0 +1,109 @@
+//===--- DefineOutline.cpp ---------------------------------------*- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "HeaderSourceSwitch.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/Stmt.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/Support/Path.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+// Deduces the FunctionDecl from a selection. Requires either the function body
+// or the function decl to be selected. Returns null if none of the above
+// criteria is met.
+// FIXME: This is shared with define inline, move them to a common header once
+// we have a place for such.
+const FunctionDecl *getSelectedFunction(const SelectionTree::Node *SelNode) {
+ if (!SelNode)
+ return nullptr;
+ const ast_type_traits::DynTypedNode &AstNode = SelNode->ASTNode;
+ if (const FunctionDecl *FD = AstNode.get<FunctionDecl>())
+ return FD;
+ if (AstNode.get<CompoundStmt>() &&
+ SelNode->Selected == SelectionTree::Complete) {
+ if (const SelectionTree::Node *P = SelNode->Parent)
+ return P->ASTNode.get<FunctionDecl>();
+ }
+ return nullptr;
+}
+
+/// Moves definition of a function/method to an appropriate implementation file.
+///
+/// Before:
+/// a.h
+/// void foo() { return; }
+/// a.cc
+/// #include "a.h"
+///
+/// ----------------
+///
+/// After:
+/// a.h
+/// void foo();
+/// a.cc
+/// #include "a.h"
+/// void foo() { return; }
+class DefineOutline : public Tweak {
+public:
+ const char *id() const override;
+
+ bool hidden() const override { return true; }
+ Intent intent() const override { return Intent::Refactor; }
+ std::string title() const override {
+ return "Move function body to out-of-line.";
+ }
+
+ bool prepare(const Selection &Sel) override {
+ // Bail out if we are not in a header file.
+ // FIXME: We might want to consider moving method definitions below class
+ // definition even if we are inside a source file.
+ if (!isHeaderFile(Sel.AST.getSourceManager().getFilename(Sel.Cursor),
+ Sel.AST.getASTContext().getLangOpts()))
+ 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())
+ return false;
+
+ // Bail out in templated classes, as it is hard to spell the class name, i.e
+ // if the template parameter is unnamed.
+ if (auto *MD = llvm::dyn_cast<CXXMethodDecl>(Source)) {
+ if (MD->getParent()->isTemplated())
+ return false;
+ }
+
+ // Note that we don't check whether an implementation file exists or not in
+ // the prepare, since performing disk IO on each prepare request might be
+ // expensive.
+ return true;
+ }
+
+ Expected<Effect> apply(const Selection &Sel) override {
+ return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "Not implemented yet");
+ }
+
+private:
+ const FunctionDecl *Source = nullptr;
+};
+
+REGISTER_TWEAK(DefineOutline);
+
+} // namespace
+} // namespace clangd
+} // namespace clang
diff --git a/clang-tools-extra/clangd/unittests/TweakTesting.cpp b/clang-tools-extra/clangd/unittests/TweakTesting.cpp
index 3331a3d93715..599b24f3cc76 100644
--- a/clang-tools-extra/clangd/unittests/TweakTesting.cpp
+++ b/clang-tools-extra/clangd/unittests/TweakTesting.cpp
@@ -63,12 +63,14 @@ std::pair<unsigned, unsigned> rangeOrPoint(const Annotations &A) {
cantFail(positionToOffset(A.code(), SelectionRng.end))};
}
-MATCHER_P6(TweakIsAvailable, TweakID, Ctx, Header, ExtraArgs, ExtraFiles, Index,
+MATCHER_P7(TweakIsAvailable, TweakID, Ctx, Header, ExtraArgs, ExtraFiles, Index,
+ FileName,
(TweakID + (negation ? " is unavailable" : " is available")).str()) {
std::string WrappedCode = wrap(Ctx, arg);
Annotations Input(WrappedCode);
auto Selection = rangeOrPoint(Input);
TestTU TU;
+ TU.Filename = FileName;
TU.HeaderCode = Header;
TU.Code = Input.code();
TU.ExtraArgs = ExtraArgs;
@@ -91,6 +93,7 @@ std::string TweakTest::apply(llvm::StringRef MarkedCode,
auto Selection = rangeOrPoint(Input);
TestTU TU;
+ TU.Filename = FileName;
TU.HeaderCode = Header;
TU.AdditionalFiles = std::move(ExtraFiles);
TU.Code = Input.code();
@@ -132,7 +135,7 @@ std::string TweakTest::apply(llvm::StringRef MarkedCode,
::testing::Matcher<llvm::StringRef> TweakTest::isAvailable() const {
return TweakIsAvailable(llvm::StringRef(TweakID), Context, Header, ExtraArgs,
- ExtraFiles, Index.get());
+ ExtraFiles, Index.get(), FileName);
}
std::vector<std::string> TweakTest::expandCases(llvm::StringRef MarkedCode) {
diff --git a/clang-tools-extra/clangd/unittests/TweakTesting.h b/clang-tools-extra/clangd/unittests/TweakTesting.h
index ffcf5a0c7ea2..10186f859bae 100644
--- a/clang-tools-extra/clangd/unittests/TweakTesting.h
+++ b/clang-tools-extra/clangd/unittests/TweakTesting.h
@@ -12,6 +12,7 @@
#include "TestTU.h"
#include "index/Index.h"
#include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/StringRef.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
#include <memory>
@@ -62,6 +63,8 @@ class TweakTest : public ::testing::Test {
// testcases.
std::string Header;
+ llvm::StringRef FileName = "TestTU.cpp";
+
// Extra flags passed to the compilation in apply().
std::vector<const char *> ExtraArgs;
diff --git a/clang-tools-extra/clangd/unittests/TweakTests.cpp b/clang-tools-extra/clangd/unittests/TweakTests.cpp
index dc7699904019..ee69cadf0227 100644
--- a/clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ b/clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -1807,6 +1807,67 @@ TEST_F(DefineInlineTest, QualifyWithUsingDirectives) {
EXPECT_EQ(apply(Test), Expected) << Test;
}
+TWEAK_TEST(DefineOutline);
+TEST_F(DefineOutlineTest, TriggersOnFunctionDecl) {
+ FileName = "Test.cpp";
+ // Not available unless in a header file.
+ EXPECT_UNAVAILABLE(R"cpp(
+ [[void [[f^o^o]]() [[{
+ return;
+ }]]]])cpp");
+
+ 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;]]
+ }]])cpp");
+
+ // Available even if there are no implementation files.
+ EXPECT_AVAILABLE(R"cpp(
+ [[void [[f^o^o]]() [[{
+ return;
+ }]]]])cpp");
+
+ // Not available for out-of-line methods.
+ EXPECT_UNAVAILABLE(R"cpp(
+ class Bar {
+ void baz();
+ };
+
+ [[void [[Bar::[[b^a^z]]]]() [[{
+ return;
+ }]]]])cpp");
+
+ // Basic check for function body and signature.
+ EXPECT_AVAILABLE(R"cpp(
+ class Bar {
+ [[void [[f^o^o]]() [[{ return; }]]]]
+ };
+
+ void foo();
+ [[void [[f^o^o]]() [[{
+ return;
+ }]]]])cpp");
+
+ // Not available on defaulted/deleted members.
+ EXPECT_UNAVAILABLE(R"cpp(
+ class Foo {
+ Fo^o() = default;
+ F^oo(const Foo&) = delete;
+ };)cpp");
+
+ // Not available within templated classes, as it is hard to spell class name
+ // out-of-line in such cases.
+ EXPECT_UNAVAILABLE(R"cpp(
+ template <typename> struct Foo { void fo^o(){} };
+ })cpp");
+}
+
} // namespace
} // namespace clangd
} // namespace clang
More information about the cfe-commits
mailing list