Skip to content

Commit d1ee37c

Browse files
committed
Retain module map names as Labels
With the exception of special module map names, all of them are effectively labels. This commit ensures that they are retained as `Label`s, not label string and thus: * memory usage is reduced for module maps from external repos, whose module map name previously resulted in a newly retained string; * module map names now use properly formatted external repo labels as names, which makes it more obvious what they are; * the roundtrip in header module compilation flows is avoided. This actually fixes a bug where the missing `@@` prefix for label strings resulted in incorrect or even failing `Label` calls due to main repo labels being resolved relative to the rules_cc repo and external repo labels failing the validation of the `Label` constructor.
1 parent 02835b3 commit d1ee37c

File tree

6 files changed

+27
-17
lines changed

6 files changed

+27
-17
lines changed

cc/private/cc_info.bzl

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -148,22 +148,22 @@ _ModuleMapInfo = provider(
148148
"ModuleMapInfo",
149149
fields = {
150150
"file": "The module map file.",
151-
"name": "The name of the module.",
151+
"identifier": "The identifier of the module, either a label or a string.",
152152
},
153153
)
154154

155-
def create_module_map(*, file, name):
155+
def create_module_map(*, file, identifier):
156156
"""
157157
Creates a module map struct.
158158
159159
Args:
160160
file: The module map file.
161-
name: The name of the module.
161+
identifier: The identifier of the module.
162162
Returns:
163163
A module map struct.
164164
"""
165165
check_private_api()
166-
return _ModuleMapInfo(file = file, name = name)
166+
return _ModuleMapInfo(file = file, identifier = identifier)
167167

168168
def create_separate_module_map(module_map):
169169
"""
@@ -174,7 +174,11 @@ def create_separate_module_map(module_map):
174174
Returns:
175175
A module map struct.
176176
"""
177-
return _ModuleMapInfo(file = module_map.file, name = module_map.name + ".sep")
177+
if type(module_map.identifier) == type(""):
178+
identifier = module_map.identifier + ".sep"
179+
else:
180+
identifier = module_map.identifier.same_package_label(module_map.identifier.name + ".sep")
181+
return _ModuleMapInfo(file = module_map.file, identifier = identifier)
178182

179183
def create_linking_context(
180184
*,

cc/private/compile/cc_compilation_helper.bzl

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -212,10 +212,16 @@ _ModuleMapInfo = provider(
212212
],
213213
)
214214

215+
def _stringify_module_map_identifier(label_or_string):
216+
unambiguous_canonical_label_string = str(label_or_string)
217+
if unambiguous_canonical_label_string.startswith("@@"):
218+
return unambiguous_canonical_label_string[2:]
219+
return unambiguous_canonical_label_string
220+
215221
def _module_map_struct_to_module_map_content(parameters, tree_expander):
216222
lines = []
217223
module_map = parameters.module_map
218-
lines.append("module \"%s\" {" % module_map.name)
224+
lines.append("module \"%s\" {" % _stringify_module_map_identifier(module_map.identifier))
219225
lines.append(" export *")
220226

221227
def expanded(artifacts):
@@ -281,10 +287,10 @@ def _module_map_struct_to_module_map_content(parameters, tree_expander):
281287

282288
dependency_module_maps = parameters.dependency_module_maps.to_list()
283289
for dep in dependency_module_maps:
284-
lines.append(" use \"" + dep.name + "\"")
290+
lines.append(" use \"" + _stringify_module_map_identifier(dep.identifier) + "\"")
285291

286292
if parameters.separate_module_headers:
287-
separate_name = module_map.name + ".sep"
293+
separate_name = _stringify_module_map_identifier(module_map.identifier) + ".sep"
288294
lines.append(" use \"" + separate_name + "\"")
289295
lines.append("}")
290296
lines.append("module \"" + separate_name + "\" {")
@@ -298,14 +304,14 @@ def _module_map_struct_to_module_map_content(parameters, tree_expander):
298304
added_paths.add(header.path)
299305

