WIP new @SA macro and comprehension support
#636
Conversation
Upside: This gives extremely attractive syntax for short static array literals. Downside: This is a syntax pun: the result of the `SA[...]` depends on whether the name `SA` is bound to `StaticArrays.SA` or is local variable of some other type. However, this is no worse than Int[1,2,3], which is also a pun on the array indexing syntax. Cases where this syntax could refer to something other than StaticArrays.SA are detectable by the compiler and in principle also accessible to a linter.
New `@SA` macro designed to be a shorter, more focussed and hopefully more correct version of `@SArray`. This macro can be used with array literal sytax like `@SA[1,2,3]`. Typed comprehension using one-based indexing and up to two dimensions is currently supported. Some work remains to generalize this but we can rely on constant propagation for things like `@SA T[f(x) for x in 1:3]` (the old macro would `eval` the `1:3` part).
|
Nice, will try to take a closer look tomorrow. |
There was a problem hiding this comment.
Cool looks reasonable to me 👍
I guess I prefer the syntax of the SA version which is why I originally went with that. But on the plus side the @SA version is not a pun on getindex.
Other than syntax, what's the difference really? Is it mainly about comprehensions? Maybe we can have that for SA as well; I should try to follow @tkf's idea from #633 down the rabbit hole and see where that leads to. Also there's the prospect of improving Base lowering so that we can support comprehensions with SA[i for i=1:3] I suppose.
| exprs = [ex.args[i].args[j] for i = 1:s1, j = 1:s2] | ||
| return esc(Expr(:call, SArray{Tuple{s1, s2}}, Expr(:tuple, exprs...))) | ||
| else # n x 1 | ||
| return esc(Expr(:call, SArray{Tuple{length(ex.args), 1}}, Expr(:tuple, ex.args...))) |
There was a problem hiding this comment.
Normal vcat returns a Vector rather than an n x 1 matrix.
| n_ranges = length(ex.args) - 1 | ||
| range_vars = [ex.args[i+1].args[1] for i = 1:n_ranges] | ||
| ranges = [ex.args[i+1].args[2] for i = 1:n_ranges] | ||
| func = esc(:(($(Expr(:tuple, range_vars...)) -> $(ex.args[1])))) |
There was a problem hiding this comment.
I wonder if we might be able to remove some of this logic and just pass the generator along to typed_sarray_comprehension. Ie, make this lowering less about the syntax, and more about just lowering to something other than typed_comprehension which is mangled by Base lowering...
c42f
added
design
|
cc @EvertSchippers @MeganDawson |
|
Replaced by #633 |
New
@SAmacro designed to be a shorter, more focussed and hopefully more correct version of the@SArraymacro. This macro can be used with array literal sytax like@SA[1,2,3](even without a space!).Typed comprehension using one-based indexing and up to two dimensions is currently supported. Some work remains to generalize this but we can rely on constant propagation for things like
@SA T[f(x) for x in 1:3](the old macro wouldevalthe1:3part).@c42f this is the other reasonable alternative to #633. Infact the PR includes that work as a subset - I suppose we should play a bit with both and see what we prefer. I suspect untyped comprehensions mightn't be too different to implement (there is type promotion logic in the
SArrayconstructor after all). The other required generalizations are to support more than two ranges and supporting multidimensional inputs in the comprehension (note thatArrays seem to take the Cartesian product of the inputkeys). We might also consider going beyond one-based indexing in the input arrays (but I'm not convinced that is necessary).