Skip to content

[C++20] [Modules] Warn for importing implementation partition unit in interface units #108493

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
Sep 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,8 @@ Improvements to Clang's diagnostics

- Clang now warns for u8 character literals used in C23 with ``-Wpre-c23-compat`` instead of ``-Wpre-c++17-compat``.

- Clang now diagnose when importing module implementation partition units in module interface units.

Improvements to Clang's time-trace
----------------------------------

Expand Down
4 changes: 4 additions & 0 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,10 @@ def warn_deprecated_literal_operator_id: Warning<
"is deprecated">, InGroup<DeprecatedLiteralOperator>, DefaultIgnore;
def warn_reserved_module_name : Warning<
"%0 is a reserved name for a module">, InGroup<ReservedModuleIdentifier>;
def warn_import_implementation_partition_unit_in_interface_unit : Warning<
"importing an implementation partition unit in a module interface is not recommended. "
"Names from %0 may not be reachable">,
InGroup<DiagGroup<"import-implementation-partition-unit-in-interface-unit">>;

def warn_parameter_size: Warning<
"%0 is a large (%1 bytes) pass-by-value argument; "
Expand Down
8 changes: 8 additions & 0 deletions clang/lib/Sema/SemaModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -650,6 +650,14 @@ DeclResult Sema::ActOnModuleImport(SourceLocation StartLoc,
else
VisibleModules.setVisible(Mod, ImportLoc);

assert((!Mod->isModulePartitionImplementation() || getCurrentModule()) &&
"We can only import a partition unit in a named module.");
if (Mod->isModulePartitionImplementation() &&
getCurrentModule()->isModuleInterfaceUnit())
Diag(ImportLoc,
diag::warn_import_implementation_partition_unit_in_interface_unit)
<< Mod->Name;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not for this PR, but at some point it might make sense to have an overload for operator<< so we can pass a const Module * directly and it automatically quotes the name, similar to how NamedDecl works.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. I'll try to make it next time


checkModuleImportContext(*this, Mod, ImportLoc, CurContext);

// FIXME: we should support importing a submodule within a different submodule
Expand Down
3 changes: 1 addition & 2 deletions clang/test/CXX/module/module.import/p2.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,8 @@ void test() {
}

//--- UseInPartA.cppm
// expected-no-diagnostics
export module M:partA;
import :impl;
import :impl; // expected-warning {{importing an implementation partition unit in a module interface is not recommended.}}
void test() {
A a;
}
Expand Down
1 change: 1 addition & 0 deletions clang/test/Modules/cxx20-10-3-ex1.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ module M:PartImpl;
export module M;
// error: exported partition :Part is an implementation unit
export import :PartImpl; // expected-error {{module partition implementations cannot be exported}}
// expected-warning@-1 {{importing an implementation partition unit in a module interface is not recommended.}}

//--- std10-3-ex1-tu3.cpp
export module M:Part;
Expand Down
Loading