Browse Source

Merge "Fix FirstUniqueStrings after conversion to generics"

Treehugger Robot 10 months ago
parent
commit
da169a1ee3
6 changed files with 18 additions and 29 deletions
  1. 1 11
      android/depset_generic.go
  2. 15 12
      android/util.go
  3. 1 1
      android/util_test.go
  4. 0 2
      dexpreopt/dexpreopt.go
  5. 0 2
      java/dexpreopt_config.go
  6. 1 1
      java/robolectric.go

+ 1 - 11
android/depset_generic.go

@@ -175,16 +175,6 @@ func (d *DepSet[T]) walk(visit func([]T)) {
 // its transitive dependencies, in which case the ordering of the duplicated element is not
 // guaranteed).
 func (d *DepSet[T]) ToList() []T {
-	return d.toList(firstUnique[T])
-}
-
-// toList returns the DepSet flattened to a list.  The order in the list is based on the order
-// of the DepSet.  POSTORDER and PREORDER orders return a postordered or preordered left to right
-// flattened list.  TOPOLOGICAL returns a list that guarantees that elements of children are listed
-// after all of their parents (unless there are duplicate direct elements in the DepSet or any of
-// its transitive dependencies, in which case the ordering of the duplicated element is not
-// guaranteed).  The firstUniqueFunc is used to remove duplicates from the list.
-func (d *DepSet[T]) toList(firstUniqueFunc func([]T) []T) []T {
 	if d == nil {
 		return nil
 	}
@@ -192,7 +182,7 @@ func (d *DepSet[T]) toList(firstUniqueFunc func([]T) []T) []T {
 	d.walk(func(paths []T) {
 		list = append(list, paths...)
 	})
-	list = firstUniqueFunc(list)
+	list = firstUniqueInPlace(list)
 	if d.reverse {
 		reverseSliceInPlace(list)
 	}

+ 15 - 12
android/util.go

@@ -25,12 +25,12 @@ import (
 )
 
 // CopyOf returns a new slice that has the same contents as s.
-func CopyOf(s []string) []string {
+func CopyOf[T any](s []T) []T {
 	// If the input is nil, return nil and not an empty list
 	if s == nil {
 		return s
 	}
-	return append([]string{}, s...)
+	return append([]T{}, s...)
 }
 
 // Concat returns a new slice concatenated from the two input slices. It does not change the input
@@ -288,22 +288,25 @@ func RemoveFromList(s string, list []string) (bool, []string) {
 }
 
 // FirstUniqueStrings returns all unique elements of a slice of strings, keeping the first copy of
-// each.  It modifies the slice contents in place, and returns a subslice of the original slice.
+// each.  It does not modify the input slice.
 func FirstUniqueStrings(list []string) []string {
-	// Do not moodify the input in-place, operate on a copy instead.
-	list = CopyOf(list)
-	// 128 was chosen based on BenchmarkFirstUniqueStrings results.
-	if len(list) > 128 {
-		return firstUnique(list)
-	}
 	return firstUnique(list)
 }
 
 // firstUnique returns all unique elements of a slice, keeping the first copy of each.  It
-// modifies the slice contents in place, and returns a subslice of the original slice.
+// does not modify the input slice.
 func firstUnique[T comparable](slice []T) []T {
-	// 4 was chosen based on Benchmark_firstUnique results.
-	if len(slice) > 4 {
+	// Do not modify the input in-place, operate on a copy instead.
+	slice = CopyOf(slice)
+	return firstUniqueInPlace(slice)
+}
+
+// firstUniqueInPlace returns all unique elements of a slice, keeping the first copy of
+// each.  It modifies the slice contents in place, and returns a subslice of the original
+// slice.
+func firstUniqueInPlace[T comparable](slice []T) []T {
+	// 128 was chosen based on BenchmarkFirstUniqueStrings results.
+	if len(slice) > 128 {
 		return firstUniqueMap(slice)
 	}
 	return firstUniqueList(slice)

+ 1 - 1
android/util_test.go

@@ -385,7 +385,7 @@ func TestCopyOfEmptyAndNil(t *testing.T) {
 	emptyList := []string{}
 	copyOfEmptyList := CopyOf(emptyList)
 	AssertBoolEquals(t, "Copy of an empty list should be an empty list and not nil", true, copyOfEmptyList != nil)
-	copyOfNilList := CopyOf(nil)
+	copyOfNilList := CopyOf([]string(nil))
 	AssertBoolEquals(t, "Copy of a nil list should be a nil list and not an empty list", true, copyOfNilList == nil)
 }
 

+ 0 - 2
dexpreopt/dexpreopt.go

@@ -630,5 +630,3 @@ func contains(l []string, s string) bool {
 	}
 	return false
 }
-
-var copyOf = android.CopyOf

+ 0 - 2
java/dexpreopt_config.go

@@ -250,8 +250,6 @@ func bcpForDexpreopt(ctx android.PathContext, withUpdatable bool) (android.Writa
 
 var defaultBootclasspathKey = android.NewOnceKey("defaultBootclasspath")
 
-var copyOf = android.CopyOf
-
 func init() {
 	android.RegisterMakeVarsProvider(pctx, dexpreoptConfigMakevars)
 }

+ 1 - 1
java/robolectric.go

@@ -299,7 +299,7 @@ func generateSameDirRoboTestConfigJar(ctx android.ModuleContext, outputFile andr
 func (r *robolectricTest) generateRoboSrcJar(ctx android.ModuleContext, outputFile android.WritablePath,
 	instrumentedApp *AndroidApp) {
 
-	srcJarArgs := copyOf(instrumentedApp.srcJarArgs)
+	srcJarArgs := android.CopyOf(instrumentedApp.srcJarArgs)
 	srcJarDeps := append(android.Paths(nil), instrumentedApp.srcJarDeps...)
 
 	for _, m := range ctx.GetDirectDepsWithTag(roboCoverageLibsTag) {