[clang] [modules] Add err_main_in_named_module

[kish1n]

Revival of #146247 which got reverted for broken test.

Now that #146461 is merged to allow extern "C++" for main, we can merge this change.

[llvmbot]

@llvm/pr-subscribers-libcxx
@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-driver

Author: Ashwin Banwari (ashwinbanwari)

Changes

Revival of #146247 which got reverted for broken test.

Now that #146461 is merged to allow extern "C++" for main, we can merge this change.

---

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

5 Files Affected:

• (modified) clang/docs/ReleaseNotes.rst (+3)
• (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+4)
• (modified) clang/lib/Sema/SemaDecl.cpp (+8)
• (modified) clang/test/Driver/autocomplete.c (+1)
• (modified) clang/test/SemaCXX/modules.cppm (+2)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 3d893e0aa8e2c..023f8ff7951d3 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -661,6 +661,9 @@ Improvements to Clang's diagnostics
   diagnostics when floating-point numbers had both width field and plus or space
   prefix specified. (#GH143951)
 
+- A warning is now emitted when ``main`` is attached to a named module,
+  which can be turned off with ``-Wno-main-attached-to-named-module``. (#GH146247)
+
 - Clang now avoids issuing `-Wreturn-type` warnings in some cases where
   the final statement of a non-void function is a `throw` expression, or
   a call to a function that is trivially known to always throw (i.e., its
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 6e48f3bc88bab..8a05b4b4af31b 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1062,6 +1062,10 @@ def err_constexpr_main : Error<
   "'main' is not allowed to be declared %select{constexpr|consteval}0">;
 def err_deleted_main : Error<"'main' is not allowed to be deleted">;
 def err_mainlike_template_decl : Error<"%0 cannot be a template">;
+def warn_main_in_named_module
+    : ExtWarn<"'main' should not be attached to a named module; consider "
+              "adding C++ language linkage">,
+      InGroup<DiagGroup<"main-attached-to-named-module">>;
 def err_main_returns_nonint : Error<"'main' must return 'int'">;
 def ext_main_returns_nonint : ExtWarn<"return type of 'main' is not 'int'">,
     InGroup<MainReturnType>;
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 61b82b8377e22..de862e1ad8850 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -12486,6 +12486,14 @@ void Sema::CheckMain(FunctionDecl *FD, const DeclSpec &DS) {
                                 : FixItHint());
       FD->setInvalidDecl(true);
     }
+
+    // In C++ [basic.start.main]p3, it is said a program attaching main to a
+    // named module is ill-formed.
+    if (FD->isInNamedModule()) {
+      const SourceLocation start = FD->getTypeSpecStartLoc();
+      Diag(start, diag::warn_main_in_named_module)
+          << FixItHint::CreateInsertion(start, "extern \"C++\" ", true);
+    }
   }
 
   // Treat protoless main() as nullary.
diff --git a/clang/test/Driver/autocomplete.c b/clang/test/Driver/autocomplete.c
index 8cc604dbff875..4983b71496834 100644
--- a/clang/test/Driver/autocomplete.c
+++ b/clang/test/Driver/autocomplete.c
@@ -111,6 +111,7 @@
 // RUN: %clang --autocomplete=-Wma | FileCheck %s -check-prefix=WARNING
 // WARNING: -Wmacro-redefined
 // WARNING-NEXT: -Wmain
+// WARNING-NEXT: -Wmain-attached-to-named-module
 // WARNING-NEXT: -Wmain-return-type
 // WARNING-NEXT: -Wmalformed-warning-check
 // WARNING-NEXT: -Wmany-braces-around-scalar-init
diff --git a/clang/test/SemaCXX/modules.cppm b/clang/test/SemaCXX/modules.cppm
index ddbbc7cd86360..5e0b3be9870c7 100644
--- a/clang/test/SemaCXX/modules.cppm
+++ b/clang/test/SemaCXX/modules.cppm
@@ -41,6 +41,8 @@ struct S {
   export static int n; // expected-error {{expected member name or ';'}}
 };
 
+int main() {} // expected-warning {{'main' should not be attached to a named module; consider adding C++ language linkage}}
+
 // FIXME: Exports of declarations without external linkage are disallowed.
 // Exports of declarations with non-external-linkage types are disallowed.
 

[kish1n]

CC @ChuanqiXu9 for review

[cor3ntin]

Maybe just `'main' never has module linkage;" the "Consider..." is somewhat redundant with the fixit

[ChuanqiXu9]

LGTM

[ChuanqiXu9]

nit: generally we we'll cite it directly:

Suggested change

	// In C++ [basic.start.main]p3, it is said a program attaching main to a
	// named module is ill-formed.
	// [basic.start.main]p3:
	//
	// ....

[ChuanqiXu9]

Please make sure the libc++ tests won't get affected. I am not sure if the modules related tests in libc++ run by default.

[kish1n]

I can't figure out how to make buildbot run the libc++ tests forcefully, so I add a dummy change in libcxx so the tests will run on this PR, and then will remove the change after.

[kish1n]

The only fail is some unrelated test timeout, seems to be flaky. I will remove the dummy libcxx change now so its ready to merge