[clangd] [C++20] [Modules] Add modules suffix for 'Header' Source Switch

[ChuanqiXu9]

Support the trivial "header"/source switch for module interfaces.

I initially thought the naming are bad and we should rename it. But later I feel it is better to split patches as much as possible.

From the codes it looks like there are problems. e.g., isHeaderFile. But let's try to fix them in different patches.

[llvmbot]

@llvm/pr-subscribers-clangd
@llvm/pr-subscribers-clang-tools-extra

@llvm/pr-subscribers-clang-modules

Author: Chuanqi Xu (ChuanqiXu9)

Changes

Support the trivial "header"/source switch for module interfaces.

I initially thought the naming are bad and we should rename it. But later I feel it is better to split patches as much as possible.

From the codes it looks like there are problems. e.g., isHeaderFile. But let's try to fix them in different patches.

---

Full diff: https://github.com/llvm/llvm-project/pull/131591.diff

2 Files Affected:

• (modified) clang-tools-extra/clangd/HeaderSourceSwitch.cpp (+3-1)
• (modified) clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp (+38)

diff --git a/clang-tools-extra/clangd/HeaderSourceSwitch.cpp b/clang-tools-extra/clangd/HeaderSourceSwitch.cpp
index 2351858cc6297..d54c3668570eb 100644
--- a/clang-tools-extra/clangd/HeaderSourceSwitch.cpp
+++ b/clang-tools-extra/clangd/HeaderSourceSwitch.cpp
@@ -22,7 +22,9 @@ std::optional<Path> getCorrespondingHeaderOrSource(
     PathRef OriginalFile, llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS) {
   llvm::StringRef SourceExtensions[] = {".cpp", ".c", ".cc", ".cxx",
                                         ".c++", ".m", ".mm"};
-  llvm::StringRef HeaderExtensions[] = {".h", ".hh", ".hpp", ".hxx", ".inc"};
+  llvm::StringRef HeaderExtensions[] = {".h",    ".hh",   ".hpp", ".hxx",
+                                        ".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 6e280f5b085a9..e600207de458a 100644
--- a/clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp
@@ -76,6 +76,44 @@ TEST(HeaderSourceSwitchTest, FileHeuristic) {
   EXPECT_FALSE(PathResult.has_value());
 }
 
+TEST(HeaderSourceSwitchTest, ModuleInterfaces) {
+  MockFS FS;
+
+  auto FooCC = testPath("foo.cc");
+  auto FooCPPM = testPath("foo.cppm");
+  FS.Files[FooCC];
+  FS.Files[FooCPPM];
+  std::optional<Path> PathResult =
+      getCorrespondingHeaderOrSource(FooCC, FS.view(std::nullopt));
+  EXPECT_TRUE(PathResult.has_value());
+  ASSERT_EQ(*PathResult, FooCPPM);
+
+  auto Foo2CPP = testPath("foo2.cpp");
+  auto Foo2CCM = testPath("foo2.ccm");
+  FS.Files[Foo2CPP];
+  FS.Files[Foo2CCM];
+  PathResult = getCorrespondingHeaderOrSource(Foo2CPP, FS.view(std::nullopt));
+  EXPECT_TRUE(PathResult.has_value());
+  ASSERT_EQ(*PathResult, Foo2CCM);
+
+  auto Foo3CXX = testPath("foo3.cxx");
+  auto Foo3CXXM = testPath("foo3.cxxm");
+  FS.Files[Foo3CXX];
+  FS.Files[Foo3CXXM];
+  PathResult = getCorrespondingHeaderOrSource(Foo3CXX, FS.view(std::nullopt));
+  EXPECT_TRUE(PathResult.has_value());
+  ASSERT_EQ(*PathResult, Foo3CXXM);
+
+  auto Foo4CPLUSPLUS = testPath("foo4.c++");
+  auto Foo4CPLUSPLUSM = testPath("foo4.c++m");
+  FS.Files[Foo4CPLUSPLUS];
+  FS.Files[Foo4CPLUSPLUSM];
+  PathResult =
+      getCorrespondingHeaderOrSource(Foo4CPLUSPLUS, FS.view(std::nullopt));
+  EXPECT_TRUE(PathResult.has_value());
+  ASSERT_EQ(*PathResult, Foo4CPLUSPLUSM);
+}
+
 MATCHER_P(declNamed, Name, "") {
   if (const NamedDecl *ND = dyn_cast<NamedDecl>(arg))
     if (ND->getQualifiedNameAsString() == Name)

[Bigcheese]

Is the expectation here that people will name implementation units "foo.cpp" and the corresponding interface unit "foo.cppm"? I guess that's reasonable.

[ChuanqiXu9]

Is the expectation here that people will name implementation units "foo.cpp" and the corresponding interface unit "foo.cppm"? I guess that's reasonable.

This is what we did in practice. The gap may be, we found module implementation units is not good. It will introduce too many dependencies. Then we found it is actually good to use module implementation partition units as the implementation units. It is fine as long as we don't generate the BMI. So it is common to have foo.cpp and foo.cppm pattern.

[ChuanqiXu9]

@kadircet ping~