Fix usages of PathFragment segments that will become inefficient.

An upcoming replacement to PathFragment will not have efficient segment semantics, causing code to become unnecessarily inefficient.

RELNOTES: None
PiperOrigin-RevId: 182553098

6 files changed, 31 insertions, 27 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java
index be9bbf157a..6c8e0401e4 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java
@@ -324,30 +324,32 @@ public class AndroidCommon {
     if (needle.equals(PathFragment.EMPTY_FRAGMENT)) {
       return haystack;
     }
+    List<String> needleSegments = needle.getSegments();
     // Compute the overlap offset for duplicated parts of the needle.
-    int[] overlap = new int[needle.segmentCount() + 1];
+    int[] overlap = new int[needleSegments.size() + 1];
     // Start overlap at -1, as it will cancel out the increment in the search.
     // See http://en.wikipedia.org/wiki/Knuth%E2%80%93Morris%E2%80%93Pratt_algorithm for the
     // details.
     overlap[0] = -1;
-    for (int i = 0, j = -1; i < needle.segmentCount(); j++, i++, overlap[i] = j) {
-      while (j >= 0 && !needle.getSegment(i).equals(needle.getSegment(j))) {
+    for (int i = 0, j = -1; i < needleSegments.size(); j++, i++, overlap[i] = j) {
+      while (j >= 0 && !needleSegments.get(i).equals(needleSegments.get(j))) {
         // Walk the overlap until the bound is found.
         j = overlap[j];
       }
     }
     // TODO(corysmith): reverse the search algorithm.
     // Keep the index of the found so that the rightmost index is taken.
+    List<String> haystackSegments = haystack.getSegments();
     int found = -1;
-    for (int i = 0, j = 0; i < haystack.segmentCount(); i++) {
+    for (int i = 0, j = 0; i < haystackSegments.size(); i++) {
 
-      while (j >= 0 && !haystack.getSegment(i).equals(needle.getSegment(j))) {
+      while (j >= 0 && !haystackSegments.get(i).equals(needleSegments.get(j))) {
         // Not matching, walk the needle index to attempt another match.
         j = overlap[j];
       }
       j++;
       // Needle index is exhausted, so the needle must match.
-      if (j == needle.segmentCount()) {
+      if (j == needleSegments.size()) {
         // Record the found index + 1 to be inclusive of the end index.
         found = i + 1;
         // Subtract one from the needle index to restart the search process
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/LocalResourceContainer.java b/src/main/java/com/google/devtools/build/lib/rules/android/LocalResourceContainer.java
index f11cd1835a..c754d9e337 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/LocalResourceContainer.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/LocalResourceContainer.java
@@ -32,6 +32,7 @@ import com.google.devtools.build.lib.rules.android.ResourceContainer.ResourceTyp
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.util.Arrays;
 import java.util.LinkedHashSet;
+import java.util.List;
 import java.util.Optional;
 import java.util.Set;
 import javax.annotation.Nullable;
@@ -142,7 +143,8 @@ public final class LocalResourceContainer {
         PathFragment packageRelativePath = file.getRootRelativePath().relativeTo(packageFragment);
         if (packageRelativePath.startsWith(assetsDir)) {
           PathFragment relativePath = packageRelativePath.relativeTo(assetsDir);
-          assetRoots.add(trimTail(file.getExecPath(), relativePath));
+          PathFragment path = file.getExecPath();
+          assetRoots.add(path.subFragment(0, path.segmentCount() - relativePath.segmentCount()));
         } else {
           ruleContext.attributeError(
               ResourceType.ASSETS.getAttribute(),
@@ -273,8 +275,11 @@ public final class LocalResourceContainer {
         file.getArtifactOwner().getLabel().getPackageIdentifier().getSourceRoot();
     PathFragment packageRelativePath = file.getRootRelativePath().relativeTo(packageFragment);
     try {
+      PathFragment path = file.getExecPath();
       resourceRoots.add(
-          trimTail(file.getExecPath(), makeRelativeTo(resourceDir, packageRelativePath)));
+          path.subFragment(
+              0,
+              path.segmentCount() - segmentCountAfterAncestor(resourceDir, packageRelativePath)));
     } catch (IllegalArgumentException e) {
       ruleErrorConsumer.attributeError(
           resourcesAttr,
@@ -304,7 +309,7 @@ public final class LocalResourceContainer {
     }
     // TODO(bazel-team): Expand Fileset to verify, or remove Fileset as an option for resources.
     if (artifact.isFileset() || artifact.isTreeArtifact()) {
-      return fragment.subFragment(segmentCount - 1, segmentCount);
+      return fragment.subFragment(segmentCount - 1);
     }
 
     // Check the resource folder type layout.
@@ -320,23 +325,20 @@ public final class LocalResourceContainer {
     return fragment.subFragment(segmentCount - 3, segmentCount - 2);
   }
 
-  /**
-   * Returns the root-part of a given path by trimming off the end specified by a given tail.
-   * Assumes that the tail is known to match, and simply relies on the segment lengths.
-   */
-  private static PathFragment trimTail(PathFragment path, PathFragment tail) {
-    return path.subFragment(0, path.segmentCount() - tail.segmentCount());
-  }
-
-  private static PathFragment makeRelativeTo(PathFragment ancestor, PathFragment path) {
+  private static int segmentCountAfterAncestor(PathFragment ancestor, PathFragment path) {
     String cutAtSegment = ancestor.getSegment(ancestor.segmentCount() - 1);
-    int totalPathSegments = path.segmentCount() - 1;
-    for (int i = totalPathSegments; i >= 0; i--) {
-      if (path.getSegment(i).equals(cutAtSegment)) {
-        return path.subFragment(i, totalPathSegments);
+    int index = -1;
+    List<String> segments = path.getSegments();
+    for (int i = segments.size() - 1; i >= 0; i--) {
+      if (segments.get(i).equals(cutAtSegment)) {
+        index = i;
+        break;
       }
     }
-    throw new IllegalArgumentException("PathFragment " + path + " is not beneath " + ancestor);
+    if (index == -1) {
+      throw new IllegalArgumentException("PathFragment " + path + " is not beneath " + ancestor);
+    }
+    return segments.size() - index - 1;
   }
 
   private final ImmutableList<Artifact> resources;
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java
index 6737c6c986..7b549540a4 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java
@@ -487,7 +487,7 @@ public final class CcCommon {
         ruleContext.attributeError("includes",
             "Path references a path above the execution root.");
       }
-      if (includesPath.segmentCount() == 0) {
+      if (includesPath.isEmpty()) {
         ruleContext.attributeError(
             "includes",
             "'"
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
index 7bede6cf39..284be8cd10 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
@@ -893,7 +893,7 @@ public class CppCompileAction extends AbstractAction
     }
     // Need to do dir/package matching: first try a quick exact lookup.
     PathFragment includeDir = input.getRootRelativePath().getParentDirectory();
-    if (includeDir.segmentCount() == 0 || declaredIncludeDirs.contains(includeDir)) {
+    if (includeDir.isEmpty() || declaredIncludeDirs.contains(includeDir)) {
       return true;  // OK: quick exact match.
     }
     // Not found in the quick lookup: try the wildcards.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaUtil.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaUtil.java
index eaddad0dfc..ee0134206e 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaUtil.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaUtil.java
@@ -157,7 +157,7 @@ public final class JavaUtil {
   public static PathFragment getJavaPath(PathFragment path) {
     int index = javaSegmentIndex(path);
     if (index >= 0) {
-      return path.subFragment(index + 1, path.segmentCount());
+      return path.subFragment(index + 1);
     }
     return null;
   }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java
index 024287deb3..2b15ced504 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java
@@ -509,7 +509,7 @@ public abstract class RepositoryFunction {
       throws IOException, InterruptedException {
     Path externalRepoDir = getExternalRepositoryDirectory(directories);
     PathFragment repositoryPath = rootedPath.asPath().relativeTo(externalRepoDir);
-    if (repositoryPath.segmentCount() == 0) {
+    if (repositoryPath.isEmpty()) {
       // We are the top of the repository path (<outputBase>/external), not in an actual external
       // repository path.
       return;