Improvement to API docs: GenTraversableLike and List. Fixes SI-5522. #239
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
eed3si9n
pushed a commit
to eed3si9n/scala
that referenced
this pull request
May 14, 2019
The following commit fixes the issues with as seen from that are explained in this fantastic issue by Greg: scala#174, as well as scala#239 that handles structural types. These issues are related to the fact that our previous approach was only inspecting types, when some type information like type bounds is only present in symbols. To get that information, we need a more precise search that looks into the core of the Scalac types for the required information. Of course, type bounds is not all we're interested about. The issue is that type members, method parameters and type parameters can have type information in its definition that is necessary at the use site to detect and propagate changes. This information is also tied to the fact that type members can have different materializations depending on the prefix (both because of type members and path-dependent types). `types-in-used-names-b` and `as-seen-from-b` are a perfect example of this. This commit turns them into passing tests. Having more in-depth look at the algorithm behind it will help us understand what it does. In essence, the new type traverser is the responsible of adding dependencies on every `TypeRef` and `SingleType`. They contain concrete information about types (they are materialized), so their presence must be recorded. We also have the presence of other types like `PolyType` and `MethodType`. These types are used for defining type parameters for classes (think List[A]) and method type parameters (think def foo[T](t: T)). They are nested, meaning that their return type can also be a `PolyType` or a `MethodType`. To handle them, we traverse the symbols in their definition -- for method types we traverse the types of the parameters, while for poly types we add directly the dependency on the symbol --so that the name of the type parameters are also recorded-- and then we continue checking for their information if they are not a class, that is, if they are an abstract type with a definition that we may need to traverse (existential type, refined type, bounds, etc). In the case of `TypeBounds`, we traverse them if they are not the default specified by the SLS (`Nothing` for low bound, `Any` for high). Refined types need special handling since we need to check their declarations, that can introduce new type members or vals. If they do have them, we add a dependency right away on those definitions. As usual, `ThisType` and `ConstantType` need to be inspected by checking their underlying representation (`C` in `C.this` and `12` in `Int(12)`). `ExistentialType`, the last type on the traverser before falling back to `mapOver`, has a list of symbols called `quantified` that needs to be traversed since they are the symbol information that constrain the existential type. As in the case of `TypeBounds`, we guard against the default types `Nothing` for low bound and `Any` for high bound, so that unnecessary names that are always present in source files don't appear. This change triggers a very weird behaviour in 2.10, in which for some reason the names `Nothing` and `Any` appear. This does not seem to come from the new TypeDependencyTraverser and I've been able to track its appearance to the case in the traverser where we check for `hasSymbol` and add with `addSymbol`. I've added a TODO, which is not urgent, to find out what's happening, since this only affect one concrete snippet of the whole test code. Benchmark: ``` [info] # Run complete. Total time: 00:25:51 [info] [info] Benchmark (_tempDir) Mode Cnt Score Error Units [info] HotScalacBenchmark.compile /tmp/sbt_abdb5ed2 sample 18 20893.226 ± 625.622 ms/op [info] HotScalacBenchmark.compile:compile·p0.00 /tmp/sbt_abdb5ed2 sample 19797.115 ms/op [info] HotScalacBenchmark.compile:compile·p0.50 /tmp/sbt_abdb5ed2 sample 21005.074 ms/op [info] HotScalacBenchmark.compile:compile·p0.90 /tmp/sbt_abdb5ed2 sample 21894.267 ms/op [info] HotScalacBenchmark.compile:compile·p0.95 /tmp/sbt_abdb5ed2 sample 22045.262 ms/op [info] HotScalacBenchmark.compile:compile·p0.99 /tmp/sbt_abdb5ed2 sample 22045.262 ms/op [info] HotScalacBenchmark.compile:compile·p0.999 /tmp/sbt_abdb5ed2 sample 22045.262 ms/op [info] HotScalacBenchmark.compile:compile·p0.9999 /tmp/sbt_abdb5ed2 sample 22045.262 ms/op [info] HotScalacBenchmark.compile:compile·p1.00 /tmp/sbt_abdb5ed2 sample 22045.262 ms/op [info] HotScalacBenchmark.compile:·gc.alloc.rate /tmp/sbt_abdb5ed2 sample 18 289.838 ± 8.669 MB/sec [info] HotScalacBenchmark.compile:·gc.alloc.rate.norm /tmp/sbt_abdb5ed2 sample 18 6500730176.000 ± 13633760.029 B/op [info] HotScalacBenchmark.compile:·gc.churn.PS_Eden_Space /tmp/sbt_abdb5ed2 sample 18 289.082 ± 24.260 MB/sec [info] HotScalacBenchmark.compile:·gc.churn.PS_Eden_Space.norm /tmp/sbt_abdb5ed2 sample 18 6480403569.778 ± 464987965.594 B/op [info] HotScalacBenchmark.compile:·gc.churn.PS_Old_Gen /tmp/sbt_abdb5ed2 sample 18 12.679 ± 12.697 MB/sec [info] HotScalacBenchmark.compile:·gc.churn.PS_Old_Gen.norm /tmp/sbt_abdb5ed2 sample 18 290767194.667 ± 290528363.065 B/op [info] HotScalacBenchmark.compile:·gc.churn.PS_Survivor_Space /tmp/sbt_abdb5ed2 sample 18 7.321 ± 2.865 MB/sec [info] HotScalacBenchmark.compile:·gc.churn.PS_Survivor_Space.norm /tmp/sbt_abdb5ed2 sample 18 165547052.444 ± 66661097.019 B/op [info] HotScalacBenchmark.compile:·gc.count /tmp/sbt_abdb5ed2 sample 18 101.000 counts [info] HotScalacBenchmark.compile:·gc.time /tmp/sbt_abdb5ed2 sample 18 21332.000 ms [info] WarmScalacBenchmark.compile /tmp/sbt_abdb5ed2 sample 3 52769.937 ± 6743.004 ms/op [info] WarmScalacBenchmark.compile:compile·p0.00 /tmp/sbt_abdb5ed2 sample 52412.023 ms/op [info] WarmScalacBenchmark.compile:compile·p0.50 /tmp/sbt_abdb5ed2 sample 52747.567 ms/op [info] WarmScalacBenchmark.compile:compile·p0.90 /tmp/sbt_abdb5ed2 sample 53150.220 ms/op [info] WarmScalacBenchmark.compile:compile·p0.95 /tmp/sbt_abdb5ed2 sample 53150.220 ms/op [info] WarmScalacBenchmark.compile:compile·p0.99 /tmp/sbt_abdb5ed2 sample 53150.220 ms/op [info] WarmScalacBenchmark.compile:compile·p0.999 /tmp/sbt_abdb5ed2 sample 53150.220 ms/op [info] WarmScalacBenchmark.compile:compile·p0.9999 /tmp/sbt_abdb5ed2 sample 53150.220 ms/op [info] WarmScalacBenchmark.compile:compile·p1.00 /tmp/sbt_abdb5ed2 sample 53150.220 ms/op [info] WarmScalacBenchmark.compile:·gc.alloc.rate /tmp/sbt_abdb5ed2 sample 3 125.382 ± 13.840 MB/sec [info] WarmScalacBenchmark.compile:·gc.alloc.rate.norm /tmp/sbt_abdb5ed2 sample 3 7055970890.667 ± 1078954896.900 B/op [info] WarmScalacBenchmark.compile:·gc.churn.PS_Eden_Space /tmp/sbt_abdb5ed2 sample 3 117.215 ± 73.864 MB/sec [info] WarmScalacBenchmark.compile:·gc.churn.PS_Eden_Space.norm /tmp/sbt_abdb5ed2 sample 3 6596470733.333 ± 4281843293.325 B/op [info] WarmScalacBenchmark.compile:·gc.churn.PS_Survivor_Space /tmp/sbt_abdb5ed2 sample 3 2.279 ± 1.015 MB/sec [info] WarmScalacBenchmark.compile:·gc.churn.PS_Survivor_Space.norm /tmp/sbt_abdb5ed2 sample 3 128269752.000 ± 72721263.065 B/op [info] WarmScalacBenchmark.compile:·gc.count /tmp/sbt_abdb5ed2 sample 3 73.000 counts [info] WarmScalacBenchmark.compile:·gc.time /tmp/sbt_abdb5ed2 sample 3 8746.000 ms [info] ColdScalacBenchmark.compile /tmp/sbt_abdb5ed2 ss 10 44611.286 ± 963.131 ms/op [info] ColdScalacBenchmark.compile:·gc.alloc.rate /tmp/sbt_abdb5ed2 ss 10 152.054 ± 2.753 MB/sec [info] ColdScalacBenchmark.compile:·gc.alloc.rate.norm /tmp/sbt_abdb5ed2 ss 10 7249761568.800 ± 95126804.264 B/op [info] ColdScalacBenchmark.compile:·gc.churn.PS_Eden_Space /tmp/sbt_abdb5ed2 ss 10 144.481 ± 9.964 MB/sec [info] ColdScalacBenchmark.compile:·gc.churn.PS_Eden_Space.norm /tmp/sbt_abdb5ed2 ss 10 6889406191.200 ± 490961958.245 B/op [info] ColdScalacBenchmark.compile:·gc.churn.PS_Old_Gen /tmp/sbt_abdb5ed2 ss 10 ≈ 10⁻³ MB/sec [info] ColdScalacBenchmark.compile:·gc.churn.PS_Old_Gen.norm /tmp/sbt_abdb5ed2 ss 10 21136.000 ± 101049.368 B/op [info] ColdScalacBenchmark.compile:·gc.churn.PS_Survivor_Space /tmp/sbt_abdb5ed2 ss 10 2.848 ± 0.335 MB/sec [info] ColdScalacBenchmark.compile:·gc.churn.PS_Survivor_Space.norm /tmp/sbt_abdb5ed2 ss 10 135792956.800 ± 16291050.509 B/op [info] ColdScalacBenchmark.compile:·gc.count /tmp/sbt_abdb5ed2 ss 10 248.000 counts [info] ColdScalacBenchmark.compile:·gc.time /tmp/sbt_abdb5ed2 ss 10 29901.000 ms [success] Total time: 1553 s, completed Feb 26, 2017 3:06:29 AM [success] Total time: 0 s, completed Feb 26, 2017 3:06:29 AM ```
lrytz
pushed a commit
to lrytz/scala
that referenced
this pull request
Nov 5, 2019
The following commit fixes the issues with as seen from that are explained in this fantastic issue by Greg: scala#174, as well as scala#239 that handles structural types. These issues are related to the fact that our previous approach was only inspecting types, when some type information like type bounds is only present in symbols. To get that information, we need a more precise search that looks into the core of the Scalac types for the required information. Of course, type bounds is not all we're interested about. The issue is that type members, method parameters and type parameters can have type information in its definition that is necessary at the use site to detect and propagate changes. This information is also tied to the fact that type members can have different materializations depending on the prefix (both because of type members and path-dependent types). `types-in-used-names-b` and `as-seen-from-b` are a perfect example of this. This commit turns them into passing tests. Having more in-depth look at the algorithm behind it will help us understand what it does. In essence, the new type traverser is the responsible of adding dependencies on every `TypeRef` and `SingleType`. They contain concrete information about types (they are materialized), so their presence must be recorded. We also have the presence of other types like `PolyType` and `MethodType`. These types are used for defining type parameters for classes (think List[A]) and method type parameters (think def foo[T](t: T)). They are nested, meaning that their return type can also be a `PolyType` or a `MethodType`. To handle them, we traverse the symbols in their definition -- for method types we traverse the types of the parameters, while for poly types we add directly the dependency on the symbol --so that the name of the type parameters are also recorded-- and then we continue checking for their information if they are not a class, that is, if they are an abstract type with a definition that we may need to traverse (existential type, refined type, bounds, etc). In the case of `TypeBounds`, we traverse them if they are not the default specified by the SLS (`Nothing` for low bound, `Any` for high). Refined types need special handling since we need to check their declarations, that can introduce new type members or vals. If they do have them, we add a dependency right away on those definitions. As usual, `ThisType` and `ConstantType` need to be inspected by checking their underlying representation (`C` in `C.this` and `12` in `Int(12)`). `ExistentialType`, the last type on the traverser before falling back to `mapOver`, has a list of symbols called `quantified` that needs to be traversed since they are the symbol information that constrain the existential type. As in the case of `TypeBounds`, we guard against the default types `Nothing` for low bound and `Any` for high bound, so that unnecessary names that are always present in source files don't appear. This change triggers a very weird behaviour in 2.10, in which for some reason the names `Nothing` and `Any` appear. This does not seem to come from the new TypeDependencyTraverser and I've been able to track its appearance to the case in the traverser where we check for `hasSymbol` and add with `addSymbol`. I've added a TODO, which is not urgent, to find out what's happening, since this only affect one concrete snippet of the whole test code. Benchmark: ``` [info] # Run complete. Total time: 00:25:51 [info] [info] Benchmark (_tempDir) Mode Cnt Score Error Units [info] HotScalacBenchmark.compile /tmp/sbt_abdb5ed2 sample 18 20893.226 ± 625.622 ms/op [info] HotScalacBenchmark.compile:compile·p0.00 /tmp/sbt_abdb5ed2 sample 19797.115 ms/op [info] HotScalacBenchmark.compile:compile·p0.50 /tmp/sbt_abdb5ed2 sample 21005.074 ms/op [info] HotScalacBenchmark.compile:compile·p0.90 /tmp/sbt_abdb5ed2 sample 21894.267 ms/op [info] HotScalacBenchmark.compile:compile·p0.95 /tmp/sbt_abdb5ed2 sample 22045.262 ms/op [info] HotScalacBenchmark.compile:compile·p0.99 /tmp/sbt_abdb5ed2 sample 22045.262 ms/op [info] HotScalacBenchmark.compile:compile·p0.999 /tmp/sbt_abdb5ed2 sample 22045.262 ms/op [info] HotScalacBenchmark.compile:compile·p0.9999 /tmp/sbt_abdb5ed2 sample 22045.262 ms/op [info] HotScalacBenchmark.compile:compile·p1.00 /tmp/sbt_abdb5ed2 sample 22045.262 ms/op [info] HotScalacBenchmark.compile:·gc.alloc.rate /tmp/sbt_abdb5ed2 sample 18 289.838 ± 8.669 MB/sec [info] HotScalacBenchmark.compile:·gc.alloc.rate.norm /tmp/sbt_abdb5ed2 sample 18 6500730176.000 ± 13633760.029 B/op [info] HotScalacBenchmark.compile:·gc.churn.PS_Eden_Space /tmp/sbt_abdb5ed2 sample 18 289.082 ± 24.260 MB/sec [info] HotScalacBenchmark.compile:·gc.churn.PS_Eden_Space.norm /tmp/sbt_abdb5ed2 sample 18 6480403569.778 ± 464987965.594 B/op [info] HotScalacBenchmark.compile:·gc.churn.PS_Old_Gen /tmp/sbt_abdb5ed2 sample 18 12.679 ± 12.697 MB/sec [info] HotScalacBenchmark.compile:·gc.churn.PS_Old_Gen.norm /tmp/sbt_abdb5ed2 sample 18 290767194.667 ± 290528363.065 B/op [info] HotScalacBenchmark.compile:·gc.churn.PS_Survivor_Space /tmp/sbt_abdb5ed2 sample 18 7.321 ± 2.865 MB/sec [info] HotScalacBenchmark.compile:·gc.churn.PS_Survivor_Space.norm /tmp/sbt_abdb5ed2 sample 18 165547052.444 ± 66661097.019 B/op [info] HotScalacBenchmark.compile:·gc.count /tmp/sbt_abdb5ed2 sample 18 101.000 counts [info] HotScalacBenchmark.compile:·gc.time /tmp/sbt_abdb5ed2 sample 18 21332.000 ms [info] WarmScalacBenchmark.compile /tmp/sbt_abdb5ed2 sample 3 52769.937 ± 6743.004 ms/op [info] WarmScalacBenchmark.compile:compile·p0.00 /tmp/sbt_abdb5ed2 sample 52412.023 ms/op [info] WarmScalacBenchmark.compile:compile·p0.50 /tmp/sbt_abdb5ed2 sample 52747.567 ms/op [info] WarmScalacBenchmark.compile:compile·p0.90 /tmp/sbt_abdb5ed2 sample 53150.220 ms/op [info] WarmScalacBenchmark.compile:compile·p0.95 /tmp/sbt_abdb5ed2 sample 53150.220 ms/op [info] WarmScalacBenchmark.compile:compile·p0.99 /tmp/sbt_abdb5ed2 sample 53150.220 ms/op [info] WarmScalacBenchmark.compile:compile·p0.999 /tmp/sbt_abdb5ed2 sample 53150.220 ms/op [info] WarmScalacBenchmark.compile:compile·p0.9999 /tmp/sbt_abdb5ed2 sample 53150.220 ms/op [info] WarmScalacBenchmark.compile:compile·p1.00 /tmp/sbt_abdb5ed2 sample 53150.220 ms/op [info] WarmScalacBenchmark.compile:·gc.alloc.rate /tmp/sbt_abdb5ed2 sample 3 125.382 ± 13.840 MB/sec [info] WarmScalacBenchmark.compile:·gc.alloc.rate.norm /tmp/sbt_abdb5ed2 sample 3 7055970890.667 ± 1078954896.900 B/op [info] WarmScalacBenchmark.compile:·gc.churn.PS_Eden_Space /tmp/sbt_abdb5ed2 sample 3 117.215 ± 73.864 MB/sec [info] WarmScalacBenchmark.compile:·gc.churn.PS_Eden_Space.norm /tmp/sbt_abdb5ed2 sample 3 6596470733.333 ± 4281843293.325 B/op [info] WarmScalacBenchmark.compile:·gc.churn.PS_Survivor_Space /tmp/sbt_abdb5ed2 sample 3 2.279 ± 1.015 MB/sec [info] WarmScalacBenchmark.compile:·gc.churn.PS_Survivor_Space.norm /tmp/sbt_abdb5ed2 sample 3 128269752.000 ± 72721263.065 B/op [info] WarmScalacBenchmark.compile:·gc.count /tmp/sbt_abdb5ed2 sample 3 73.000 counts [info] WarmScalacBenchmark.compile:·gc.time /tmp/sbt_abdb5ed2 sample 3 8746.000 ms [info] ColdScalacBenchmark.compile /tmp/sbt_abdb5ed2 ss 10 44611.286 ± 963.131 ms/op [info] ColdScalacBenchmark.compile:·gc.alloc.rate /tmp/sbt_abdb5ed2 ss 10 152.054 ± 2.753 MB/sec [info] ColdScalacBenchmark.compile:·gc.alloc.rate.norm /tmp/sbt_abdb5ed2 ss 10 7249761568.800 ± 95126804.264 B/op [info] ColdScalacBenchmark.compile:·gc.churn.PS_Eden_Space /tmp/sbt_abdb5ed2 ss 10 144.481 ± 9.964 MB/sec [info] ColdScalacBenchmark.compile:·gc.churn.PS_Eden_Space.norm /tmp/sbt_abdb5ed2 ss 10 6889406191.200 ± 490961958.245 B/op [info] ColdScalacBenchmark.compile:·gc.churn.PS_Old_Gen /tmp/sbt_abdb5ed2 ss 10 ≈ 10⁻³ MB/sec [info] ColdScalacBenchmark.compile:·gc.churn.PS_Old_Gen.norm /tmp/sbt_abdb5ed2 ss 10 21136.000 ± 101049.368 B/op [info] ColdScalacBenchmark.compile:·gc.churn.PS_Survivor_Space /tmp/sbt_abdb5ed2 ss 10 2.848 ± 0.335 MB/sec [info] ColdScalacBenchmark.compile:·gc.churn.PS_Survivor_Space.norm /tmp/sbt_abdb5ed2 ss 10 135792956.800 ± 16291050.509 B/op [info] ColdScalacBenchmark.compile:·gc.count /tmp/sbt_abdb5ed2 ss 10 248.000 counts [info] ColdScalacBenchmark.compile:·gc.time /tmp/sbt_abdb5ed2 ss 10 29901.000 ms [success] Total time: 1553 s, completed Feb 26, 2017 3:06:29 AM [success] Total time: 0 s, completed Feb 26, 2017 3:06:29 AM ``` Rewritten from sbt/zinc@929b758
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Tiny adjustment of wording in
slicemethod onGenTraversableLikeAPI docs. Additionally added example toListAPI docs.Closes SI-5522