https://github.com/dmaclach created https://github.com/llvm/llvm-project/pull/213751
When inserting a header with an `#import` directive, if an existing `#include` directive for the same header and quotation style is found, replace it with the `#import` directive instead of adding a duplicate. This is based on two assumptions: 1. that `#import` outranks `#include` since headers that are included are assumed to have appropriate include guards to prevent multiple inclusions 2. that there is no good reason to have an include and import of the same header in a given source file. >From 7f665b0b4c30b1242c709f8fd5b91a6ffe1407c2 Mon Sep 17 00:00:00 2001 From: Dave MacLachlan <[email protected]> Date: Mon, 3 Aug 2026 13:00:54 -0700 Subject: [PATCH] Replace existing `#include` directives with `#import` when inserting an import. When inserting a header with an `#import` directive, if an existing `#include` directive for the same header and quotation style is found, replace it with the `#import` directive instead of adding a duplicate. This is based on two assumptions: 1) that `#import` outranks `#include` since headers that are included are assumed to have appropriate include guards to prevent multiple inclusions 2) that there is no good reason to have an include and import of the same header in a given source file. --- .../lib/Tooling/Inclusions/HeaderIncludes.cpp | 29 +++++++++-- .../unittests/Tooling/HeaderIncludesTest.cpp | 48 +++++++++++++++---- 2 files changed, 64 insertions(+), 13 deletions(-) diff --git a/clang/lib/Tooling/Inclusions/HeaderIncludes.cpp b/clang/lib/Tooling/Inclusions/HeaderIncludes.cpp index eef06fb5e0517..2603df929b469 100644 --- a/clang/lib/Tooling/Inclusions/HeaderIncludes.cpp +++ b/clang/lib/Tooling/Inclusions/HeaderIncludes.cpp @@ -454,11 +454,30 @@ HeaderIncludes::insert(llvm::StringRef Header, bool IsAngled, // FIXME: figure out if this is the best behavior. auto It = ExistingIncludes.find(Header); if (It != ExistingIncludes.end()) { - for (const auto &Inc : It->second) - if (Inc.Directive == Directive && - ((IsAngled && StringRef(Inc.Name).starts_with("<")) || - (!IsAngled && StringRef(Inc.Name).starts_with("\"")))) - return std::nullopt; + for (const auto &Inc : It->second) { + bool SameQuotation = (IsAngled && StringRef(Inc.Name).starts_with("<")) || + (!IsAngled && StringRef(Inc.Name).starts_with("\"")); + if (SameQuotation) { + // If the directive is the same, or if the directive is an include and + // the existing directive is an import, then we don't need to insert + // the header. + if ((Inc.Directive == Directive) || + (Inc.Directive == IncludeDirective::Import && + Directive == IncludeDirective::Include)) { + return std::nullopt; + } + + // "import" outranks "include" with the assumption that includes are + // designed to handle multiple inclusions while import is not. + char Open = IsAngled ? '<' : '"'; + char Close = IsAngled ? '>' : '"'; + std::string NewInclude = + llvm::formatv("#import {0}{1}{2}\n", Open, Header, Close); + + return tooling::Replacement(FileName, Inc.R.getOffset(), + Inc.R.getLength(), NewInclude); + } + } } std::string Quoted = std::string(llvm::formatv(IsAngled ? "<{0}>" : "\"{0}\"", Header)); diff --git a/clang/unittests/Tooling/HeaderIncludesTest.cpp b/clang/unittests/Tooling/HeaderIncludesTest.cpp index 4c1848252df56..3a42498513fd5 100644 --- a/clang/unittests/Tooling/HeaderIncludesTest.cpp +++ b/clang/unittests/Tooling/HeaderIncludesTest.cpp @@ -7,12 +7,12 @@ //===----------------------------------------------------------------------===// #include "clang/Tooling/Inclusions/HeaderIncludes.h" -#include "../Tooling/ReplacementTest.h" -#include "../Tooling/RewriterTestContext.h" #include "clang/Format/Format.h" #include "clang/Tooling/Core/Replacement.h" +#include "llvm/ADT/StringRef.h" #include "gtest/gtest.h" +#include <cassert> namespace clang { namespace tooling { @@ -65,7 +65,7 @@ TEST_F(HeaderIncludesTest, RepeatedIncludes) { TEST_F(HeaderIncludesTest, InsertImportWithSameInclude) { std::string Code = "#include \"a.h\"\n"; - std::string Expected = Code + "#import \"a.h\"\n"; + std::string Expected = "#import \"a.h\"\n"; EXPECT_EQ(Expected, insert(Code, "\"a.h\"", IncludeDirective::Import)); } @@ -106,20 +106,52 @@ TEST_F(HeaderIncludesTest, ImportWithSpacesAndTabs) { // Try inserting "b.h" again as include - should be blocked. EXPECT_EQ(CodeWithSpaces, insert(CodeWithSpaces, "\"b.h\"", IncludeDirective::Include)); + + // Try inserting "b.h" again as import - should replace. + std::string ExpectedAfterBImport = + "# import \"a.h\"\n#import \"b.h\"\nint x;\n"; + EXPECT_EQ(ExpectedAfterBImport, + insert(CodeWithSpaces, "\"b.h\"", IncludeDirective::Import)); } TEST_F(HeaderIncludesTest, InsertIncludeWhenImportExists) { std::string Code = "#import \"a.h\"\n"; - std::string Expected = Code + "#include \"a.h\"\n"; - // Currently, the logic allows inserting #include even if #import exists - // because the Directive differs. This test verifies this current behavior. + EXPECT_EQ(Code, insert(Code, "\"a.h\"", IncludeDirective::Include)); +} + +TEST_F(HeaderIncludesTest, InsertIncludeAfterImportStays) { + std::string Code = "#import \"a.h\"\n"; + std::string Expected = "#import \"a.h\"\n"; + // Verifies that the import stays and include is not added (or is absorbed). EXPECT_EQ(Expected, insert(Code, "\"a.h\"", IncludeDirective::Include)); } TEST_F(HeaderIncludesTest, InsertImportWhenIncludeExists) { std::string Code = "#include \"a.h\"\n"; - std::string Expected = Code + "#import \"a.h\"\n"; - // Similarly, allows inserting #import even if #include exists. + std::string Expected = "#import \"a.h\"\n"; + // Replaces #include with #import. + EXPECT_EQ(Expected, insert(Code, "\"a.h\"", IncludeDirective::Import)); +} + +TEST_F(HeaderIncludesTest, InsertImportWhenIncludeExistsAngled) { + std::string Code = "#include <a.h>\n"; + std::string Expected = "#import <a.h>\n"; + // Replaces #include with #import. + EXPECT_EQ(Expected, insert(Code, "<a.h>", IncludeDirective::Import)); +} + +TEST_F(HeaderIncludesTest, InsertImportAngledWhenIncludeQuotedExists) { + std::string Code = "#include \"a.h\"\n"; + std::string Expected = Code + "#import <a.h>\n"; + // Different quotation, so it should insert alongside, not replace. + EXPECT_EQ(Expected, insert(Code, "<a.h>", IncludeDirective::Import)); +} + +TEST_F(HeaderIncludesTest, InsertImportQuotedWhenIncludeAngledExists) { + std::string Code = "#include <a.h>\n"; + std::string Expected = "#import \"a.h\"\n#include <a.h>\n"; + // Different quotation, so it should insert alongside, not replace. + // " comes before < in ASCII, so it is inserted before. EXPECT_EQ(Expected, insert(Code, "\"a.h\"", IncludeDirective::Import)); } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
