kadircet created this revision.
Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay,
ilya-biryukov, mgorny.
Herald added a project: clang.
Introduces DefineInline action and initial version of availability
checks.
Repository:
rG LLVM Github Monorepo
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/TweakTests.cpp
Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ 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<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);
+ << "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<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,
+ llvm::StringMap<std::string> AdditionalFiles = {}) {
+ return checkAvailable(ID, Input, /*Available=*/false,
+ std::move(AdditionalFiles));
}
llvm::Expected<Tweak::Effect> 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
Index: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
===================================================================
--- /dev/null
+++ 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 <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 *> getNonLocalDecls(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 {
+ Decls.insert(D->getCanonicalDecl());
+ return true;
+ }
+
+ llvm::DenseSet<const Decl *> 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<CXXMethodDecl>(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<NamedDecl>(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<CXXMethodDecl>(Result))
+ Parent = MD->getParent();
+ else if (const auto *FD = llvm::dyn_cast<FieldDecl>(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<Effect> apply(const Selection &Sel) override {
+ return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "Not implemented");
+ }
+};
+
+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
