Skip to content
This repository was archived by the owner on Nov 10, 2023. It is now read-only.

Commit 7b3de33

Browse files
rmazfacebook-github-bot
rmaz
authored and
facebook-github-bot
committed
add -fmodule-map-file when using explicit modules
Summary: When compiling with explicit modules we need to pass up the module map files of the dependent modules being compiled so that Clang knows which module to load for a corresponding header. We can then disable implicit module map loading. This fixes loading modules using the `-fmodule-file=name=path` syntax as well as avoiding issues with absolute paths being embedded in pcm files, as the command line overrides will take precedence over the serialized paths. Reviewed By: milend fbshipit-source-id: 38d82654b59df17a2e71ed6b0bee3fef25afc818
1 parent 211cbd0 commit 7b3de33

8 files changed

Lines changed: 94 additions & 36 deletions

File tree

‎src/com/facebook/buck/swift/SwiftCompileBase.java‎

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,9 @@
3636
import com.facebook.buck.core.sourcepath.resolver.SourcePathResolverAdapter;
3737
import com.facebook.buck.core.toolchain.tool.Tool;
3838
import com.facebook.buck.cxx.CxxDescriptionEnhancer;
39+
import com.facebook.buck.cxx.CxxLibraryDescription;
3940
import com.facebook.buck.cxx.PreprocessorFlags;
41+
import com.facebook.buck.cxx.toolchain.HeaderMode;
4042
import com.facebook.buck.cxx.toolchain.HeaderVisibility;
4143
import com.facebook.buck.cxx.toolchain.LinkerMapMode;
4244
import com.facebook.buck.cxx.toolchain.PathShortener;
@@ -315,7 +317,12 @@ public ImmutableList<String> constructCompilerArgs(SourcePathResolverAdapter res
315317

316318
if (usesExplicitModules) {
317319
argBuilder.add(
318-
frontendFlag, "-disable-implicit-swift-modules", "-Xcc", "-fno-implicit-modules");
320+
frontendFlag,
321+
"-disable-implicit-swift-modules",
322+
"-Xcc",
323+
"-fno-implicit-modules",
324+
"-Xcc",
325+
"-fno-implicit-module-maps");
319326

320327
Path swiftModuleMapPath =
321328
getProjectFilesystem().getRootPath().resolve(this.swiftModuleMapPath.get()).getPath();
@@ -328,7 +335,6 @@ public ImmutableList<String> constructCompilerArgs(SourcePathResolverAdapter res
328335

329336
// Import the swiftmodule and pcm output of SDK frameworks and Objc dependencies
330337
for (ExplicitModuleOutput module : moduleDeps) {
331-
String path = resolver.getIdeallyRelativePath(module.getOutputPath()).toString();
332338
if (module.getName().equals(moduleName)) {
333339
// We cannot import the direct path to the underlying module as it will conflict with
334340
// the exported module of the same name when debugging. Instead we need to import the
@@ -352,15 +358,28 @@ public ImmutableList<String> constructCompilerArgs(SourcePathResolverAdapter res
352358
break;
353359
}
354360
}
361+
String underlyingPcmPath =
362+
resolver.getIdeallyRelativePath(module.getOutputPath()).toString();
355363
argBuilder.add(
356364
"-Xcc",
357365
"-fmodule-file="
358-
+ path.replaceAll(AppleFlavors.SWIFT_UNDERLYING_MODULE_FLAVOR + ",", ""));
366+
+ moduleName
367+
+ "="
368+
+ underlyingPcmPath.replaceAll(
369+
AppleFlavors.SWIFT_UNDERLYING_MODULE_FLAVOR + ",", ""));
370+
371+
String underlyingModulemapPath = module.getModulemapPath().get().resolve(resolver);
372+
argBuilder.add(
373+
"-Xcc",
374+
"-fmodule-map-file="
375+
+ underlyingModulemapPath.replaceAll(
376+
AppleFlavors.SWIFT_UNDERLYING_MODULE_FLAVOR.toString(),
377+
HeaderMode.SYMLINK_TREE_WITH_MODULEMAP.getFlavor()
378+
+ ","
379+
+ CxxLibraryDescription.Type.EXPORTED_HEADERS.getFlavor()));
380+
359381
} else if (!module.getIsSwiftmodule()) {
360-
// TODO: this should use -fmodule-file=modulename=path syntax to avoid always loading
361-
// module files when they are not imported. Unfortunately this does not seem to work
362-
// correctly and the modules don't get loaded sometimes.
363-
argBuilder.add("-Xcc", "-fmodule-file=" + path);
382+
argBuilder.addAll(module.getClangArgs(resolver));
364383
}
365384
}
366385
} else {

‎src/com/facebook/buck/swift/SwiftInterfaceCompile.java‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import com.facebook.buck.rules.modern.OutputPath;
3232
import com.facebook.buck.rules.modern.OutputPathResolver;
3333
import com.facebook.buck.step.Step;
34+
import com.facebook.buck.swift.toolchain.ExplicitModuleInput;
3435
import com.facebook.buck.swift.toolchain.ExplicitModuleOutput;
3536
import com.google.common.collect.ImmutableList;
3637
import com.google.common.collect.ImmutableMap;
@@ -113,7 +114,9 @@ public ImmutableList<Step> getBuildSteps(
113114
"-disable-modules-validate-system-headers",
114115
"-suppress-warnings",
115116
"-Xcc",
116-
"-fno-implicit-modules");
117+
"-fno-implicit-modules",
118+
"-Xcc",
119+
"-fno-implicit-module-maps");
117120
argsBuilder.addAll(Arg.stringify(swiftArgs, resolver));
118121

