Skip to content

Commit

Permalink
[clang][modules] Deprecate module.map in favor of module.modulemap (#…
Browse files Browse the repository at this point in the history
…75142)

This patch deprecates `module.map` in favor of `module.modulemap`, which
has been the preferred form since 2014. The eventual goal is to remove
support for `module.map` to reduce the number of stats Clang needs to do
while searching for module map files.

This patch touches a lot of files, but the majority of them are just
renaming tests or references to the file in comments or documentation.

The relevant files are:
* lib/Lex/HeaderSearch.cpp
* include/clang/Basic/DiagnosticGroups.td
* include/clang/Basic/DiagnosticLexKinds.td
  • Loading branch information
Bigcheese committed Dec 14, 2023
1 parent f956bfe commit a171d24
Show file tree
Hide file tree
Showing 142 changed files with 255 additions and 222 deletions.
4 changes: 2 additions & 2 deletions clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1917,14 +1917,14 @@ TEST_F(SymbolCollectorTest, UndefOfModuleMacro) {
#undef X
)cpp";
TU.AdditionalFiles["foo.h"] = "#define X 1";
TU.AdditionalFiles["module.map"] = R"cpp(
TU.AdditionalFiles["module.modulemap"] = R"cpp(
module foo {
header "foo.h"
export *
}
)cpp";
TU.ExtraArgs.push_back("-fmodules");
TU.ExtraArgs.push_back("-fmodule-map-file=" + testPath("module.map"));
TU.ExtraArgs.push_back("-fmodule-map-file=" + testPath("module.modulemap"));
TU.OverlayRealFileSystemForModules = true;

TU.build();
Expand Down
2 changes: 1 addition & 1 deletion clang-tools-extra/docs/modularize.rst
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ map.
:program:`modularize` also has an assistant mode option for generating
a module map file based on the provided header list. The generated file
is a functional module map that can be used as a starting point for a
module.map file.
module.modulemap file.

