support a[begin] for a[firstindex(a)]

[stevengj]

This is resurrected from #25458, and reverts commit e016f11. Since the use of begin inside indexing expressions was deprecated in Julia 0.7 (#25614), this is not a breaking change. Resolves #15750.

cc @Keno, @timholy

[JeffBezanson]

I think it's really unfortunate not to be able to use begin ... end blocks inside indexing or comprehensions.

[stevengj]

I think it's really unfortunate not to be able to use begin ... end blocks inside indexing or comprehensions.

It's been deprecated for a while now and I haven't seen any complaints? You can always use let ... end blocks in the (apparently) extremely rare cases where you really would like this. To quote @StefanKarpinski,

[...] using begin inside of an indexing expression is so bizarre that I don't think it's a practical problem.

The basic motivation here is to fix the asymmetry with end in indexing, which was fine when array indices all started with 1, but now that offset arrays are more widely supported it is kind of unfortunate that it's so much less convenient to write indexing code for that case.

[stevengj]

(Windows CI failures look unrelated. One is a download failure and another is something with backtraces.)

[StefanKarpinski]

@JeffBezanson, it seems like if we're not going to do this, then we should introduce an alternative syntax for this, such as the a[$begin:$end] syntax proposed. Not sure if you like that better.

[JeffBezanson]

I remember we spent quite a while discussing syntax for this in the 1.0 timeframe, and unfortunately nobody was able to come up with anything appealing. Personally I do prefer $begin or @begin (or maybe @first and @last); anything to avoid overloading the keywords.

[stevengj]

That discussion was at #15750?

Given that we already have a[end] and can't change this, using anything other than a[begin] seems weird to me.

[StefanKarpinski]

Notes from triage.

@JeffBezanson is against continuing to disallow begin ... end in indexing context, preferring instead to reallow it and introduce different syntax, soft deprecating end in indexing until 2.0. However, there's not a lot of agreement on what syntax would be good except that it probably should be syntax, rather than @begin and @end for example. That pretty much leaves $begin and $end as options. These would be technically breaking:

julia> var"end" = 1
1

julia> :(a[$end])
:(a[1])

julia> var"begin" = 1
1

julia> :($begin)
1

julia> :(a[$begin])
:(a[1])

However, those usages are so weird that breaking them in a minor release would be fine. We're also fortunate that this does not work:

julia> :($begin 1 end)
ERROR: syntax: missing comma or ) in argument list
Stacktrace:
 [1] top-level scope at REPL[13]:0

Therefore using $begin and $end is possible.

However, @vtjnash and I were ok with just merging this and making a[begin] do indexing.

[ararslan]

Another +1 to merging this and going ahead with a[begin] to mirror a[end], which has been widely used and appreciated for quite a long time. (Also a hearty -1 to soft-deprecating a[end].)

[vtjnash]

I think this may need to include an update to show_expr, similar to #34038?

[stevengj]

I suspect that deprecating end indexing in 2.x…especially to replace it with something ugly like @end or $end, will set up such howls of protest that it will never happen. This is one of those things that is pleasing in theory but not in practice…

And anyway, given that we are not going to get rid of a[end] in 1.x, doesn't it make more sense to support a[begin] along with a[end] in 1.x, and then later on debate replacing them both in 2.0?

[StefanKarpinski]

Yes, I think we should just go for it. We already did the painful part: deprecating being ... end in indexing context. Might as well go with the rest of it for now. If someone comes up with a much better idea in the meantime, we can always use it in 2.0.

[stevengj]

We already did the painful part: deprecating being ... end in indexing context.