119122
if (moduleName.equals("Swift") || moduleName.equals("SwiftOnoneSupport")) {
@@ -128,8 +131,7 @@ public ImmutableList<Step> getBuildSteps(
128131

129132
for (ExplicitModuleOutput dep : moduleDeps) {
130133
if (!dep.getIsSwiftmodule()) {
131-
Path modulePath = resolver.getIdeallyRelativePath(dep.getOutputPath());
132-
argsBuilder.add("-Xcc", "-fmodule-file=" + dep.getName() + "=" + modulePath);
134+
argsBuilder.addAll(dep.getClangArgs(resolver));
133135
}
134136
}
135137

‎src/com/facebook/buck/swift/SwiftLibraryDescription.java‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@
8181
import com.facebook.buck.rules.macros.LocationMacroExpander;
8282
import com.facebook.buck.rules.macros.StringWithMacros;
8383
import com.facebook.buck.rules.macros.StringWithMacrosConverter;
84+
import com.facebook.buck.swift.toolchain.ExplicitModuleInput;
8485
import com.facebook.buck.swift.toolchain.ExplicitModuleOutput;
8586
import com.facebook.buck.swift.toolchain.SwiftPlatform;
8687
import com.facebook.buck.swift.toolchain.SwiftPlatformsProvider;
@@ -694,8 +695,8 @@ private static ImmutableSet<ExplicitModuleOutput> getModuleDependencies(
694695
if (dep instanceof SwiftCompile) {
695696
SwiftCompile swiftCompile = (SwiftCompile) dep;
696697
depsBuilder.add(
697-
ExplicitModuleOutput.of(
698-
swiftCompile.getModuleName(), true, swiftCompile.getSwiftModuleOutputPath()));
698+
ExplicitModuleOutput.ofSwiftmodule(
699+
swiftCompile.getModuleName(), swiftCompile.getSwiftModuleOutputPath(), false));
699700
}
700701
}
701702

@@ -881,8 +882,11 @@ private static Iterable<ExplicitModuleOutput> createUnderlyingModulePcmCompileRu
881882
.getSourcePathToOutput()),
882883
clangModuleDependencies));
883884
return ImmutableList.of(
884-
ExplicitModuleOutput.of(
885-
moduleName, false, underlyingModuleCompileRule.getSourcePathToOutput()));
885+
ExplicitModuleOutput.ofClangModule(
886+
moduleName,
887+
ExplicitModuleInput.of(
888+
graphBuilder.requireRule(underlyingModuleCompileTarget).getSourcePathToOutput()),
889+
underlyingModuleCompileRule.getSourcePathToOutput()));
886890
}
887891

888892
private static Iterable<HeaderSymlinkTreeWithModuleMap> getModuleMapDepRules(
@@ -1057,7 +1061,8 @@ private static ExplicitModuleOutput createPcmCompile(
10571061
depsBuilder.build());
10581062
});
10591063

1060-
return ExplicitModuleOutput.of(moduleName, false, rule.getSourcePathToOutput());
1064+
return ExplicitModuleOutput.ofClangModule(
1065+
moduleName, moduleMapInput, rule.getSourcePathToOutput());
10611066
}
10621067

