diff --git a/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt b/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt --- a/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt +++ b/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 diff --git a/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp b/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp @@ -0,0 +1,193 @@ +//===--- 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 "ClangdUnit.h" +#include "Selection.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/ExprCXX.h" +#include "clang/AST/PrettyPrinter.h" +#include "clang/AST/Stmt.h" +#include "clang/AST/TypeLoc.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Basic/SourceManager.h" +#include "clang/Index/IndexDataConsumer.h" +#include "clang/Index/IndexingAction.h" +#include "clang/Lex/Preprocessor.h" +#include "clang/Sema/Lookup.h" +#include "clang/Sema/Sema.h" +#include "llvm/ADT/DenseSet.h" +#include "llvm/Support/Casting.h" +#include "llvm/Support/Error.h" +#include + +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()) + return FD; + if (AstNode.get() && + SelNode->Selected == SelectionTree::Complete) { + if (const SelectionTree::Node *P = SelNode->Parent) + return P->ASTNode.get(); + } + return nullptr; +} + +// Runs clang indexing api to get a list of declarations referenced inside +// function decl. Skips local symbols. +llvm::DenseSet getNonLocalDecls(const FunctionDecl *FD, + ParsedAST &AST) { + class DeclRecorder : public index::IndexDataConsumer { + public: + bool handleDeclOccurence(const Decl *D, index::SymbolRoleSet Roles, + ArrayRef Relations, + SourceLocation Loc, ASTNodeInfo ASTNode) override { + Decls.insert(D->getCanonicalDecl()); + return true; + } + + llvm::DenseSet Decls; + }; + DeclRecorder Recorder; + index::IndexingOptions Opts; + Opts.SystemSymbolFilter = + index::IndexingOptions::SystemSymbolFilterKind::None; + + index::indexTopLevelDecls(AST.getASTContext(), AST.getPreprocessor(), {FD}, + Recorder, std::move(Opts)); + return Recorder.Decls; +} + +// 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 occuring before Target, +// unless they are declared in the same class. +bool checkDeclsAreVisible(const FunctionDecl *Source, + const FunctionDecl *Target, ParsedAST &AST) { + 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(Target)) + Class = MD->getParent(); + + const SourceManager &SM = AST.getSourceManager(); + const auto Decls = getNonLocalDecls(Source, AST); + for (const Decl *D : Decls) { + if (D == Target) + continue; + const NamedDecl *ND = llvm::dyn_cast(D); + if (!ND) + continue; + DeclContextLookupResult LR = TargetContext->lookup(ND->getIdentifier()); + bool FoundVisible = false; + for (auto Result : LR) { + // FIXME: Allow declarations from different files with include insertion. + if (!SM.isWrittenInSameFile(Result->getLocation(), Target->getLocation())) + continue; + // Since we've got the full translation unit in the AST, make sure + // declaration of Result comes before Target. + if (SM.isBeforeInTranslationUnit(Result->getLocation(), + Target->getLocation())) { + FoundVisible = true; + break; + } + // Or they are in the same class. + if (!Class) + continue; + const RecordDecl *Parent = nullptr; + if (const auto *MD = llvm::dyn_cast(Result)) + Parent = MD->getParent(); + else if (const auto *FD = llvm::dyn_cast(Result)) + Parent = FD->getParent(); + if (Parent == Class) { + FoundVisible = true; + break; + } + } + if (!FoundVisible) + return false; + } + return true; +} + +/// 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 { + 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; + const FunctionDecl *Source = getSelectedFunction(SelNode); + if (!Source || !Source->isThisDeclarationADefinition()) + return false; + + const FunctionDecl *Target = Source->getCanonicalDecl(); + // The only declaration is Source. No other declaration to move function + // body. + // FIXME: Figure out a suitable location to put declaration. Possibly + // using other declarations in the AST. + if (Target == Source) + return false; + + // Check if the decls referenced in function body are visible in the + // declaration location. + if (!checkDeclsAreVisible(Source, Target, Sel.AST)) + return false; + + return true; + } + + Expected apply(const Selection &Sel) override { + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "Not implemented"); + } +}; + +REGISTER_TWEAK(DefineInline); + +} // namespace +} // namespace clangd +} // namespace clang diff --git a/clang-tools-extra/clangd/unittests/TweakTests.cpp b/clang-tools-extra/clangd/unittests/TweakTests.cpp --- a/clang-tools-extra/clangd/unittests/TweakTests.cpp +++ b/clang-tools-extra/clangd/unittests/TweakTests.cpp @@ -41,26 +41,31 @@ .str(); } -void checkAvailable(StringRef ID, llvm::StringRef Input, bool Available) { +void checkAvailable(StringRef ID, llvm::StringRef Input, bool Available, + llvm::StringMap 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); + << "code is " << markRange(Code.code(), Selection) + << Sel.ASTSelection; else EXPECT_THAT_EXPECTED(T, Failed()) - << "code is " << markRange(Code.code(), Selection); + << "code is " << markRange(Code.code(), Selection) + << Sel.ASTSelection; }; for (auto P : Code.points()) CheckOver(Range{P, P}); @@ -69,13 +74,17 @@ } /// 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, + llvm::StringMap 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, + llvm::StringMap AdditionalFiles = {}) { + return checkAvailable(ID, Input, /*Available=*/false, + std::move(AdditionalFiles)); } llvm::Expected apply(StringRef ID, llvm::StringRef Input) { @@ -131,10 +140,10 @@ /// Check if apply returns an error and that the @ErrorMessage is contained /// in that error void checkApplyContainsError(llvm::StringRef ID, llvm::StringRef Input, - const std::string& ErrorMessage) { + const std::string &ErrorMessage) { auto Result = apply(ID, Input); - ASSERT_FALSE(Result) << "expected error message:\n " << ErrorMessage << - "\non input:" << Input; + ASSERT_FALSE(Result) << "expected error message:\n " << ErrorMessage + << "\non input:" << Input; EXPECT_THAT(llvm::toString(Result.takeError()), testing::HasSubstr(ErrorMessage)) << Input; @@ -815,6 +824,113 @@ checkTransform(ID, Input, Output); } +TEST(DefineInline, TriggersOnFunctionDecl) { + llvm::StringLiteral ID = "DefineInline"; + + // Basic check for function body and signature. + checkAvailable(ID, 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"); + + checkNotAvailable(ID, R"cpp( + // Not a definition + vo^i[[d^ ^f]]^oo(); + + [[vo^id ]]foo[[()]] {[[ + [[(void)(5+3); + return;]] + }]] + )cpp"); +} + +TEST(DefineInline, NoDecl) { + llvm::StringLiteral ID = "DefineInline"; + + checkNotAvailable(ID, 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(DefineInline, ReferencedDecls) { + llvm::StringLiteral ID = "DefineInline"; + + checkAvailable(ID, R"cpp( + void bar(); + void foo(int test); + + void fo^o(int baz) { + int x = 10; + bar(); + })cpp"); + + checkAvailable(ID, R"cpp( + #include "a.h" + void fo^o(int baz) { + int x = 10; + bar(); + })cpp", + true, {{"a.h", "void bar(); void foo(int test);"}}); + + // Internal symbol usage. + checkNotAvailable(ID, 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. + checkNotAvailable(ID, R"cpp( + void foo(); + void bar(); + + void fo^o() { + bar(); + })cpp"); + + // Order doesn't matter within a class. + checkAvailable(ID, R"cpp( + class Bar { + void foo(); + void bar(); + }; + + void Bar::fo^o() { + bar(); + })cpp"); + + // FIXME: Perform include insertion to make symbol visible. + checkNotAvailable(ID, 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