diff options
author | Googler <noreply@google.com> | 2017-02-22 23:02:39 +0000 |
---|---|---|
committer | Yue Gan <yueg@google.com> | 2017-02-23 11:30:51 +0000 |
commit | 1d7db156291762b46550b9610ee1b1c73fde58ad (patch) | |
tree | 335b6a3e02cc6fc12482c6b71105c75bec162ba7 /src/main/java | |
parent | 8b2e2459c61c5a112cd8328e97c40da224e9c9ed (diff) |
Update to header thinning feature to create header_scanner actions based on compiler flags.
--
PiperOrigin-RevId: 148273368
MOS_MIGRATED_REVID=148273368
Diffstat (limited to 'src/main/java')
-rw-r--r-- | src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java | 92 | ||||
-rw-r--r-- | src/main/java/com/google/devtools/build/lib/rules/objc/LegacyCompilationSupport.java | 250 |
2 files changed, 174 insertions, 168 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java index ed19ace0c9..df7dc609f4 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java @@ -38,9 +38,11 @@ import com.google.common.base.Optional; import com.google.common.base.Preconditions; import com.google.common.base.Predicate; import com.google.common.base.Predicates; +import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; +import com.google.common.collect.ListMultimap; import com.google.common.collect.Lists; import com.google.common.collect.Sets; import com.google.devtools.build.lib.actions.ActionAnalysisMetadata; @@ -53,6 +55,7 @@ import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode; import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.actions.CustomCommandLine; import com.google.devtools.build.lib.analysis.actions.ParameterFileWriteAction; +import com.google.devtools.build.lib.analysis.actions.SpawnAction; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; @@ -1054,6 +1057,23 @@ public abstract class CompilationSupport { return parents.build(); } + /** Holds information about Objective-C compile actions that require header thinning. */ + protected static final class ObjcHeaderThinningInfo { + /** Source file for compile action. */ + public final Artifact sourceFile; + /** headers_list file for compile action. */ + public final Artifact headersListFile; + /** Command line arguments for compile action execution. */ + public final ImmutableList<String> arguments; + + public ObjcHeaderThinningInfo( + Artifact sourceFile, Artifact headersListFile, ImmutableList<String> arguments) { + this.sourceFile = Preconditions.checkNotNull(sourceFile); + this.headersListFile = Preconditions.checkNotNull(headersListFile); + this.arguments = Preconditions.checkNotNull(arguments); + } + } + /** * Returns true when ObjC header thinning is enabled via configuration and an a valid * header_scanner executable target is provided. @@ -1075,6 +1095,78 @@ public abstract class CompilationSupport { .getProvider(FilesToRunProvider.class); } + protected void registerHeaderScanningActions( + ImmutableList<ObjcHeaderThinningInfo> headerThinningInfo, + ObjcProvider objcProvider, + CompilationArtifacts compilationArtifacts) { + if (headerThinningInfo.isEmpty()) { + return; + } + + FilesToRunProvider headerScannerTool = getHeaderThinningToolExecutable(); + ListMultimap<ImmutableList<String>, ObjcHeaderThinningInfo> + objcHeaderThinningInfoByCommandLine = groupActionsByCommandLine(headerThinningInfo); + // Register a header scanning spawn action for each unique set of command line arguments + for (ImmutableList<String> args : objcHeaderThinningInfoByCommandLine.keySet()) { + SpawnAction.Builder builder = + new SpawnAction.Builder() + .setMnemonic("ObjcHeaderScanning") + .setExecutable(headerScannerTool); + CustomCommandLine.Builder cmdLine = CustomCommandLine.builder(); + for (ObjcHeaderThinningInfo info : objcHeaderThinningInfoByCommandLine.get(args)) { + cmdLine.addJoinPaths( + ":", + Lists.newArrayList(info.sourceFile.getExecPath(), info.headersListFile.getExecPath())); + builder.addInput(info.sourceFile).addOutput(info.headersListFile); + } + ruleContext.registerAction( + builder + .setCommandLine(cmdLine.add("--").add(args).build()) + .addInputs(compilationArtifacts.getPrivateHdrs()) + .addTransitiveInputs(attributes.hdrs()) + .addTransitiveInputs(objcProvider.get(ObjcProvider.HEADER)) + .addInputs(compilationArtifacts.getPchFile().asSet()) + .addTransitiveInputs(objcProvider.get(ObjcProvider.STATIC_FRAMEWORK_FILE)) + .addTransitiveInputs(objcProvider.get(ObjcProvider.DYNAMIC_FRAMEWORK_FILE)) + .build(ruleContext)); + } + } + + /** + * Groups {@link ObjcHeaderThinningInfo} objects based on the command line arguments of the + * ObjcCompile action. + * + * <p>Grouping by command line arguments allows {@link + * #registerHeaderScanningActions(ImmutableList, ObjcProvider, CompilationArtifacts)} to create a + * {@link SpawnAction} based on the compiler command line flags that may cause a difference in + * behaviour by the preprocessor. Some of the command line arguments must be filtered out as they + * change with every source {@link Artifact}; for example the object file (-o) and dotd filenames + * (-MF). These arguments are known not to change the preprocessor behaviour. + * + * @param headerThinningInfos information for compile actions that require header thinning + * @return values in {@code headerThinningInfos} grouped by compile action command line arguments + */ + private static ListMultimap<ImmutableList<String>, ObjcHeaderThinningInfo> + groupActionsByCommandLine(ImmutableList<ObjcHeaderThinningInfo> headerThinningInfos) { + // Maintain insertion order so that iteration in #registerHeaderScanningActions is deterministic + ListMultimap<ImmutableList<String>, ObjcHeaderThinningInfo> + objcHeaderThinningInfoByCommandLine = ArrayListMultimap.create(); + for (ObjcHeaderThinningInfo info : headerThinningInfos) { + ImmutableList.Builder<String> filteredArgumentsBuilder = ImmutableList.builder(); + List<String> arguments = info.arguments; + for (int i = 0; i < arguments.size(); ++i) { + String arg = arguments.get(i); + if (arg.equals("-MF") || arg.equals("-o") || arg.equals("-c")) { + ++i; + } else if (!arg.equals("-MD")) { + filteredArgumentsBuilder.add(arg); + } + } + objcHeaderThinningInfoByCommandLine.put(filteredArgumentsBuilder.build(), info); + } + return objcHeaderThinningInfoByCommandLine; + } + @Nullable private CcToolchainProvider maybeGetCcToolchain() { // TODO(rduan): Remove this check once all rules are using the crosstool support. diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/LegacyCompilationSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/LegacyCompilationSupport.java index d3c4805f8e..8a1c5778dd 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/LegacyCompilationSupport.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/LegacyCompilationSupport.java @@ -40,13 +40,10 @@ import com.google.common.base.Joiner; import com.google.common.base.Optional; import com.google.common.base.Predicates; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; -import com.google.common.collect.Lists; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact; -import com.google.devtools.build.lib.analysis.FilesToRunProvider; import com.google.devtools.build.lib.analysis.PrerequisiteArtifacts; import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode; import com.google.devtools.build.lib.analysis.RuleContext; @@ -76,7 +73,6 @@ import com.google.devtools.build.lib.util.Preconditions; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.PathFragment; import java.util.List; -import java.util.Map.Entry; import javax.annotation.Nullable; /** @@ -188,23 +184,8 @@ public class LegacyCompilationSupport extends CompilationSupport { ExtraCompileArgs extraCompileArgs, Iterable<PathFragment> priorityHeaders, Optional<CppModuleMap> moduleMap) { - ImmutableList.Builder<Artifact> objFiles = new ImmutableList.Builder<>(); - ImmutableMap<Artifact, Artifact> sourceFilesToHeadersListFiles = ImmutableMap.of(); - if (isHeaderThinningEnabled()) { - sourceFilesToHeadersListFiles = - generateHeadersListArtifactsForSources( - Iterables.concat( - compilationArtifacts.getSrcs(), compilationArtifacts.getNonArcSrcs())); - if (!sourceFilesToHeadersListFiles.isEmpty()) { - registerHeaderScanningAction( - sourceFilesToHeadersListFiles, - objcProvider, - compilationArtifacts, - priorityHeaders, - moduleMap, - extraCompileArgs); - } - } + ImmutableList.Builder<Artifact> objFiles = ImmutableList.builder(); + ImmutableList.Builder<ObjcHeaderThinningInfo> objcHeaderThinningInfos = ImmutableList.builder(); for (Artifact sourceFile : compilationArtifacts.getSrcs()) { Artifact objFile = intermediateArtifacts.objFile(sourceFile); @@ -220,15 +201,18 @@ public class LegacyCompilationSupport extends CompilationSupport { compilationArtifacts, Iterables.concat(extraCompileArgs, ImmutableList.of("-fobjc-arc"))); } else { - registerCompileAction( - sourceFile, - objFile, - objcProvider, - priorityHeaders, - moduleMap, - compilationArtifacts, - Iterables.concat(extraCompileArgs, ImmutableList.of("-fobjc-arc")), - sourceFilesToHeadersListFiles); + ObjcHeaderThinningInfo objcHeaderThinningInfo = + registerCompileAction( + sourceFile, + objFile, + objcProvider, + priorityHeaders, + moduleMap, + compilationArtifacts, + Iterables.concat(extraCompileArgs, ImmutableList.of("-fobjc-arc"))); + if (objcHeaderThinningInfo != null) { + objcHeaderThinningInfos.add(objcHeaderThinningInfo); + } } } for (Artifact nonArcSourceFile : compilationArtifacts.getNonArcSrcs()) { @@ -244,15 +228,18 @@ public class LegacyCompilationSupport extends CompilationSupport { compilationArtifacts, Iterables.concat(extraCompileArgs, ImmutableList.of("-fno-objc-arc"))); } else { - registerCompileAction( - nonArcSourceFile, - objFile, - objcProvider, - priorityHeaders, - moduleMap, - compilationArtifacts, - Iterables.concat(extraCompileArgs, ImmutableList.of("-fno-objc-arc")), - sourceFilesToHeadersListFiles); + ObjcHeaderThinningInfo objcHeaderThinningInfo = + registerCompileAction( + nonArcSourceFile, + objFile, + objcProvider, + priorityHeaders, + moduleMap, + compilationArtifacts, + Iterables.concat(extraCompileArgs, ImmutableList.of("-fno-objc-arc"))); + if (objcHeaderThinningInfo != null) { + objcHeaderThinningInfos.add(objcHeaderThinningInfo); + } } } @@ -261,32 +248,43 @@ public class LegacyCompilationSupport extends CompilationSupport { for (Artifact archive : compilationArtifacts.getArchive().asSet()) { registerArchiveActions(objFiles.build(), archive); } + + registerHeaderScanningActions( + objcHeaderThinningInfos.build(), objcProvider, compilationArtifacts); } - /** - * Configures a {@link CustomCommandLine.Builder} with common compilation arguments for building - * Objective-C sources. - * - * <p>The command line arguments generated by this method are common to both ObjcCompile and - * ObjcHeaderScanning actions. Arguments that are only necessary or useful when doing more than - * preprocessing (i.e. building an object file via ObjcCompile) are not specified here. - * - * @param commandLine existing {@link CustomCommandLine.Builder} to add common arguments to - * @return passed in {@code commandLine} with common arguments added - */ - private CustomCommandLine.Builder commonCompileActionCommandLine( - CustomCommandLine.Builder commandLine, + private CustomCommandLine compileActionCommandLine( + Artifact sourceFile, + Artifact objFile, ObjcProvider objcProvider, Iterable<PathFragment> priorityHeaders, Optional<CppModuleMap> moduleMap, Optional<Artifact> pchFile, + Optional<Artifact> dotdFile, Iterable<String> otherFlags, + boolean collectCodeCoverage, boolean isCPlusPlusSource) { + CustomCommandLine.Builder commandLine = new CustomCommandLine.Builder().add(CLANG); + if (isCPlusPlusSource) { commandLine.add("-stdlib=libc++"); commandLine.add("-std=gnu++11"); } + // The linker needs full debug symbol information to perform binary dead-code stripping. + if (objcConfiguration.shouldStripBinary()) { + commandLine.add("-g"); + } + + List<String> coverageFlags = ImmutableList.of(); + if (collectCodeCoverage) { + if (buildConfiguration.isLLVMCoverageMapFormatEnabled()) { + coverageFlags = CLANG_LLVM_COVERAGE_FLAGS; + } else { + coverageFlags = CLANG_GCOV_COVERAGE_FLAGS; + } + } + commandLine .add(compileFlagsForClang(appleConfiguration)) .add(commonLinkAndCompileFlagsForClang(objcProvider, objcConfiguration, appleConfiguration)) @@ -299,61 +297,9 @@ public class LegacyCompilationSupport extends CompilationSupport { .addBeforeEachPath("-isystem", objcProvider.get(INCLUDE_SYSTEM)) .add(otherFlags) .addFormatEach("-D%s", objcProvider.get(DEFINE)) + .add(coverageFlags) .add(getCompileRuleCopts()); - // Add module map arguments. - if (moduleMap.isPresent()) { - // If modules are enabled for the rule, -fmodules is added to the copts already. (This implies - // module map usage). Otherwise, we need to pass -fmodule-maps. - if (!attributes.enableModules()) { - commandLine.add("-fmodule-maps"); - } - // -fmodule-map-file only loads the module in Xcode 7, so we add the module maps's directory - // to the include path instead. - // TODO(bazel-team): Use -fmodule-map-file when Xcode 6 support is dropped. - commandLine - .add("-iquote") - .add(moduleMap.get().getArtifact().getExecPath().getParentDirectory().toString()) - .add("-fmodule-name=" + moduleMap.get().getName()); - } - - return commandLine; - } - - private CustomCommandLine compileActionCommandLine( - Artifact sourceFile, - Artifact objFile, - ObjcProvider objcProvider, - Iterable<PathFragment> priorityHeaders, - Optional<CppModuleMap> moduleMap, - Optional<Artifact> pchFile, - Optional<Artifact> dotdFile, - Iterable<String> otherFlags, - boolean collectCodeCoverage, - boolean isCPlusPlusSource) { - CustomCommandLine.Builder commandLine = - commonCompileActionCommandLine( - new CustomCommandLine.Builder().add(CLANG), - objcProvider, - priorityHeaders, - moduleMap, - pchFile, - otherFlags, - isCPlusPlusSource); - - // The linker needs full debug symbol information to perform binary dead-code stripping. - if (objcConfiguration.shouldStripBinary()) { - commandLine.add("-g"); - } - - if (collectCodeCoverage) { - if (buildConfiguration.isLLVMCoverageMapFormatEnabled()) { - commandLine.add(CLANG_LLVM_COVERAGE_FLAGS); - } else { - commandLine.add(CLANG_GCOV_COVERAGE_FLAGS); - } - } - // Add input source file arguments commandLine.add("-c"); if (!sourceFile.getExecPath().isAbsolute() @@ -393,18 +339,34 @@ public class LegacyCompilationSupport extends CompilationSupport { commandLine.add("-MD").addExecPath("-MF", dotdFile.get()); } + // Add module map arguments. + if (moduleMap.isPresent()) { + // If modules are enabled for the rule, -fmodules is added to the copts already. (This implies + // module map usage). Otherwise, we need to pass -fmodule-maps. + if (!attributes.enableModules()) { + commandLine.add("-fmodule-maps"); + } + // -fmodule-map-file only loads the module in Xcode 7, so we add the module maps's directory + // to the include path instead. + // TODO(bazel-team): Use -fmodule-map-file when Xcode 6 support is dropped. + commandLine + .add("-iquote") + .add(moduleMap.get().getArtifact().getExecPath().getParentDirectory().toString()) + .add("-fmodule-name=" + moduleMap.get().getName()); + } + return commandLine.build(); } - private void registerCompileAction( + @Nullable + private ObjcHeaderThinningInfo registerCompileAction( Artifact sourceFile, Artifact objFile, ObjcProvider objcProvider, Iterable<PathFragment> priorityHeaders, Optional<CppModuleMap> moduleMap, CompilationArtifacts compilationArtifacts, - Iterable<String> otherFlags, - ImmutableMap<Artifact, Artifact> sourceToOutput) { + Iterable<String> otherFlags) { boolean isCPlusPlusSource = ObjcRuleClasses.CPP_SOURCES.matches(sourceFile.getExecPath()); boolean runCodeCoverage = buildConfiguration.isCodeCoverageEnabled() && ObjcRuleClasses.isInstrumentable(sourceFile); @@ -446,9 +408,14 @@ public class LegacyCompilationSupport extends CompilationSupport { .addTransitiveMandatoryInputs(objcProvider.get(DYNAMIC_FRAMEWORK_FILE)) .setDotdFile(dotdFile) .addMandatoryInputs(compilationArtifacts.getPchFile().asSet()); - if (sourceToOutput.containsKey(sourceFile)) { - compileBuilder.setHeadersListFile(sourceToOutput.get(sourceFile)); + + Artifact headersListFile = null; + if (isHeaderThinningEnabled() + && SOURCES_FOR_HEADER_THINNING.matches(sourceFile.getFilename())) { + headersListFile = intermediateArtifacts.headersListFile(sourceFile); + compileBuilder.setHeadersListFile(headersListFile); } + ruleContext.registerAction( compileBuilder .setMnemonic("ObjcCompile") @@ -458,6 +425,11 @@ public class LegacyCompilationSupport extends CompilationSupport { .addOutputs(gcnoFile.asSet()) .addOutput(dotdFile.artifact()) .build(ruleContext)); + + return headersListFile == null + ? null + : new ObjcHeaderThinningInfo( + sourceFile, headersListFile, ImmutableList.copyOf(commandLine.arguments())); } /** @@ -523,64 +495,6 @@ public class LegacyCompilationSupport extends CompilationSupport { .build(ruleContext.getActionOwner())); } - private ImmutableMap<Artifact, Artifact> generateHeadersListArtifactsForSources( - Iterable<Artifact> sources) { - ImmutableMap.Builder<Artifact, Artifact> sourceFileToHeadersListBuilder = - ImmutableMap.builder(); - for (Artifact source : sources) { - // Tree artifacts are not currently supported - if (!source.isTreeArtifact() && SOURCES_FOR_HEADER_THINNING.matches(source.getFilename())) { - sourceFileToHeadersListBuilder.put(source, intermediateArtifacts.headersListFile(source)); - } - } - return sourceFileToHeadersListBuilder.build(); - } - - private void registerHeaderScanningAction( - ImmutableMap<Artifact, Artifact> sourceFilesToHeadersListFiles, - ObjcProvider objcProvider, - CompilationArtifacts compilationArtifacts, - Iterable<PathFragment> priorityHeaders, - Optional<CppModuleMap> moduleMap, - Iterable<String> otherFlags) { - FilesToRunProvider headerScannerExecutable = getHeaderThinningToolExecutable(); - - CustomCommandLine.Builder cmdLine = CustomCommandLine.builder(); - for (Entry<Artifact, Artifact> entry : sourceFilesToHeadersListFiles.entrySet()) { - cmdLine.addJoinPaths( - ":", Lists.newArrayList(entry.getKey().getExecPath(), entry.getValue().getExecPath())); - } - cmdLine.add("--"); - commonCompileActionCommandLine( - cmdLine, - objcProvider, - priorityHeaders, - moduleMap, - compilationArtifacts.getPchFile(), - otherFlags, - true); - - NestedSet<Artifact> moduleMapInputs = NestedSetBuilder.emptySet(Order.STABLE_ORDER); - if (objcConfiguration.moduleMapsEnabled()) { - moduleMapInputs = objcProvider.get(MODULE_MAP); - } - - ruleContext.registerAction( - new SpawnAction.Builder() - .setMnemonic("ObjcHeaderScanning") - .setExecutable(headerScannerExecutable) - .setCommandLine(cmdLine.build()) - .addInputs(sourceFilesToHeadersListFiles.keySet()) - .addInputs(compilationArtifacts.getPrivateHdrs()) - .addInputs(compilationArtifacts.getPchFile().asSet()) - .addTransitiveInputs(objcProvider.get(ObjcProvider.HEADER)) - .addTransitiveInputs(moduleMapInputs) - .addTransitiveInputs(objcProvider.get(ObjcProvider.STATIC_FRAMEWORK_FILE)) - .addTransitiveInputs(objcProvider.get(ObjcProvider.DYNAMIC_FRAMEWORK_FILE)) - .addOutputs(sourceFilesToHeadersListFiles.values()) - .build(ruleContext)); - } - private void registerArchiveActions(List<Artifact> objFiles, Artifact archive) { Artifact objList = intermediateArtifacts.archiveObjList(); registerObjFilelistAction(objFiles, objList); |