10631068
private static void collectFrameworkPcmDependencies(

‎src/com/facebook/buck/swift/SwiftModuleMapCompile.java‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@
3131
import com.facebook.buck.rules.modern.OutputPath;
3232
import com.facebook.buck.rules.modern.OutputPathResolver;
3333
import com.facebook.buck.step.Step;
34+
import com.facebook.buck.swift.toolchain.ExplicitModuleInput;
3435
import com.facebook.buck.swift.toolchain.ExplicitModuleOutput;
35-
import com.google.common.base.Preconditions;
3636
import com.google.common.collect.ImmutableList;
3737
import com.google.common.collect.ImmutableMap;
3838
import com.google.common.collect.ImmutableSet;
@@ -129,6 +129,8 @@ public ImmutableList<Step> getBuildSteps(
129129
"-disable-implicit-swift-modules",
130130
"-Xcc",
131131
"-fno-implicit-modules",
132+
"-Xcc",
133+
"-fno-implicit-module-maps",
132134
// Embed all input files into the PCM so we don't need to include module map files when
133135
// building remotely.
134136
// https://github.com/apple/llvm-project/commit/fb1e7f7d1aca7bcfc341e9214bda8b554f5ae9b6
@@ -155,11 +157,7 @@ public ImmutableList<Step> getBuildSteps(
155157
argsBuilder.add("-Xcc", "-I", "-Xcc", moduleMapPath.getParent().toString());
156158

157159
for (ExplicitModuleOutput dep : clangModuleDeps) {
158-
Preconditions.checkState(
159-
!dep.getIsSwiftmodule(),
160-
"Clang module compilation should not have a swiftmodule dependency");
161-
Path depPath = resolver.getIdeallyRelativePath(dep.getOutputPath());
162-
argsBuilder.add("-Xcc", "-fmodule-file=" + dep.getName() + "=" + depPath);
160+
argsBuilder.addAll(dep.getClangArgs(resolver));
163161
}
164162

165163
return ImmutableList.of(

‎src/com/facebook/buck/swift/SwiftSdkDependencies.java‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import com.facebook.buck.core.util.immutables.BuckStyleValue;
3232
import com.facebook.buck.io.filesystem.ProjectFilesystem;
3333
import com.facebook.buck.rules.args.Arg;
34+
import com.facebook.buck.swift.toolchain.ExplicitModuleInput;
3435
import com.facebook.buck.swift.toolchain.ExplicitModuleOutput;
3536
import com.facebook.buck.swift.toolchain.SwiftSdkDependenciesProvider;
3637
import com.fasterxml.jackson.annotation.JsonProperty;
@@ -295,8 +296,8 @@ private ImmutableSet<ExplicitModuleOutput> getSwiftmoduleDependencies(
295296

296297
ImmutableSet.Builder<ExplicitModuleOutput> depsBuilder = ImmutableSet.builder();
297298
depsBuilder.add(
298-
ExplicitModuleOutput.of(
299-
key.getName(), true, rule.getSourcePathToOutput(), module.isFramework()));
299+
ExplicitModuleOutput.ofSwiftmodule(
300+
key.getName(), rule.getSourcePathToOutput(), module.isFramework()));
300301

301302
// We need to collect the transitive dependencies at the specified target triple, which
302303
// is most likely different from this modules target.
@@ -345,6 +346,9 @@ private ImmutableSet<ExplicitModuleOutput> getClangModuleDependencies(
345346
.configure(UnconfiguredTargetConfiguration.INSTANCE)
346347
.withFlavors(clangFlavor);
347348

349+
ExplicitModuleInput moduleMapInput =
350+
replacePathPrefix(clangModule.getModulemapPath(), sdkPath, platformPath, swiftResourceDir);
351+
348352
BuildRule rule =
349353
graphBuilder.computeIfAbsent(
350354
buildTarget,
@@ -359,11 +363,12 @@ private ImmutableSet<ExplicitModuleOutput> getClangModuleDependencies(
359363
false,
360364
key.getName(),
361365
true,
362-
replacePathPrefix(
363-
clangModule.getModulemapPath(), sdkPath, platformPath, swiftResourceDir),
366+
moduleMapInput,
364367
depsBuilder.build()));
365368

366-
depsBuilder.add(ExplicitModuleOutput.of(key.getName(), false, rule.getSourcePathToOutput()));
369+
depsBuilder.add(
370+
ExplicitModuleOutput.ofClangModule(
371+
key.getName(), moduleMapInput, rule.getSourcePathToOutput()));
367372
return depsBuilder.build();
368373
}
369374

src/com/facebook/buck/swift/ExplicitModuleInput.java renamed to src/com/facebook/buck/swift/toolchain/ExplicitModuleInput.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
* limitations under the License.
1515
*/
1616

17-
package com.facebook.buck.swift;
17+
package com.facebook.buck.swift.toolchain;
1818

1919
import com.facebook.buck.core.rulekey.AddToRuleKey;
2020
import com.facebook.buck.core.rulekey.AddsToRuleKey;

‎src/com/facebook/buck/swift/toolchain/ExplicitModuleOutput.java‎

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,12 @@
1919
import com.facebook.buck.core.rulekey.AddToRuleKey;
2020
import com.facebook.buck.core.rulekey.AddsToRuleKey;
2121
import com.facebook.buck.core.sourcepath.SourcePath;
22+
import com.facebook.buck.core.sourcepath.resolver.SourcePathResolverAdapter;
2223
import com.facebook.buck.core.util.immutables.BuckStyleValue;
24+
import com.google.common.base.Preconditions;
25+
import com.google.common.collect.ImmutableList;
26+
import java.nio.file.Path;
27+
import java.util.Optional;
2328

2429
/**
2530
* A class that wraps the output from a module compilation rule. This is used to determine if we
@@ -35,6 +40,10 @@ public abstract class ExplicitModuleOutput implements AddsToRuleKey {
3540
@AddToRuleKey
3641
public abstract boolean getIsSwiftmodule();
3742

43+
/** The path to the rules modulemap if present. */
44+
@AddToRuleKey
45+
public abstract Optional<ExplicitModuleInput> getModulemapPath();
46+
3847
/** The path for this output. */
3948
@AddToRuleKey
4049
public abstract SourcePath getOutputPath();
@@ -43,12 +52,26 @@ public abstract class ExplicitModuleOutput implements AddsToRuleKey {
4352
@AddToRuleKey
4453
public abstract boolean getIsFramework();
4554

46-
public static ExplicitModuleOutput of(
47-
String name, boolean isSwiftModule, SourcePath outputPath, boolean isFramework) {
48-
return ImmutableExplicitModuleOutput.ofImpl(name, isSwiftModule, outputPath, isFramework);
55+
public Iterable<String> getClangArgs(SourcePathResolverAdapter resolver) {
56+
Preconditions.checkState(
57+
!getIsSwiftmodule(), "Trying to get clang args for a swiftmodule dependency");
58+
Path modulePath = resolver.getIdeallyRelativePath(getOutputPath());
59+
return ImmutableList.of(
60+
"-Xcc",
61+
"-fmodule-file=" + getName() + "=" + modulePath,
62+
"-Xcc",
63+
"-fmodule-map-file=" + getModulemapPath().get().resolve(resolver));
64+
}
65+
66+
public static ExplicitModuleOutput ofSwiftmodule(
67+
String name, SourcePath outputPath, boolean isFramework) {
68+
return ImmutableExplicitModuleOutput.ofImpl(
69+
name, true, Optional.empty(), outputPath, isFramework);
4970
}
5071

51-
public static ExplicitModuleOutput of(String name, boolean isSwiftModule, SourcePath outputPath) {
52-
return ImmutableExplicitModuleOutput.ofImpl(name, isSwiftModule, outputPath, false);
72+
public static ExplicitModuleOutput ofClangModule(
73+
String name, ExplicitModuleInput modulemapPath, SourcePath outputPath) {
74+
return ImmutableExplicitModuleOutput.ofImpl(
75+
name, false, Optional.of(modulemapPath), outputPath, false);
5376
}
5477
}

‎test/com/facebook/buck/swift/SwiftModuleMapFileStepTest.java‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,12 +64,14 @@ public void testSwiftModuleMapOutput() throws IOException {
6464
var outputFilePath = Optional.of(tempFile.toPath()).get();
6565

6666
ExplicitModuleOutput foo =
67-
ExplicitModuleOutput.of("Foo", true, FakeSourcePath.of("path/to/Foo.swiftmodule"));
67+
ExplicitModuleOutput.ofSwiftmodule(
68+
"Foo", FakeSourcePath.of("path/to/Foo.swiftmodule"), false);
6869
ExplicitModuleOutput bar =
69-
ExplicitModuleOutput.of("Bar", false, FakeSourcePath.of("path/to/Bar.swiftmodule"));
70+
ExplicitModuleOutput.ofSwiftmodule(
71+
"Bar", FakeSourcePath.of("path/to/Bar.swiftmodule"), false);
7072
ExplicitModuleOutput swiftUI =
71-
ExplicitModuleOutput.of(
72-
"SwiftUI", true, FakeSourcePath.of("path/to/SwiftUI.swiftmodule"), true);
73+
ExplicitModuleOutput.ofSwiftmodule(
74+
"SwiftUI", FakeSourcePath.of("path/to/SwiftUI.swiftmodule"), true);
7375

7476
ImmutableSet<ExplicitModuleOutput> moduleDeps = ImmutableSet.of(foo, bar, swiftUI);
7577

@@ -88,6 +90,10 @@ public void testSwiftModuleMapOutput() throws IOException {
8890
+ " \"modulePath\" : \"path/to/Foo.swiftmodule\",\n"
8991
+ " \"isFramework\" : false\n"
9092
+ "}, {\n"
93+
+ " \"moduleName\" : \"Bar\",\n"
94+
+ " \"modulePath\" : \"path/to/Bar.swiftmodule\",\n"
95+
+ " \"isFramework\" : false\n"
96+
+ "}, {\n"
9197
+ " \"moduleName\" : \"SwiftUI\",\n"
9298
+ " \"modulePath\" : \"path/to/SwiftUI.swiftmodule\",\n"
9399
+ " \"isFramework\" : true\n"

0 commit comments

Comments
 (0)