Skip to content

Commit

Permalink
Add a fast path for validating include directives. Most -isystem dire…
Browse files Browse the repository at this point in the history
…ctives are

actually added for cc_library's include-attribute. For these, the same path is
present on the command line, but also ignored. For N of those directives, we
currently do O(N^2) prefix checks, which is unnecessarily expensive.

RELNOTES: None.
PiperOrigin-RevId: 196477307
  • Loading branch information
Googler authored and Copybara-Service committed May 14, 2018
1 parent 7fae58d commit 43181c9
Showing 1 changed file with 7 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
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.devtools.build.lib.actions.ActionOwner;
import com.google.devtools.build.lib.actions.Artifact;
Expand Down Expand Up @@ -481,7 +482,7 @@ private boolean shouldPruneModules() {
}

private void verifyActionIncludePaths(CppCompileAction action, Consumer<String> errorReporter) {
Iterable<PathFragment> ignoredDirs = action.getValidationIgnoredDirs();
ImmutableSet<PathFragment> ignoredDirs = ImmutableSet.copyOf(action.getValidationIgnoredDirs());
// We currently do not check the output of:
// - getQuoteIncludeDirs(): those only come from includes attributes, and are checked in
// CcCommon.getIncludeDirsFromIncludesAttribute().
Expand All @@ -491,7 +492,11 @@ private void verifyActionIncludePaths(CppCompileAction action, Consumer<String>
Iterable<PathFragment> includePathsToVerify =
Iterables.concat(action.getIncludeDirs(), action.getSystemIncludeDirs());
for (PathFragment includePath : includePathsToVerify) {
if (FileSystemUtils.startsWithAny(includePath, ignoredDirs)) {
// includePathsToVerify contains all paths that are added as -isystem directive on the command
// line, most of which are added for include directives in the CcCompilationContext and are
// thus also in ignoredDirs. The hash lookup prevents this from becoming O(N^2) for these.
if (ignoredDirs.contains(includePath)
|| FileSystemUtils.startsWithAny(includePath, ignoredDirs)) {
continue;
}
// One starting ../ is okay for getting to a sibling repository.
Expand Down

0 comments on commit 43181c9

Please sign in to comment.