Getting Started
===============
Expand Down
12 changes: 6 additions & 6 deletions clang-tools-extra/modularize/Modularize.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,20 +51,20 @@
// to the header list file directory. Use -prefix to specify a
// different directory.
// -module-map-path=(module map)
// Skip the checks, and instead act as a module.map generation
// Skip the checks, and instead act as a module.modulemap generation
// assistant, generating a module map file based on the header list.
// An optional "-root-module=(rootName)" argument can specify a root
// module to be created in the generated module.map file. Note that
// you will likely need to edit this file to suit the needs of your
// headers.
// module to be created in the generated module.modulemap file. Note
// that you will likely need to edit this file to suit the needs of
// your headers.
// -problem-files-list=(problem files list file name)
// For use only with module map assistant. Input list of files that
// have problems with respect to modules. These will still be
// included in the generated module map, but will be marked as
// "excluded" headers.
// -root-module=(root module name)
// Specifies a root module to be created in the generated module.map
// file.
// Specifies a root module to be created in the generated
// module.modulemap file.
// -block-check-header-list-only
// Only warn if #include directives are inside extern or namespace
// blocks if the included header is in the header list.
Expand Down
4 changes: 2 additions & 2 deletions clang-tools-extra/modularize/ModularizeUtilities.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -102,10 +102,10 @@ std::error_code ModularizeUtilities::loadAllHeaderListsAndDependencies() {

// Do coverage checks.
// For each loaded module map, do header coverage check.
// Starting from the directory of the module.map file,
// Starting from the directory of the module.modulemap file,
// Find all header files, optionally looking only at files
// covered by the include path options, and compare against
// the headers referenced by the module.map file.
// the headers referenced by the module.modulemap file.
// Display warnings for unaccounted-for header files.
// Returns 0 if there were no errors or warnings, 1 if there
// were warnings, 2 if any other problem, such as a bad
Expand Down
4 changes: 2 additions & 2 deletions clang-tools-extra/modularize/ModularizeUtilities.h
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,10 @@ class ModularizeUtilities {

/// Do coverage checks.
/// For each loaded module map, do header coverage check.
/// Starting from the directory of the module.map file,
/// Starting from the directory of the module.modulemap file,
/// Find all header files, optionally looking only at files
/// covered by the include path options, and compare against
/// the headers referenced by the module.map file.
/// the headers referenced by the module.modulemap file.
/// Display warnings for unaccounted-for header files.
/// \param IncludePaths The include paths to check for files.
/// (Note that other directories above these paths are ignored.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// module.map
// module.modulemap

module Level1A {
header "Level1A.h"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// module.map
// module.modulemap

module Level1A {
header "Includes1/Level1A.h"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// module.map
// module.modulemap

module Level1A {
header "Level1A.h"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// module.map
// module.modulemap

module Level1A {
header "Level1A.h"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// module.map
// module.modulemap

module Level1A {
header "Level1A.h"
Expand Down
6 changes: 3 additions & 3 deletions clang/include/clang-c/BuildSystem.h
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ CINDEX_LINKAGE void clang_free(void *buffer);
CINDEX_LINKAGE void clang_VirtualFileOverlay_dispose(CXVirtualFileOverlay);

/**
* Object encapsulating information about a module.map file.
* Object encapsulating information about a module.modulemap file.
*/
typedef struct CXModuleMapDescriptorImpl *CXModuleMapDescriptor;

Expand All @@ -109,15 +109,15 @@ CINDEX_LINKAGE CXModuleMapDescriptor
clang_ModuleMapDescriptor_create(unsigned options);

/**
* Sets the framework module name that the module.map describes.
* Sets the framework module name that the module.modulemap describes.
* \returns 0 for success, non-zero to indicate an error.
*/
CINDEX_LINKAGE enum CXErrorCode
clang_ModuleMapDescriptor_setFrameworkModuleName(CXModuleMapDescriptor,
const char *name);

/**
* Sets the umbrella header name that the module.map describes.
* Sets the umbrella header name that the module.modulemap describes.
* \returns 0 for success, non-zero to indicate an error.
*/
CINDEX_LINKAGE enum CXErrorCode
Expand Down
1 change: 1 addition & 0 deletions clang/include/clang/Basic/DiagnosticGroups.td
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ def : DiagGroup<"auto-import">;
def FrameworkHdrQuotedInclude : DiagGroup<"quoted-include-in-framework-header">;
def FrameworkIncludePrivateFromPublic :
DiagGroup<"framework-include-private-from-public">;
def DeprecatedModuleDotMap : DiagGroup<"deprecated-module-dot-map">;
def FrameworkHdrAtImport : DiagGroup<"atimport-in-framework-header">;
def CXX14BinaryLiteral : DiagGroup<"c++14-binary-literal">;
def CXXPre14CompatBinaryLiteral : DiagGroup<"c++98-c++11-compat-binary-literal">;
Expand Down
3 changes: 3 additions & 0 deletions clang/include/clang/Basic/DiagnosticLexKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -884,6 +884,9 @@ def warn_quoted_include_in_framework_header : Warning<
def warn_framework_include_private_from_public : Warning<
"public framework header includes private framework header '%0'"
>, InGroup<FrameworkIncludePrivateFromPublic>;
def warn_deprecated_module_dot_map : Warning<
"'%0' as a module map name is deprecated, rename it to %select{module.modulemap|module.private.modulemap}1%select{| in the 'Modules' directory of the framework}2">,
InGroup<DeprecatedModuleDotMap>;

def remark_pp_include_directive_modular_translation : Remark<
"treating #%select{include|import|include_next|__include_macros}0 as an "
Expand Down
22 changes: 17 additions & 5 deletions clang/lib/Lex/HeaderSearch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1656,7 +1656,8 @@ bool HeaderSearch::findUsableModuleForFrameworkHeader(
}

static OptionalFileEntryRef getPrivateModuleMap(FileEntryRef File,
FileManager &FileMgr) {
FileManager &FileMgr,
DiagnosticsEngine &Diags) {
StringRef Filename = llvm::sys::path::filename(File.getName());
SmallString<128> PrivateFilename(File.getDir().getName());
if (Filename == "module.map")
Expand All @@ -1665,7 +1666,14 @@ static OptionalFileEntryRef getPrivateModuleMap(FileEntryRef File,
llvm::sys::path::append(PrivateFilename, "module.private.modulemap");
else
return std::nullopt;
return FileMgr.getOptionalFileRef(PrivateFilename);
auto PMMFile = FileMgr.getOptionalFileRef(PrivateFilename);
if (PMMFile) {
if (Filename == "module.map")
Diags.Report(diag::warn_deprecated_module_dot_map)
<< PrivateFilename << 1
<< File.getDir().getName().endswith(".framework");
}
return PMMFile;
}

bool HeaderSearch::loadModuleMapFile(FileEntryRef File, bool IsSystem,
Expand Down Expand Up @@ -1731,7 +1739,8 @@ HeaderSearch::loadModuleMapFileImpl(FileEntryRef File, bool IsSystem,
}

// Try to load a corresponding private module map.
if (OptionalFileEntryRef PMMFile = getPrivateModuleMap(File, FileMgr)) {
if (OptionalFileEntryRef PMMFile =
getPrivateModuleMap(File, FileMgr, Diags)) {
if (ModMap.parseModuleMapFile(*PMMFile, IsSystem, Dir)) {
LoadedModuleMaps[File] = false;
return LMM_InvalidModuleMap;
Expand All @@ -1755,11 +1764,14 @@ HeaderSearch::lookupModuleMapFile(DirectoryEntryRef Dir, bool IsFramework) {
if (auto F = FileMgr.getOptionalFileRef(ModuleMapFileName))
return *F;

// Continue to allow module.map
// Continue to allow module.map, but warn it's deprecated.
ModuleMapFileName = Dir.getName();
llvm::sys::path::append(ModuleMapFileName, "module.map");
if (auto F = FileMgr.getOptionalFileRef(ModuleMapFileName))
if (auto F = FileMgr.getOptionalFileRef(ModuleMapFileName)) {
Diags.Report(diag::warn_deprecated_module_dot_map)
<< ModuleMapFileName << 0 << IsFramework;
return *F;
}

// For frameworks, allow to have a private module map with a preferred
// spelling when a public module map is absent.
Expand Down
File renamed without changes.
File renamed without changes.
File renamed without changes.
4 changes: 2 additions & 2 deletions clang/test/Index/Inputs/vfsoverlay.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
'roots': [
{ 'name': 'OUT_DIR', 'type': 'directory',
'contents': [
{ 'name': 'module.map', 'type': 'file',
'external-contents': 'INPUT_DIR/module.map'
{ 'name': 'module.modulemap', 'type': 'file',
'external-contents': 'INPUT_DIR/module.modulemap'
},
{ 'name': 'ModuleNeedsVFS.h', 'type': 'file',
'external-contents': 'INPUT_DIR/module_needs_vfs.h'
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
void will_be_found1(void);
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module module_map {
header "a.h"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module module_map.Private {
header "private.h"
}
Empty file.
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
framework module Module_Map_F {
header "a.h"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
explicit framework module Module_Map_F.Private {
header "private.h"
}
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
4 changes: 2 additions & 2 deletions clang/test/Modules/concept_differ.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@
// RUN: mkdir %t
// RUN: split-file %s %t
//
// RUN: %clang_cc1 -x c++ -std=c++20 -fmodules -fmodules-cache-path=%t -fmodule-map-file=%t/module.map %t/foo.cpp -verify
// RUN: %clang_cc1 -x c++ -std=c++20 -fmodules -fmodules-cache-path=%t -fmodule-map-file=%t/module.modulemap %t/foo.cpp -verify

//--- module.map
//--- module.modulemap
module "foo" {
export *
header "foo.h"
Expand Down
2 changes: 1 addition & 1 deletion clang/test/Modules/config_macros.m
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,6 @@
@import config; // expected-warning{{definition of configuration macro 'WANT_BAR' has no effect on the import of 'config'; pass '-DWANT_BAR=...' on the command line to configure the module}}

// RUN: rm -rf %t
// RUN: %clang_cc1 -std=c99 -fmodules -fimplicit-module-maps -x objective-c -fmodules-cache-path=%t -DWANT_FOO=1 -emit-module -fmodule-name=config %S/Inputs/module.map
// RUN: %clang_cc1 -std=c99 -fmodules -fimplicit-module-maps -x objective-c -fmodules-cache-path=%t -DWANT_FOO=1 -emit-module -fmodule-name=config %S/Inputs/module.modulemap
// RUN: %clang_cc1 -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs -DWANT_FOO=1 %s -verify

2 changes: 1 addition & 1 deletion clang/test/Modules/crash-vfs-ivfsoverlay.m
Original file line number Diff line number Diff line change
Expand Up @@ -41,5 +41,5 @@
// CHECKYAML: 'contents': [
// CHECKYAML-NEXT: {
// CHECKYAML-NEXT: 'type': 'file',
// CHECKYAML-NEXT: 'name': "module.map",
// CHECKYAML-NEXT: 'name': "module.modulemap",
// CHECKYAML-NEXT: 'external-contents': "/[[OTHERPATH:.*]]/actual_module2.map"
4 changes: 2 additions & 2 deletions clang/test/Modules/crash-vfs-path-symlink-component.m
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@
// CHECKYAML-NEXT: 'contents': [
// CHECKYAML-NEXT: {
// CHECKYAML-NEXT: 'type': 'file',
// CHECKYAML-NEXT: 'name': "module.map",
// CHECKYAML-NEXT: 'external-contents': "/[[PATH]]/i/usr/include/module.map"
// CHECKYAML-NEXT: 'name': "module.modulemap",
// CHECKYAML-NEXT: 'external-contents': "/[[PATH]]/i/usr/include/module.modulemap"
// CHECKYAML-NEXT: },

// Test that by using the previous generated YAML file clang is able to find the
Expand Down
4 changes: 2 additions & 2 deletions clang/test/Modules/crash-vfs-path-traversal.m
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@
// CHECKYAML-NEXT: 'contents': [
// CHECKYAML-NEXT: {
// CHECKYAML-NEXT: 'type': 'file',
// CHECKYAML-NEXT: 'name': "module.map",
// CHECKYAML-NEXT: 'external-contents': "/[[PATH]]/Inputs/crash-recovery/usr/include/module.map"
// CHECKYAML-NEXT: 'name': "module.modulemap",
// CHECKYAML-NEXT: 'external-contents': "/[[PATH]]/Inputs/crash-recovery/usr/include/module.modulemap"
// CHECKYAML-NEXT: },

// Replace the paths in the YAML files with relative ".." traversals
Expand Down
4 changes: 2 additions & 2 deletions clang/test/Modules/crash-vfs-relative-incdir.m
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@
// CHECKYAML-NEXT: 'contents': [
// CHECKYAML-NEXT: {
// CHECKYAML-NEXT: 'type': 'file',
// CHECKYAML-NEXT: 'name': "module.map",
// CHECKYAML-NEXT: 'name': "module.modulemap",
// CHECKYAML-NOT: 'external-contents': "{{[^ ]*}}.cache
// CHECKYAML-NEXT: 'external-contents': "/[[PATH]]/Inputs/crash-recovery/usr/include/module.map"
// CHECKYAML-NEXT: 'external-contents': "/[[PATH]]/Inputs/crash-recovery/usr/include/module.modulemap"
// CHECKYAML-NEXT: },

// Run the reproducer script - regular exit code is enough to test it works.
Expand Down
4 changes: 2 additions & 2 deletions clang/test/Modules/crash-vfs-relative-overlay.m
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,9 @@
// CHECKYAML-NEXT: 'contents': [
// CHECKYAML-NEXT: {
// CHECKYAML-NEXT: 'type': 'file',
// CHECKYAML-NEXT: 'name': "module.map",
// CHECKYAML-NEXT: 'name': "module.modulemap",
// CHECKYAML-NOT: 'external-contents': "{{[^ ]*}}.cache
// CHECKYAML-NEXT: 'external-contents': "/[[PATH]]/Inputs/crash-recovery/usr/include/module.map"
// CHECKYAML-NEXT: 'external-contents': "/[[PATH]]/Inputs/crash-recovery/usr/include/module.modulemap"
// CHECKYAML-NEXT: },

// Test that reading the YAML file will yield the correct path after
Expand Down
4 changes: 2 additions & 2 deletions clang/test/Modules/crash-vfs-run-reproducer.m
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@
// CHECKYAML-NEXT: 'contents': [
// CHECKYAML-NEXT: {
// CHECKYAML-NEXT: 'type': 'file',
// CHECKYAML-NEXT: 'name': "module.map",
// CHECKYAML-NEXT: 'name': "module.modulemap",
// CHECKYAML-NOT: 'external-contents': "{{[^ ]*}}.cache
// CHECKYAML-NEXT: 'external-contents': "/[[PATH]]/Inputs/crash-recovery/usr/include/module.map"
// CHECKYAML-NEXT: 'external-contents': "/[[PATH]]/Inputs/crash-recovery/usr/include/module.modulemap"
// CHECKYAML-NEXT: },

// Run the reproducer script - regular exit code is enough to test it works.
Expand Down
8 changes: 4 additions & 4 deletions clang/test/Modules/declare-use-compatible.cpp
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Used module not built with -decluse.
// RUN: rm -rf %t
// RUN: %clang_cc1 -x c++ -fmodules -fimplicit-module-maps -fmodule-name=XB -emit-module \
// RUN: -I %S/Inputs/declare-use %S/Inputs/declare-use/module.map -o %t/b.pcm
// RUN: -I %S/Inputs/declare-use %S/Inputs/declare-use/module.modulemap -o %t/b.pcm
// RUN: %clang_cc1 -x c++ -fmodules -fimplicit-module-maps -fmodules-cache-path=%t \
// RUN: -fmodules-decluse \
// RUN: -fmodule-file=%t/b.pcm -fmodule-name=XE -I %S/Inputs/declare-use %s
Expand All @@ -10,14 +10,14 @@
// RUN: rm -rf %t
// RUN: %clang_cc1 -x c++ -fmodules -fimplicit-module-maps -fmodule-name=XB -emit-module \
// RUN: -fmodules-decluse \
// RUN: -I %S/Inputs/declare-use %S/Inputs/declare-use/module.map -o %t/b.pcm
// RUN: -I %S/Inputs/declare-use %S/Inputs/declare-use/module.modulemap -o %t/b.pcm
// RUN: %clang_cc1 -x c++ -fmodules -fimplicit-module-maps -fmodules-cache-path=%t \
// RUN: -fmodule-file=%t/b.pcm -fmodule-name=XE -I %S/Inputs/declare-use %s
//
// Used module not built with -decluse.
// RUN: rm -rf %t
// RUN: %clang_cc1 -x c++ -fmodules -fimplicit-module-maps -fmodule-name=XB -emit-module \
// RUN: -I %S/Inputs/declare-use %S/Inputs/declare-use/module.map -o %t/b.pcm
// RUN: -I %S/Inputs/declare-use %S/Inputs/declare-use/module.modulemap -o %t/b.pcm
// RUN: %clang_cc1 -x c++ -fmodules -fimplicit-module-maps -fmodules-cache-path=%t \
// RUN: -fmodules-strict-decluse \
// RUN: -fmodule-file=%t/b.pcm -fmodule-name=XE -I %S/Inputs/declare-use %s
Expand All @@ -26,7 +26,7 @@
// RUN: rm -rf %t
// RUN: %clang_cc1 -x c++ -fmodules -fimplicit-module-maps -fmodule-name=XB -emit-module \
// RUN: -fmodules-strict-decluse \
// RUN: -I %S/Inputs/declare-use %S/Inputs/declare-use/module.map -o %t/b.pcm
// RUN: -I %S/Inputs/declare-use %S/Inputs/declare-use/module.modulemap -o %t/b.pcm
// RUN: %clang_cc1 -x c++ -fmodules -fimplicit-module-maps -fmodules-cache-path=%t \
// RUN: -fmodule-file=%t/b.pcm -fmodule-name=XE -I %S/Inputs/declare-use %s

Expand Down
2 changes: 1 addition & 1 deletion clang/test/Modules/dependency-gen-pch.m
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
// RUN: FileCheck %s < %t.d
// CHECK: dependency-gen-pch.m.o
// CHECK-NEXT: dependency-gen-pch.m
// CHECK-NEXT: Inputs{{.}}module.map
// CHECK-NEXT: Inputs{{.}}module.modulemap
// CHECK-NEXT: diamond_top.pcm
// CHECK-NEXT: Inputs{{.}}diamond_top.h

Expand Down
8 changes: 4 additions & 4 deletions clang/test/Modules/dependency-gen.m
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,18 @@
// RUN: %clang_cc1 -x objective-c -isystem %S/Inputs/System/usr/include -dependency-file %t.d.1 -MT %s.o -I %S/Inputs -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t-mcp %s
// RUN: FileCheck %s < %t.d.1
// CHECK: dependency-gen.m
// CHECK: Inputs{{.}}module.map
// CHECK: Inputs{{.}}module.modulemap
// CHECK: Inputs{{.}}diamond_top.h
// CHECK-NOT: usr{{.}}include{{.}}module.map
// CHECK-NOT: usr{{.}}include{{.}}module.modulemap
// CHECK-NOT: stdint.h


// RUN: %clang_cc1 -x objective-c -isystem %S/Inputs/System/usr/include -dependency-file %t.d.2 -MT %s.o -I %S/Inputs -sys-header-deps -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t-mcp %s
// RUN: FileCheck %s -check-prefix=CHECK-SYS < %t.d.2
// CHECK-SYS: dependency-gen.m
// CHECK-SYS: Inputs{{.}}module.map
// CHECK-SYS: Inputs{{.}}module.modulemap
// CHECK-SYS: Inputs{{.}}diamond_top.h
// CHECK-SYS: usr{{.}}include{{.}}module.map
// CHECK-SYS: usr{{.}}include{{.}}module.modulemap
// CHECK-SYS: stdint.h

#import "diamond_top.h"
Expand Down

0 comments on commit a171d24

Please sign in to comment.