Author: Kadir Cetinkaya Date: 2019-12-13T10:07:18+01:00 New Revision: 087528a331786228221d7a56a51ab97a3fcac8f1
URL: https://github.com/llvm/llvm-project/commit/087528a331786228221d7a56a51ab97a3fcac8f1 DIFF: https://github.com/llvm/llvm-project/commit/087528a331786228221d7a56a51ab97a3fcac8f1.diff LOG: [clangd] Add "inline" keyword to prevent ODR-violations in DefineInline Reviewers: ilya-biryukov, hokein Subscribers: MaskRay, jkorous, arphaman, usaxena95, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D68261 Added: Modified: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp clang-tools-extra/clangd/unittests/TweakTests.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp b/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp index 3bc5df0edbfd..57690ee3d684 100644 --- 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" @@ -360,6 +361,25 @@ const SourceLocation getBeginLoc(const FunctionDecl *FD) { return FD->getBeginLoc(); } +llvm::Optional<tooling::Replacement> +addInlineIfInHeader(const FunctionDecl *FD) { + // This includes inline functions and constexpr functions. + if (FD->isInlined() || llvm::isa<CXXMethodDecl>(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()); + + // If it is not a header we don't need to mark function as "inline". + if (!isHeaderFile(FileName, FD->getASTContext().getLangOpts())) + return llvm::None; + + return tooling::Replacement(SM, FD->getInnerLocStart(), 0, "inline "); +} + /// Moves definition of a function/method to its declaration location. /// Before: /// a.h: @@ -436,6 +456,7 @@ class DefineInline : public Tweak { "Couldn't find semicolon for target declaration."); } + auto AddInlineIfNecessary = addInlineIfInHeader(Target); auto ParamReplacements = renameParameters(Target, Source); if (!ParamReplacements) return ParamReplacements.takeError(); @@ -446,6 +467,13 @@ class DefineInline : public Tweak { 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); + } + auto DefRange = toHalfOpenFileRange( SM, AST.getLangOpts(), SM.getExpansionRange(CharSourceRange::getCharRange(getBeginLoc(Source), @@ -462,9 +490,8 @@ class DefineInline : public Tweak { llvm::SmallVector<std::pair<std::string, Edit>, 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/TweakTests.cpp b/clang-tools-extra/clangd/unittests/TweakTests.cpp index d50d27afb1c8..ebeea82864cb 100644 --- a/clang-tools-extra/clangd/unittests/TweakTests.cpp +++ b/clang-tools-extra/clangd/unittests/TweakTests.cpp @@ -1864,6 +1864,66 @@ TEST_F(DefineInlineTest, QualifyWithUsingDirectives) { EXPECT_EQ(apply(Test), Expected) << Test; } +TEST_F(DefineInlineTest, AddInline) { + llvm::StringMap<std::string> EditedFiles; + ExtraFiles["a.h"] = "void foo();"; + apply(R"cpp(#include "a.h" + void fo^o() {})cpp", &EditedFiles); + 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", &EditedFiles); + 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", &EditedFiles); + 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", &EditedFiles); + 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", &EditedFiles); + 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 <typename T> void foo();"; + apply(R"cpp(#include "a.h" + template <typename T> + void fo^o() {})cpp", &EditedFiles); + EXPECT_THAT(EditedFiles, + testing::ElementsAre(FileWithContents( + testPath("a.h"), "template <typename T> void foo(){}"))); + + // Specializations needs to be marked "inline". + ExtraFiles["a.h"] = R"cpp( + template <typename T> void foo(); + template <> void foo<int>();)cpp"; + apply(R"cpp(#include "a.h" + template <> + void fo^o<int>() {})cpp", &EditedFiles); + EXPECT_THAT(EditedFiles, + testing::ElementsAre(FileWithContents(testPath("a.h"), + R"cpp( + template <typename T> void foo(); + template <> inline void foo<int>(){})cpp"))); +} + TWEAK_TEST(DefineOutline); TEST_F(DefineOutlineTest, TriggersOnFunctionDecl) { FileName = "Test.cpp"; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
