Skip to content

[C++20] [Modules] Emit Errors when compiling a non-module source as module #102565

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 9, 2024

Conversation

ChuanqiXu9
Copy link
Member

Close #101398

The root cause of the issue is that I removed the codes before and failed to recognize it in time and this was not found for a long time due to it only crashes with invalid codes.

…odule

Close llvm#101398

The root cause of the issue is that I removed the codes before and
failed to recognize it in time and this was not found for a long time
due to it only crashes with invalid codes.
@ChuanqiXu9 ChuanqiXu9 added clang:modules C++20 modules and Clang Header Modules skip-precommit-approval PR for CI feedback, not intended for review labels Aug 9, 2024
@ChuanqiXu9 ChuanqiXu9 self-assigned this Aug 9, 2024
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Aug 9, 2024
@llvmbot
Copy link
Member

llvmbot commented Aug 9, 2024

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-modules

Author: Chuanqi Xu (ChuanqiXu9)

Changes

Close #101398

The root cause of the issue is that I removed the codes before and failed to recognize it in time and this was not found for a long time due to it only crashes with invalid codes.


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

4 Files Affected:

  • (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+2)
  • (modified) clang/lib/Sema/Sema.cpp (+12)
  • (modified) clang/test/CXX/module/dcl.dcl/dcl.module/dcl.module.interface/p1.cppm (+3-1)
  • (added) clang/test/Modules/pr101398.cppm (+5)
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 5cdf36660b2a66..554dbaff2ce0d8 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -11693,6 +11693,8 @@ def err_module_not_defined : Error<
 def err_module_redeclaration : Error<
   "translation unit contains multiple module declarations">;
 def note_prev_module_declaration : Note<"previous module declaration is here">;
+def err_module_declaration_missing : Error<
+  "missing 'export module' declaration in module interface unit">;
 def err_module_declaration_missing_after_global_module_introducer : Error<
   "missing 'module' declaration at end of global module fragment "
   "introduced here">;
diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index 19d8692ee64849..633b8220ffbf11 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -1272,6 +1272,18 @@ void Sema::ActOnEndOfTranslationUnit() {
                                    Module::ExplicitGlobalModuleFragment) {
     Diag(ModuleScopes.back().BeginLoc,
          diag::err_module_declaration_missing_after_global_module_introducer);
+  } else if (getLangOpts().getCompilingModule() ==
+                 LangOptions::CMK_ModuleInterface &&
+             // We can't use ModuleScopes here since ModuleScopes is always
+             // empty if we're compiling the BMI.
+             !getASTContext().getCurrentNamedModule()) {
+    // If we are building a module interface unit, we should have seen the
+    // module declaration.
+    //
+    // FIXME: Make a better guess as to where to put the module declaration.
+    Diag(getSourceManager().getLocForStartOfFile(
+             getSourceManager().getMainFileID()),
+         diag::err_module_declaration_missing);
   }
 
   // Now we can decide whether the modules we're building need an initializer.
diff --git a/clang/test/CXX/module/dcl.dcl/dcl.module/dcl.module.interface/p1.cppm b/clang/test/CXX/module/dcl.dcl/dcl.module/dcl.module.interface/p1.cppm
index 1a01ffac0154ae..84ef85126c369a 100644
--- a/clang/test/CXX/module/dcl.dcl/dcl.module/dcl.module.interface/p1.cppm
+++ b/clang/test/CXX/module/dcl.dcl/dcl.module/dcl.module.interface/p1.cppm
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++20 %s -verify -o /dev/null
+// RUN: %clang_cc1 -std=c++20 %s -verify -emit-module-interface -o /dev/null
 // RUN: %clang_cc1 -std=c++20 %s -DINTERFACE -verify -emit-module-interface -o %t
 // RUN: %clang_cc1 -std=c++20 %s -DIMPLEMENTATION -verify -fmodule-file=A=%t -o /dev/null
 //
@@ -15,6 +15,8 @@ module A; // #module-decl
   // expected-error@-2 {{missing 'export' specifier in module declaration while building module interface}}
   #define INTERFACE
  #endif
+#else // Not in a module
+// expected-error@* {{missing 'export module' declaration in module interface unit}}
 #endif
 
 #ifndef INTERFACE
diff --git a/clang/test/Modules/pr101398.cppm b/clang/test/Modules/pr101398.cppm
new file mode 100644
index 00000000000000..843d0ce84fdce3
--- /dev/null
+++ b/clang/test/Modules/pr101398.cppm
@@ -0,0 +1,5 @@
+// RUN: mkdir -p %t
+// RUN: %clang -std=c++20 -xc++-module %s -Xclang -verify --precompile -o %t/tmp.pcm
+// not modules
+
+// expected-error@* {{missing 'export module' declaration in module interface unit}}

@ChuanqiXu9 ChuanqiXu9 merged commit be66c50 into llvm:main Aug 9, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:modules C++20 modules and Clang Header Modules clang Clang issues not falling into any other category skip-precommit-approval PR for CI feedback, not intended for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Assertion failure when compiling non-module as a module
2 participants