300306
for dep in dependency_module_maps:
301-
lines.append(" use \"" + dep.name + "\"")
307+
lines.append(" use \"" + _stringify_module_map_identifier(dep.identifier) + "\"")
302308

303309
lines.append("}")
304310

305311
if parameters.extern_dependencies:
306312
for dep in dependency_module_maps:
307313
lines.append(
308-
"extern module \"" + dep.name + "\" \"" +
314+
"extern module \"" + _stringify_module_map_identifier(dep.identifier) + "\" \"" +
309315
parameters.leading_periods + dep.file.path + "\"",
310316
)
311317

@@ -508,7 +514,7 @@ def _init_cc_compilation_context(
508514
if not module_map:
509515
module_map = create_module_map(
510516
file = actions.declare_file(label.name + ".cppmap"),
511-
name = label.workspace_name + "//" + label.package + ":" + label.name,
517+
identifier = label,
512518
)
513519

514520
# There are different modes for module compilation:

cc/private/compile/compile.bzl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1149,7 +1149,7 @@ def _create_cc_compile_actions(
11491149

11501150
if _should_provide_header_modules(feature_configuration, private_headers, public_headers):
11511151
cpp_module_map = cc_compilation_context._module_map
1152-
module_map_label = Label(cpp_module_map.name)
1152+
module_map_label = Label(cpp_module_map.identifier)
11531153
modules = _create_module_action(
11541154
action_construction_context = action_construction_context,
11551155
cc_compilation_context = cc_compilation_context,
@@ -2106,7 +2106,7 @@ def _create_module_action(
21062106
additional_compilation_inputs,
21072107
additional_include_scanning_roots,
21082108
outputs):
2109-
module_map_label = Label(cpp_module_map.name)
2109+
module_map_label = Label(cpp_module_map.identifier)
21102110
return _create_pic_nopic_compile_source_actions(
21112111
action_construction_context = action_construction_context,
21122112
cc_compilation_context = cc_compilation_context,

cc/private/compile/compile_build_variables.bzl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,7 @@ def get_specific_compile_build_variables(
324324
result = {}
325325

326326
if feature_configuration.is_enabled("module_maps") and cpp_module_map:
327-
result[_VARS.MODULE_NAME] = cpp_module_map.name
327+
result[_VARS.MODULE_NAME] = str(cpp_module_map.identifier)
328328
result[_VARS.MODULE_MAP_FILE] = cpp_module_map.file
329329
result[_VARS.DEPENDENT_MODULE_MAP_FILES] = direct_module_maps
330330

cc/private/rules_impl/cc_toolchain_provider_helper.bzl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,7 @@ def get_cc_toolchain_provider(ctx, attributes):
230230

231231
module_map = None
232232
if attributes.module_map != None and attributes.module_map_artifact != None:
233-
module_map = cc_common.create_module_map(file = attributes.module_map_artifact, name = "crosstool")
233+
module_map = cc_common.create_module_map(file = attributes.module_map_artifact, identifier = "crosstool")
234234

235235
cc_compilation_context = cc_common.create_compilation_context(module_map = module_map)
236236

cc/private/rules_impl/objc_intermediate_artifacts.bzl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,13 +54,13 @@ def _swift_module_map(ctx):
5454
ctx,
5555
".modulemaps/module.modulemap",
5656
),
57-
name = module_name,
57+
identifier = module_name,
5858
)
5959

6060
def _internal_module_map(ctx):
6161
return cc_common.create_module_map(
6262
file = _declare_file_with_extension(ctx, ".internal.cppmap"),
63-
name = str(ctx.label),
63+
identifier = ctx.label,
6464
)
6565

6666
def _create_closure_struct(ctx, archive_file_name_suffix, enforce_always_link):

0 commit comments

Comments
 (0)