llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tools-extra Author: Baranov Victor (vbvictor) <details> <summary>Changes</summary> Previously, The "Switch Between Source/Header" action picked `.h` over `.hpp` when both files existed next to a `.cpp` file, because `.h` is listed first in the header-extension list. This patch reorders `HeaderExtensions` and `SourceExtensions` so the `C++`-flavored extensions come before `.h` and `.c`. `C++`-flavor of file is preffered since (at least in my understanding) more people using `clangd` for `C++` than for `C` so switching from `.cpp` should go into `.hpp`, not `.h`. This brings an edje case that when swithing from `.c` it will go into `.hpp` instead of `.h`, but I think this situation is more rare than having `.cpp` and `.hpp` and `.h` combination since `.h` headers can be used as `extern "C"` wrapper of cpp library. Ideally, we should support a mapping between Header and Source (like `c <-> .h`, `.cxx, .cc <-> .hpp, hxx`, but it takes 50-100 lines of additional code and I don't think we should bring this complexity now. --- Full diff: https://github.com/llvm/llvm-project/pull/198152.diff 2 Files Affected: - (modified) clang-tools-extra/clangd/HeaderSourceSwitch.cpp (+2-2) - (modified) clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp (+17) ``````````diff diff --git a/clang-tools-extra/clangd/HeaderSourceSwitch.cpp b/clang-tools-extra/clangd/HeaderSourceSwitch.cpp index ee4bea1401490..6c1e2ca99427e 100644 --- a/clang-tools-extra/clangd/HeaderSourceSwitch.cpp +++ b/clang-tools-extra/clangd/HeaderSourceSwitch.cpp @@ -21,9 +21,9 @@ namespace clangd { std::optional<Path> getCorrespondingHeaderOrSource( PathRef OriginalFile, llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS) { static constexpr llvm::StringRef SourceExtensions[] = { - ".cpp", ".c", ".cc", ".cxx", ".c++", ".m", ".mm"}; + ".cpp", ".cc", ".cxx", ".c++", ".c", ".m", ".mm"}; static constexpr llvm::StringRef HeaderExtensions[] = { - ".h", ".hh", ".hpp", ".hxx", ".inc", + ".hpp", ".hh", ".hxx", ".h++", ".h", ".inc", ".cppm", ".ccm", ".cxxm", ".c++m", ".ixx"}; llvm::StringRef PathExt = llvm::sys::path::extension(OriginalFile); diff --git a/clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp b/clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp index e600207de458a..782e3aacba538 100644 --- a/clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp +++ b/clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp @@ -74,6 +74,23 @@ TEST(HeaderSourceSwitchTest, FileHeuristic) { // string. PathResult = getCorrespondingHeaderOrSource(Invalid, FS.view(std::nullopt)); EXPECT_FALSE(PathResult.has_value()); + + // Test when both .h and .hpp exist next to a .cpp file, prefer .hpp + auto FooHpp = testPath("foo.hpp"); + FS.Files[FooHpp]; + PathResult = getCorrespondingHeaderOrSource(FooCpp, FS.view(std::nullopt)); + EXPECT_TRUE(PathResult.has_value()); + ASSERT_EQ(*PathResult, FooHpp); + + auto BarCc = testPath("bar2.cc"); + auto BarH = testPath("bar2.h"); + auto BarHh = testPath("bar2.hh"); + FS.Files[BarCc]; + FS.Files[BarH]; + FS.Files[BarHh]; + PathResult = getCorrespondingHeaderOrSource(BarCc, FS.view(std::nullopt)); + EXPECT_TRUE(PathResult.has_value()); + ASSERT_EQ(*PathResult, BarHh); } TEST(HeaderSourceSwitchTest, ModuleInterfaces) { `````````` </details> https://github.com/llvm/llvm-project/pull/198152 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
