[C++][Modules] Import declaration should in global module fragment, module interface or module implementation #164106
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…odule interface or module implementation Signed-off-by: yronglin <yronglin777@gmail.com>
yronglin
requested review from
AaronBallman,
Bigcheese,
ChuanqiXu9,
cor3ntin and
hubert-reinterpretcast
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-modules Author: None (yronglin) ChangesThis patch implement the restriction for C++ import declaration, it's can only appears in global module fragment, module interface or module implementation Since this patch. the following code is ill-formed: // 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:
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@-1 {{add 'module;' to the start of the file to introduce a global module fragment}}
+// expected-error@-2 {{module 'std' not found}}
|
|
@llvm/pr-subscribers-clang Author: None (yronglin) ChangesThis patch implement the restriction for C++ import declaration, it's can only appears in global module fragment, module interface or module implementation Since this patch. the following code is ill-formed: // 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:
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@-1 {{add 'module;' to the start of the file to introduce a global module fragment}}
+// expected-error@-2 {{module 'std' not found}}
|
There was a problem hiding this comment.
I am unaware of any such restriction.
An pp-import is a control-line, which is in turn a group-part.
A preprocessing-file can directly contain a group without being a module-file.
There is a restriction on pp-global-module-fragments: at the start of phase 4, they can contain neither a pp-import nor a text-line.
Thanks, got it! I misunderstand before. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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:
Since this patch. the following code is ill-formed:
// Need to add a 'module;' directive before import directive. impot std;