diff --git a/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp b/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp --- a/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp +++ b/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp @@ -32,6 +32,7 @@ #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" #include "clang/Basic/TokenKinds.h" +#include "clang/Driver/Types.h" #include "clang/Index/IndexDataConsumer.h" #include "clang/Index/IndexSymbol.h" #include "clang/Index/IndexingAction.h" @@ -297,6 +298,30 @@ return FD->getBeginLoc(); } +llvm::Optional +addInlineIfInHeader(const FunctionDecl *FD) { + // This includes inline functions and constexpr functions. + if (FD->isInlined() || llvm::isa(FD)) + return llvm::None; + // Primary template doesn't need inline. + if (FD->isTemplated() && !FD->isFunctionTemplateSpecialization()) + return llvm::None; + + const SourceManager &SM = FD->getASTContext().getSourceManager(); + llvm::StringRef FileName = SM.getFilename(FD->getLocation()); + + auto Type = driver::types::lookupTypeForExtension( + llvm::sys::path::extension(FileName).substr(1)); + + // If it is not a header we don't need to mark function as "inline". + if (Type == driver::types::TY_INVALID || + !driver::types::onlyPrecompileType(Type)) { + return llvm::None; + } + + return tooling::Replacement(SM, FD->getInnerLocStart(), 0, "inline "); +} + /// Moves definition of a function/method to its declaration location. /// Before: /// a.h: @@ -368,6 +393,7 @@ "Couldn't find semicolon for target declaration"); } + auto AddInlineIfNecessary = addInlineIfInHeader(Target); auto ParamReplacements = renameParameters(Target, Source); if (!ParamReplacements) return ParamReplacements.takeError(); @@ -378,6 +404,13 @@ const tooling::Replacement SemiColonToFuncBody(SM, *SemiColon, 1, *QualifiedBody); + tooling::Replacements TargetFileReplacements(SemiColonToFuncBody); + TargetFileReplacements = TargetFileReplacements.merge(*ParamReplacements); + if (AddInlineIfNecessary) { + if (auto Err = TargetFileReplacements.add(*AddInlineIfNecessary)) + return std::move(Err); + } + SourceLocation BeginLoc = getBeginLoc(Source); unsigned int SourceLen = SM.getFileOffset(Source->getEndLoc()) - SM.getFileOffset(BeginLoc) + 1; @@ -385,9 +418,8 @@ llvm::SmallVector, 2> Edits; // Edit for Target. - auto FE = Effect::fileEdit( - SM, SM.getFileID(*SemiColon), - tooling::Replacements(SemiColonToFuncBody).merge(*ParamReplacements)); + auto FE = Effect::fileEdit(SM, SM.getFileID(*SemiColon), + std::move(TargetFileReplacements)); if (!FE) return FE.takeError(); Edits.push_back(std::move(*FE)); diff --git a/clang-tools-extra/clangd/unittests/TweakTesting.cpp b/clang-tools-extra/clangd/unittests/TweakTesting.cpp --- a/clang-tools-extra/clangd/unittests/TweakTesting.cpp +++ b/clang-tools-extra/clangd/unittests/TweakTesting.cpp @@ -114,7 +114,7 @@ if (It.first() == testPath(TU.Filename)) EditedMainFile = Unwrapped; else - EditedFiles.try_emplace(It.first(), Unwrapped.str()); + EditedFiles[It.first()] = Unwrapped.str(); } return EditedMainFile; } 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 @@ -1307,6 +1307,66 @@ )cpp"); } + +TEST_F(DefineInlineTest, AddInline) { + ExtraFiles["a.h"] = "void foo();"; + apply(R"cpp(#include "a.h" + void fo^o() {})cpp"); + EXPECT_THAT(EditedFiles, testing::ElementsAre(FileWithContents( + testPath("a.h"), "inline void foo(){}"))); + + // Check we put inline before cv-qualifiers. + ExtraFiles["a.h"] = "const int foo();"; + apply(R"cpp(#include "a.h" + const int fo^o() {})cpp"); + EXPECT_THAT(EditedFiles, testing::ElementsAre(FileWithContents( + testPath("a.h"), "inline const int foo(){}"))); + + // No double inline. + ExtraFiles["a.h"] = "inline void foo();"; + apply(R"cpp(#include "a.h" + inline void fo^o() {})cpp"); + EXPECT_THAT(EditedFiles, testing::ElementsAre(FileWithContents( + testPath("a.h"), "inline void foo(){}"))); + + // Constexprs don't need "inline". + ExtraFiles["a.h"] = "constexpr void foo();"; + apply(R"cpp(#include "a.h" + constexpr void fo^o() {})cpp"); + EXPECT_THAT(EditedFiles, testing::ElementsAre(FileWithContents( + testPath("a.h"), "constexpr void foo(){}"))); + + // Class members don't need "inline". + ExtraFiles["a.h"] = "struct Foo { void foo(); }"; + apply(R"cpp(#include "a.h" + void Foo::fo^o() {})cpp"); + EXPECT_THAT(EditedFiles, + testing::ElementsAre(FileWithContents( + testPath("a.h"), "struct Foo { void foo(){} }"))); + + // Function template doesn't need to be "inline"d. + ExtraFiles["a.h"] = "template void foo();"; + apply(R"cpp(#include "a.h" + template + void fo^o() {})cpp"); + EXPECT_THAT(EditedFiles, + testing::ElementsAre(FileWithContents( + testPath("a.h"), "template void foo(){}"))); + + // Specializations needs to be marked "inline". + ExtraFiles["a.h"] = R"cpp( + template void foo(); + template <> void foo();)cpp"; + apply(R"cpp(#include "a.h" + template <> + void fo^o() {})cpp"); + EXPECT_THAT(EditedFiles, + testing::ElementsAre(FileWithContents(testPath("a.h"), + R"cpp( + template void foo(); + template <> inline void foo(){})cpp"))); +} + } // namespace } // namespace clangd } // namespace clang