SE-0093 - Add public base property to slices #2929
Conversation
|
Thank you! Could you add some tests? They should go into For this one, a test that constructs a slice around a reference-typed collection, and then checks that the |
| @@ -435,6 +435,13 @@ public struct ${Self}<Base : ${BaseRequirements}> | |||
| % else: | |||
| internal let _base: Base | |||
| % end | |||
|
|
|||
| /// The underlying collection of the slice. | |||
| public var base: Base { | |||
There was a problem hiding this comment.
public var base: Base {
return _base
}would be sufficient. (No need for get {}.)
ultramiraculous
force-pushed
the
slice-base
branch
from
88d41fc to
3237bdf
Compare
|
Addressed the comment and added a basic test. |
|
On second thought, should it actually be copying the collection passed in instead of just referencing it? It seems like you could mutate the reference-type collection you pass in as base and basically break Slice in unexpected ways. |
This is expected. Slices inherit value or reference semantics from the base collection. |
| @@ -69,6 +91,12 @@ SliceTests.test("${Slice}/init(base:bounds:)") { | |||
| } | |||
| } | |||
|
|
|||
| SliceTests.test("${Slice}/baseProperty") { | |||
| let referenceCollection = ReferenceCollection() | |||
| let testSlice = ${Collection}(base: ReferenceCollection(), bounds: 0..<1) | |||
There was a problem hiding this comment.
I'm afraid this wouldn't compile because ReferenceCollection is only a Collection, but some slices have stronger requirements. Since all slice types are generated from the same code, let's only test the most simple Slice type.
There was a problem hiding this comment.
Oh yeah this isn't going to work at all. It looks like I didn't stage all my changes (sign I was working too late 😐). To start with ${Collection} isn't the right variable, and it's not using referenceCollection at all in this iteration. I was intending to just use Slice, like you suggested, but I also figured out how to test a few more by filtering out RangeReplaceable and Mutable slices.
|
@swift-ci Please smoke test |
|
@ultramiraculous Could you take a look at the test failures? |
ultramiraculous
force-pushed
the
slice-base
branch
from
3237bdf to
d08cbd6
Compare
|
@swift-ci Please test |
|
Awesome, thanks Chris. Please add an entry to the changelog too. |
[pull] swiftwasm from main
What's in this pull request?
Implement SE-0093 with a readonly property on Slice that returns the internal value
_base.Before merging this pull request to apple/swift repository:
Triggering Swift CI
The swift-ci is triggered by writing a comment on this PR addressed to the GitHub user @swift-ci. Different tests will run depending on the specific comment that you use. The currently available comments are:
Smoke Testing
Validation Testing
Lint Testing
Note: Only members of the Apple organization can trigger swift-ci.