[clang] [C++][Modules] Import declaration should in global module fragment, module interface or module implementation (PR #164106)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-modules

Author: None (yronglin)

<details>
<summary>Changes</summary>

This patch implement the restriction for C++ import declaration, it's can only appears in global module fragment, module interface or module implementation

[cpp.pre](https://eel.is/c++draft/cpp.pre):
```
module-file:
    pp-global-module-fragment[opt] pp-module group[opt] pp-private-module-fragment[opt]
```


Since this patch. the following code is ill-formed:

```cpp
// Need to add a 'module;' directive before import directive.
impot std; 
```

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


4 Files Affected:

- (modified) clang/include/clang/Basic/DiagnosticParseKinds.td (+3) 
- (modified) clang/lib/Parse/Parser.cpp (+7) 
- (modified) clang/lib/Sema/SemaModule.cpp (+1-1) 
- (added) clang/test/CXX/module/cpp.pre/p1.cpp (+10) 


``````````diff
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td
index e5e071f43fa75..e930d713b07f3 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1814,6 +1814,9 @@ def err_import_in_wrong_fragment : Error<
   "module%select{| partition}0 imports cannot be in the %select{global|private}1 module fragment">;
 
 def err_export_empty : Error<"export declaration cannot be empty">;
+def err_import_decl_not_in_module_fragment : Error<
+  "module import declaration can only appears in "
+  "global module fragment, module interface or module implementation">;
 }
 
 let CategoryName = "Generics Issue" in {
diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp
index ec01faf446e8d..ecddd93d197a4 100644
--- a/clang/lib/Parse/Parser.cpp
+++ b/clang/lib/Parse/Parser.cpp
@@ -2486,6 +2486,13 @@ Decl *Parser::ParseModuleImport(SourceLocation AtLoc,
     SeenError = false;
     break;
   case Sema::ModuleImportState::FirstDecl:
+    if (getLangOpts().CPlusPlusModules) {
+      Diag(ImportLoc, diag::err_import_decl_not_in_module_fragment);
+      Diag(ImportLoc, diag::note_global_module_introducer_missing)
+          << FixItHint::CreateInsertion(PP.getMainFileFirstPPTokenLoc(),
+                                        "module;");
+    }
+
     // If we found an import decl as the first declaration, we must be not in
     // a C++20 module unit or we are in an invalid state.
     ImportState = Sema::ModuleImportState::NotACXX20Module;
diff --git a/clang/lib/Sema/SemaModule.cpp b/clang/lib/Sema/SemaModule.cpp
index a2aa3eaaa7f6d..e525f64f9a7e9 100644
--- a/clang/lib/Sema/SemaModule.cpp
+++ b/clang/lib/Sema/SemaModule.cpp
@@ -342,7 +342,7 @@ Sema::ActOnModuleDecl(SourceLocation StartLoc, SourceLocation ModuleLoc,
     Diag(ModuleLoc, diag::err_module_decl_not_at_start);
     SourceLocation BeginLoc = PP.getMainFileFirstPPTokenLoc();
     Diag(BeginLoc, diag::note_global_module_introducer_missing)
-        << FixItHint::CreateInsertion(BeginLoc, "module;\n");
+        << FixItHint::CreateInsertion(BeginLoc, "module;");
   }
 
   // C++23 [module.unit]p1: ... The identifiers module and import shall not
diff --git a/clang/test/CXX/module/cpp.pre/p1.cpp b/clang/test/CXX/module/cpp.pre/p1.cpp
new file mode 100644
index 0000000000000..3ec7556734f65
--- /dev/null
+++ b/clang/test/CXX/module/cpp.pre/p1.cpp
@@ -0,0 +1,10 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -std=c++20 %t/import_is_first_decl.cpp -fsyntax-only -verify
+
+//--- import_is_first_decl.cpp
+import std; // expected-error {{module import declaration can only appears in global module fragment, module interface or module implementation}}
+// expected-note at -1 {{add 'module;' to the start of the file to introduce a global module fragment}}
+// expected-error at -2 {{module 'std' not found}}

``````````

</details>


https://github.com/llvm/llvm-project/pull/164106