(Not too painful in practice. Did anyone even notice that this happened? I don't recall it ever being mentioned, and there are no links to #25614 on github or discourse. More evidence that this usage is not significant, compared to the ubiquitous need for indexing from the beginning and ending of arrays.)

[stevengj]

@vtjnash: I think this may need to include an update to show_expr

As far as I can tell, expression printing looks fine, including keyword cases ala #34038. For example

julia> :(a[begin:end, begin+1, end-1] + f(foo[begin]; kw=bar[begin+2]))
:(a[begin:end, begin + 1, end - 1] + f(foo[begin]; kw=bar[begin + 2]))

Is there a specific case you are concerned about?

(The base/show.jl file appears to have no special-casing of the end keyword, so it's not clear to me why it would need special-casing of begin.)

[StefanKarpinski]

Not too painful in practice. Did anyone even notice that this happened? I don't recall it ever being mentioned, and there are no links to #25614 on github or discourse.

I believe that @JeffBezanson fixed a number of instances of people doing this in packages for 1.0.

[vtjnash]

appears to have no special-casing of the end keyword

Hm, probably should. Outside of [] it should print as $:end or var"end". Here's a sampling of cases:

julia> Base.remove_linenums!(:(a[$:begin, $:end, ($:begin; $:end;)]))
:(a[begin, end, begin
          begin
          end
      end])

[mbauman]

I believe that JeffBezanson fixed a number of instances of people doing this in packages for 1.0.

I hit a few, too. IIRC the cases where this cropped up were almost entirely typed comprehensions.

[stevengj]

@vtjnash, I see…

Inside indexing expressions, blocks should probably print as (a; b; c). I'll work on a patch for that.

I feel like the printing of $:end and other keywords outside of [] should probably be handled in a separate PR, though, as this extends far beyond begin, e.g.

julia> :($:for + 1)
:(for + 1)

[stevengj]

Inside indexing expressions, blocks should probably print as (a; b; c). I'll work on a patch for that.

@vtjnash, this should be fixed now.

julia> Base.remove_linenums!(:(a[$:begin, $:end, ($:begin; $:end;)]))
:(a[begin, end, (begin;
          end)])

[maleadt]

@nanosoldier runtests(ALL, vs = ":master")

[stevengj]

@maleadt, note that this is just syntactic sugar (aside from changes to show(::Expr)), so any benchmark changes will surely just be noise.

[stevengj]

Win32 test failure is:

Error in testset backtrace:
Test Failed at D:\buildbot\worker-tabularasa\tester_win32\build\share\julia\test\backtrace.jl:239
  Expression: read(`$(Base.julia_cmd()) --startup-file=no --compile=min -e $code`, String) == "1000 1000"
   Evaluated: "998 1000" == "1000 1000"

Linux64 failure is:

���     ArrayAndChar(similar(Array{ElType}, axes(bc)), A.char)
��� end
��� 
��� "`A = find_aac(As)` returns the first ArrayAndChar among the arguments."
��� find_aac(bc::Base.Broadcast.Broadcasted) = find_aac(bc.args)
��� find_aac(args::Tuple) = find_aac(find_aac(args[1]), Base.tail(args))
��� find_aac(x) = x
��� find_aac(::Tuple{}) = nothing
��� find_aac(a::ArrayAndChar, rest) = a
��� find_aac(::Any, rest) = find_aac(rest)
��� 
��� Evaluated output:
��� 
��� find_aac (generic function with 6 methods)
��� 
��� Expected output:
��� 
��� find_aac (generic function with 5 methods)
��� 
���   diff =
���    Warning: Diff output requires color.
���    find_aac (generic function with 5 6 methods)
��� @ Documenter.DocTests /buildworker/worker/doctest_linux64/build/doc/deps/packages/Documenter/IsF1B/src/DocTests.jl:364

Both seem unrelated?

[KristofferC]

@maleadt, note that this is just syntactic sugar (aside from changes to show(::Expr)), so any benchmark changes will surely just be noise.

The tests run are not benchmarks. It is running the tests of all registered packages on master vs this branch.

[fredrikekre]

Linux64 failure is:

See #34075

[JeffBezanson]

Triage: will merge this.

[ZacLN]

Before I replicate in CSTParser, is the following intended?

Meta.parse("x[end 1]") # OK
Meta.parse("x[1 end]") # not OK

[JeffBezanson]

That looks like a bug to me.

[mbauman]

I missed this, but it's a bit of a shame that this isn't firstindex(a, n) to mirror lastindex(a, n).

[stevengj]

Sounds like a good non-breaking PR?