Skip to content
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

[Modules] textual headers in submodules never resolve their uses #69651

Merged
merged 1 commit into from
Oct 20, 2023

Conversation

ian-twilightcoder
Copy link
Contributor

When an include from a textual header is resolved, the textual header's submodule is used as the requesting module. The submodule's uses are resolved, but that doesn't work because only top level modules have uses, and only the top level module uses are used for checking uses in Module::directlyUses. ModuleMap::resolveUses to resolve the top level module instead of the submodule.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:modules C++20 modules and Clang Header Modules labels Oct 19, 2023
@ian-twilightcoder
Copy link
Contributor Author

Somewhat indirectly caused by https://reviews.llvm.org/D132779

@llvmbot
Copy link
Member

llvmbot commented Oct 19, 2023

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-modules

Author: Ian Anderson (ian-twilightcoder)

Changes

When an include from a textual header is resolved, the textual header's submodule is used as the requesting module. The submodule's uses are resolved, but that doesn't work because only top level modules have uses, and only the top level module uses are used for checking uses in Module::directlyUses. ModuleMap::resolveUses to resolve the top level module instead of the submodule.


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

5 Files Affected:

  • (modified) clang/lib/Lex/ModuleMap.cpp (+7-6)
  • (added) clang/test/Modules/Inputs/no-undeclared-includes/assert.h (+1)
  • (added) clang/test/Modules/Inputs/no-undeclared-includes/base.h (+6)
  • (added) clang/test/Modules/Inputs/no-undeclared-includes/module.modulemap (+11)
  • (added) clang/test/Modules/no-undeclared-includes.c (+5)
diff --git a/clang/lib/Lex/ModuleMap.cpp b/clang/lib/Lex/ModuleMap.cpp
index f65a5f145c04395..259c97796ae19f2 100644
--- a/clang/lib/Lex/ModuleMap.cpp
+++ b/clang/lib/Lex/ModuleMap.cpp
@@ -1398,16 +1398,17 @@ bool ModuleMap::resolveExports(Module *Mod, bool Complain) {
 }
 
 bool ModuleMap::resolveUses(Module *Mod, bool Complain) {
-  auto Unresolved = std::move(Mod->UnresolvedDirectUses);
-  Mod->UnresolvedDirectUses.clear();
+  auto *Top = Mod->getTopLevelModule();
+  auto Unresolved = std::move(Top->UnresolvedDirectUses);
+  Top->UnresolvedDirectUses.clear();
   for (auto &UDU : Unresolved) {
-    Module *DirectUse = resolveModuleId(UDU, Mod, Complain);
+    Module *DirectUse = resolveModuleId(UDU, Top, Complain);
     if (DirectUse)
-      Mod->DirectUses.push_back(DirectUse);
+      Top->DirectUses.push_back(DirectUse);
     else
-      Mod->UnresolvedDirectUses.push_back(UDU);
+      Top->UnresolvedDirectUses.push_back(UDU);
   }
-  return !Mod->UnresolvedDirectUses.empty();
+  return !Top->UnresolvedDirectUses.empty();
 }
 
 bool ModuleMap::resolveConflicts(Module *Mod, bool Complain) {
diff --git a/clang/test/Modules/Inputs/no-undeclared-includes/assert.h b/clang/test/Modules/Inputs/no-undeclared-includes/assert.h
new file mode 100644
index 000000000000000..6ca1ffb2ad7ea6e
--- /dev/null
+++ b/clang/test/Modules/Inputs/no-undeclared-includes/assert.h
@@ -0,0 +1 @@
+#include <base.h>
diff --git a/clang/test/Modules/Inputs/no-undeclared-includes/base.h b/clang/test/Modules/Inputs/no-undeclared-includes/base.h
new file mode 100644
index 000000000000000..e559063bef43311
--- /dev/null
+++ b/clang/test/Modules/Inputs/no-undeclared-includes/base.h
@@ -0,0 +1,6 @@
+#ifndef base_h
+#define base_h
+
+
+
+#endif /* base_h */
diff --git a/clang/test/Modules/Inputs/no-undeclared-includes/module.modulemap b/clang/test/Modules/Inputs/no-undeclared-includes/module.modulemap
new file mode 100644
index 000000000000000..23d04305b3becbd
--- /dev/null
+++ b/clang/test/Modules/Inputs/no-undeclared-includes/module.modulemap
@@ -0,0 +1,11 @@
+module cstd [system] [no_undeclared_includes] {
+  use base
+  module assert {
+    textual header "assert.h"
+  }
+}
+
+module base [system] {
+  header "base.h"
+  export *
+}
diff --git a/clang/test/Modules/no-undeclared-includes.c b/clang/test/Modules/no-undeclared-includes.c
new file mode 100644
index 000000000000000..613418ef0041328
--- /dev/null
+++ b/clang/test/Modules/no-undeclared-includes.c
@@ -0,0 +1,5 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -I %S/Inputs/no-undeclared-includes %s -verify
+
+// expected-no-diagnostics
+#include <assert.h>

When an include from a textual header is resolved, the textual header's submodule is used as the requesting module. The submodule's uses are resolved, but that doesn't work because only top level modules have uses, and only the top level module uses are used for checking uses in Module::directlyUses. ModuleMap::resolveUses to resolve the top level module instead of the submodule.
Copy link
Contributor

@Bigcheese Bigcheese left a comment

Choose a reason for hiding this comment

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

LGTM. The rest of clang reasons about this at the top level module, so so should this.

@ian-twilightcoder ian-twilightcoder merged commit 09ec000 into llvm:main Oct 20, 2023
2 checks passed
@ian-twilightcoder ian-twilightcoder deleted the fix-uses branch October 20, 2023 20:23
ian-twilightcoder pushed a commit to ian-twilightcoder/llvm-project that referenced this pull request Oct 22, 2023
abdulowork pushed a commit to abdulowork/llvm-project that referenced this pull request Jul 12, 2024
…lvm#69651)

When an include from a textual header is resolved, the textual header's
submodule is used as the requesting module. The submodule's uses are
resolved, but that doesn't work because only top level modules have
uses, and only the top level module uses are used for checking uses in
Module::directlyUses. ModuleMap::resolveUses to resolve the top level
module instead of the submodule.
SvyatoslavScherbina pushed a commit to Kotlin/llvm-project that referenced this pull request Jul 15, 2024
…lvm#69651)

When an include from a textual header is resolved, the textual header's
submodule is used as the requesting module. The submodule's uses are
resolved, but that doesn't work because only top level modules have
uses, and only the top level module uses are used for checking uses in
Module::directlyUses. ModuleMap::resolveUses to resolve the top level
module instead of the submodule.
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants