Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

split Base.lastindex semantics, avoid sentinel value #34697

Closed
goretkin opened this issue Feb 7, 2020 · 3 comments
Closed

split Base.lastindex semantics, avoid sentinel value #34697

goretkin opened this issue Feb 7, 2020 · 3 comments

Comments

@goretkin
Copy link
Contributor

goretkin commented Feb 7, 2020

It seems like lastindex serves two roles that are tempting to conflate, but could possibly benefit from being split up.

"what is the index of the last element"
This is what lastindex currently does, except for when the collection is empty (so there is no last element)

julia> lastindex([])
0

Possibly it should return nothing or throw a BoundsError, but 0 is really the only reasonable choice because of the second use:

"if I push!ed another element, what would the index to it be, minus 1"
This is the use case I find myself wanting, and it's simple enough to do lastindex(...) + 1 as can be found in many uses of lastindex e.g.

capacity(buffer::Buffer) = Int(pointer(buffer.data, lastindex(buffer.data) + 1) - buffer.ptr)

The behavior of lastindex can't be changed until 2.0, clearly. But if the distinction I am making above is useful, then we should have a separate function, possibly newindex. Having two closely related functions might be confusing, and users might not understand the big deal, since it's easy enough to do + 1 or - 1 depending on what you need, but I would argue that splitting up functions like this issue suggests makes it easier to handle boundaries / edge cases.

To be clear,

julia> my_newindex([])
1
@goretkin
Copy link
Contributor Author

goretkin commented Feb 7, 2020

I forgot to mention, if a collection can't be push!'d, then newindex should return an error. i.e. my_newindex(42) is not defined, even though lastindex(42) is, which I think further motivates the split.

@goretkin
Copy link
Contributor Author

goretkin commented May 5, 2020

Related:
#26511
#22354
#25385

Further motivation for this semantic split: if e.g. last(1:0) is defined to throw BoundsError, then lastindex([]) would necessarily as well.

Perhaps it would be not a pun to use Base.nextind(a::AbstractVector) for what I called my_newindex(a).

[EDIT: added more links 2020-aug-5]

@mbauman
Copy link
Member

mbauman commented May 5, 2020

There's a third very prominent role for lastindex:

julia> Meta.@lower A[end]
:($(Expr(:thunk, CodeInfo(
    @ none within `top-level scope'
1 ─ %1 = Base.lastindex(A)
│   %2 = Base.getindex(A, %1)
└──      return %2
))))

julia> Meta.@lower A[end, end]
:($(Expr(:thunk, CodeInfo(
    @ none within `top-level scope'
1 ─ %1 = Base.lastindex(A, 1)
│   %2 = Base.lastindex(A, 2)
│   %3 = Base.getindex(A, %1, %2)
└──      return %3
))))

Returning 0 for an empty collection is very helpful for syntaxes like A[begin:end]. Note that the empty behavior of firstindex is analogous for the same reason.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants