kadircet updated this revision to Diff 216815.
kadircet added a comment.
- Rebase
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65433/new/
https://reviews.llvm.org/D65433
Files:
clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
clang-tools-extra/clangd/unittests/TweakTesting.h
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
@@ -8,6 +8,7 @@
#include "Annotations.h"
#include "SourceCode.h"
+#include "TestFS.h"
#include "TestTU.h"
#include "TweakTesting.h"
#include "refactor/Tweak.h"
@@ -15,6 +16,7 @@
#include "clang/Basic/LLVM.h"
#include "clang/Rewrite/Core/Rewriter.h"
#include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Error.h"
#include "llvm/Testing/Support/Error.h"
@@ -22,6 +24,8 @@
#include "gmock/gmock.h"
#include "gtest/gtest.h"
#include <cassert>
+#include <string>
+#include <vector>
using llvm::Failed;
using llvm::Succeeded;
@@ -47,20 +51,28 @@
.str();
}
-void checkAvailable(StringRef ID, llvm::StringRef Input, bool Available) {
+struct TransformInputFile {
+ std::string InitialText;
+ std::string ModifiedText;
+};
+
+void checkAvailable(StringRef ID, llvm::StringRef Input, bool Available,
+ const llvm::StringMap<std::string> &AdditionalFiles) {
Annotations Code(Input);
ASSERT_TRUE(0 < Code.points().size() || 0 < Code.ranges().size())
<< "no points of interest specified";
TestTU TU;
TU.Filename = "foo.cpp";
TU.Code = Code.code();
+ TU.AdditionalFiles = std::move(AdditionalFiles);
ParsedAST AST = TU.build();
auto CheckOver = [&](Range Selection) {
unsigned Begin = cantFail(positionToOffset(Code.code(), Selection.start));
unsigned End = cantFail(positionToOffset(Code.code(), Selection.end));
- auto T = prepareTweak(ID, Tweak::Selection(AST, Begin, End));
+ const Tweak::Selection Sel = Tweak::Selection(AST, Begin, End);
+ auto T = prepareTweak(ID, Sel);
if (Available)
EXPECT_THAT_EXPECTED(T, Succeeded())
<< "code is " << markRange(Code.code(), Selection);
@@ -75,13 +87,18 @@
}
/// Checks action is available at every point and range marked in \p Input.
-void checkAvailable(StringRef ID, llvm::StringRef Input) {
- return checkAvailable(ID, Input, /*Available=*/true);
+void checkAvailable(StringRef ID, llvm::StringRef Input,
+ const llvm::StringMap<std::string> &AdditionalFiles = {}) {
+ return checkAvailable(ID, Input, /*Available=*/true,
+ std::move(AdditionalFiles));
}
/// Same as checkAvailable, but checks the action is not available.
-void checkNotAvailable(StringRef ID, llvm::StringRef Input) {
- return checkAvailable(ID, Input, /*Available=*/false);
+void checkNotAvailable(
+ StringRef ID, llvm::StringRef Input,
+ const llvm::StringMap<std::string> &AdditionalFiles = {}) {
+ return checkAvailable(ID, Input, /*Available=*/false,
+ std::move(AdditionalFiles));
}
llvm::Expected<Tweak::Effect> apply(StringRef ID, llvm::StringRef Input) {
@@ -603,6 +620,111 @@
R"cpp(const char * x = "test")cpp");
}
+TWEAK_TEST(DefineInline);
+TEST_F(DefineInlineTest, TriggersOnFunctionDecl) {
+ // Basic check for function body and signature.
+ EXPECT_AVAILABLE(R"cpp(
+ class Bar {
+ void baz();
+ };
+
+ // Should it also trigger on NestedNameSpecifier? i.e "Bar::"
+ [[void [[Bar::[[b^a^z]]]]() [[{
+ return;
+ }]]]]
+
+ void foo();
+ [[void [[f^o^o]]() [[{
+ return;
+ }]]]]
+ )cpp");
+
+ EXPECT_UNAVAILABLE(R"cpp(
+ // Not a definition
+ vo^i[[d^ ^f]]^oo();
+
+ [[vo^id ]]foo[[()]] {[[
+ [[(void)(5+3);
+ return;]]
+ }]]
+ )cpp");
+}
+
+TEST_F(DefineInlineTest, NoDecl) {
+ checkNotAvailable(TweakID, R"cpp(
+ #include "a.h"
+ void bar() {
+ return;
+ }
+ // FIXME: Generate a decl in the header.
+ void fo^o() {
+ return;
+ })cpp",
+ {{"a.h", "void bar();"}});
+}
+
+TEST_F(DefineInlineTest, ReferencedDecls) {
+ EXPECT_AVAILABLE(R"cpp(
+ void bar();
+ void foo(int test);
+
+ void fo^o(int baz) {
+ int x = 10;
+ bar();
+ })cpp");
+
+ checkAvailable(TweakID,
+ R"cpp(
+ #include "a.h"
+ void fo^o(int baz) {
+ int x = 10;
+ bar();
+ })cpp",
+ {{"a.h", "void bar(); void foo(int test);"}});
+
+ // Internal symbol usage.
+ checkNotAvailable(TweakID,
+ R"cpp(
+ #include "a.h"
+ void bar();
+ void fo^o(int baz) {
+ int x = 10;
+ bar();
+ })cpp",
+ {{"a.h", "void foo(int test);"}});
+
+ // FIXME: Move declaration below bar to make it visible.
+ EXPECT_UNAVAILABLE(R"cpp(
+ void foo();
+ void bar();
+
+ void fo^o() {
+ bar();
+ })cpp");
+
+ // Order doesn't matter within a class.
+ EXPECT_AVAILABLE(R"cpp(
+ class Bar {
+ void foo();
+ void bar();
+ };
+
+ void Bar::fo^o() {
+ bar();
+ })cpp");
+
+ // FIXME: Perform include insertion to make symbol visible.
+ checkNotAvailable(TweakID,
+ R"cpp(
+ #include "a.h"
+ #include "b.h"
+ void fo^o(int baz) {
+ int x = 10;
+ bar();
+ })cpp",
+ {{"b.h", "void foo(int test);"}, {"a.h", "void bar();"}});
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/unittests/TweakTesting.h
===================================================================
--- clang-tools-extra/clangd/unittests/TweakTesting.h
+++ clang-tools-extra/clangd/unittests/TweakTesting.h
@@ -33,8 +33,6 @@
// EXPECT_UNAVAILABLE("auto ^X^ = ^foo<int>();");
// }
class TweakTest : public ::testing::Test {
- const char *TweakID;
-
public:
// Inputs are wrapped in file boilerplate before attempting to apply a tweak.
// Context describes the type of boilerplate.
@@ -76,6 +74,8 @@
// Returns a matcher that accepts marked code snippets where the tweak is
// available at the marked range.
::testing::Matcher<llvm::StringRef> isAvailable() const;
+
+ const char *TweakID;
};
#define TWEAK_TEST(TweakID) \
Index: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
@@ -0,0 +1,239 @@
+//===--- DefineInline.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 "AST.h"
+#include "ClangdUnit.h"
+#include "Logger.h"
+#include "Selection.h"
+#include "SourceCode.h"
+#include "refactor/Tweak.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/ASTTypeTraits.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclBase.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclTemplate.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/ExprCXX.h"
+#include "clang/AST/NestedNameSpecifier.h"
+#include "clang/AST/PrettyPrinter.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/Stmt.h"
+#include "clang/AST/TemplateBase.h"
+#include "clang/AST/Type.h"
+#include "clang/AST/TypeLoc.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/TokenKinds.h"
+#include "clang/Index/IndexDataConsumer.h"
+#include "clang/Index/IndexSymbol.h"
+#include "clang/Index/IndexingAction.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Lex/Preprocessor.h"
+#include "clang/Lex/Token.h"
+#include "clang/Sema/Lookup.h"
+#include "clang/Sema/Sema.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/Signals.h"
+#include "llvm/Support/raw_ostream.h"
+#include <cstddef>
+#include <set>
+#include <string>
+#include <unordered_map>
+#include <vector>
+
+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.
+const FunctionDecl *getSelectedFunction(const SelectionTree::Node *SelNode) {
+ 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;
+}
+
+// Runs clang indexing api to get a list of declarations referenced inside
+// function decl. Skips local symbols.
+llvm::DenseSet<const Decl *> getNonLocalDeclRefs(const FunctionDecl *FD,
+ ParsedAST &AST) {
+ class DeclRecorder : public index::IndexDataConsumer {
+ public:
+ bool handleDeclOccurence(const Decl *D, index::SymbolRoleSet Roles,
+ ArrayRef<index::SymbolRelation> Relations,
+ SourceLocation Loc, ASTNodeInfo ASTNode) override {
+ // We only care about referenced decls.
+ if (!(Roles & static_cast<unsigned int>(index::SymbolRole::Reference)))
+ return true;
+
+ // Skip method and field refs, since they are accessed through a VarDecl,
+ // and that is the relavant Decl.
+ if (llvm::isa<CXXMethodDecl>(D) || llvm::isa<FieldDecl>(D))
+ return true;
+
+ // Store the original decl, e.g. specialization rather than templated
+ // decl.
+ DeclRefs.insert(ASTNode.OrigD);
+ return true;
+ }
+
+ llvm::DenseSet<const Decl *> DeclRefs;
+ };
+ DeclRecorder Recorder;
+ index::IndexingOptions Opts;
+ Opts.SystemSymbolFilter =
+ index::IndexingOptions::SystemSymbolFilterKind::None;
+
+ index::indexTopLevelDecls(AST.getASTContext(), AST.getPreprocessor(), {FD},
+ Recorder, std::move(Opts));
+ return Recorder.DeclRefs;
+}
+
+// Checks the decls mentioned in Source are visible in the context of Target.
+// Achives that by performing lookups in Target's DeclContext.
+// Unfortunately these results need to be post-filtered since the AST we get is
+// for the fully parsed TU. Therefore might contain decls that were not visible
+// while parsing Target.
+// Post filtering is currently done by filtering decls defined after Target,
+// unless they are declared in the same class.
+bool checkDeclsAreVisible(const llvm::DenseSet<const Decl *> &DeclRefs,
+ const FunctionDecl *Target, const SourceManager &SM) {
+ const DeclContext *TargetContext = Target->getDeclContext();
+ if (!TargetContext)
+ return false;
+
+ // To be used in visibility check below, decls in a class are visible
+ // independent of order.
+ const RecordDecl *Class = nullptr;
+ if (const auto *MD = llvm::dyn_cast<CXXMethodDecl>(Target))
+ Class = MD->getParent();
+
+ for (const auto *D : DeclRefs) {
+ if (D == Target)
+ continue;
+
+ // FIXME: Allow declarations from different files with include insertion.
+ if (!SM.isWrittenInSameFile(D->getLocation(), Target->getLocation()))
+ return false;
+ // Since we've got the full translation unit in the AST, make sure
+ // declaration of Result comes before Target, or decl and target are in the
+ // same class.
+ if (!SM.isBeforeInTranslationUnit(D->getLocation(),
+ Target->getLocation())) {
+ if (!Class)
+ return false;
+ // Or they are in the same class.
+ const RecordDecl *Parent = nullptr;
+ if (const auto *MD = llvm::dyn_cast<CXXMethodDecl>(D))
+ Parent = MD->getParent();
+ else if (const auto *FD = llvm::dyn_cast<FieldDecl>(D))
+ Parent = FD->getParent();
+ if (Parent != Class)
+ return false;
+ }
+ }
+ return true;
+}
+
+// Returns the canonical declaration for the given FunctionDecl. This will
+// usually be the first declaration in current translation unit with the
+// exception of template specialization. For those we return the previous
+// declaration instead of the first one, since it will be template decl itself
+// rather then declaration of specialization.
+const FunctionDecl *choseCanonical(const FunctionDecl *FD) {
+ if (FD->isFunctionTemplateSpecialization()) {
+ // For specializations CanonicalDecl is the TemplatedDecl, which is not the
+ // target we want to inline into. Instead we use the previous declaration.
+ return FD->getPreviousDecl();
+ }
+ return FD->getCanonicalDecl();
+}
+
+/// Moves definition of a function to its declaration location.
+/// Before:
+/// a.h:
+/// void foo();
+///
+/// a.cc:
+/// void foo() { return; }
+///
+/// ------------------------
+/// After:
+/// a.h:
+/// void foo() { return ; }
+///
+/// a.cc:
+///
+class DefineInline : public Tweak {
+public:
+ const char *id() const override final;
+
+ Intent intent() const override { return Intent::Refactor; }
+ std::string title() const override { return "Inline function definition"; }
+
+ // Returns true when selection is on a function definition that does not
+ // make use of any internal symbols.
+ bool prepare(const Selection &Sel) override {
+ const SelectionTree::Node *SelNode = Sel.ASTSelection.commonAncestor();
+ if (!SelNode)
+ return false;
+ Source = getSelectedFunction(SelNode);
+ if (!Source || !Source->isThisDeclarationADefinition())
+ return false;
+
+ Target = choseCanonical(Source);
+ if (Target == Source) {
+ // The only declaration is Source. No other declaration to move function
+ // body.
+ // FIXME: If we are in an implementation file, figure out a suitable
+ // location to put declaration. Possibly using other declarations in the
+ // AST.
+ return false;
+ }
+
+ // Check if the decls referenced in function body are visible in the
+ // declaration location.
+ auto DeclRefs = getNonLocalDeclRefs(Source, Sel.AST);
+ if (!checkDeclsAreVisible(getNonLocalDeclRefs(Source, Sel.AST), Target,
+ Sel.AST.getSourceManager()))
+ return false;
+
+ return true;
+ }
+
+ Expected<Effect> apply(const Selection &Sel) override {
+ return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "Not implemented yet");
+ }
+
+private:
+ const FunctionDecl *Source = nullptr;
+ const FunctionDecl *Target = nullptr;
+};
+
+REGISTER_TWEAK(DefineInline);
+
+} // namespace
+} // namespace clangd
+} // namespace clang
Index: clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
===================================================================
--- clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
+++ clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
@@ -14,6 +14,7 @@
add_clang_library(clangDaemonTweaks OBJECT
AnnotateHighlightings.cpp
DumpAST.cpp
+ DefineInline.cpp
ExpandAutoType.cpp
ExpandMacro.cpp
ExtractVariable.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
