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

Allow juxtaposition of macro and array literal. fixes #23519 #23547

Merged
merged 4 commits into from
Oct 2, 2017

Conversation

cdsousa
Copy link
Contributor

@cdsousa cdsousa commented Sep 1, 2017

This is an attempt to implement what I have suggested in #23519:
interpret @foo[1, 2, 3] as @foo([1, 2, 3]).

I have almost (now) zero knowledge of scheme, so pardon me if the implementation is too wrong.
It seems to work ok in many situations, but it doesn't have any test yet.

@ararslan ararslan added arrays [a, r, r, a, y, s] macros @macros needs docs Documentation for this change is required needs news A NEWS entry is required for this change needs tests Unit tests are required for this change labels Sep 1, 2017
@ararslan ararslan removed the needs tests Unit tests are required for this change label Sep 2, 2017
@cdsousa cdsousa force-pushed the fix23519 branch 3 times, most recently from 4b776b8 to 17ec447 Compare September 2, 2017 12:50
@JeffBezanson
Copy link
Member

Implementation looks fine to me.

@JeffBezanson JeffBezanson added the parser Language parsing and surface syntax label Sep 2, 2017
@ararslan ararslan removed the needs docs Documentation for this change is required label Sep 3, 2017
@ararslan
Copy link
Member

ararslan commented Sep 3, 2017

What you've noted in the docs, that @m[x] * v is not the same as @m [x] * v implies to me that this will introduce another subtle "gotcha" for macro invocations. The fact that omitting a space changes the meaning in a subtle way is concerning. Is it really so onerous to use @m([x])?

@cdsousa
Copy link
Contributor Author

cdsousa commented Sep 4, 2017

Yes, unfortunately, it adds yet another gotcha, along with @m(1,2) vs. @m (1,2), but it is somehow of the same type and thus hopefully easier to remember...

From my experience, my brain defaults to already interpret @m[x] * v as applying only to [x] although it has never been the case. That was the reason which triggers me to open the issue in the first place.

This desire for a lightweight syntax for "non-standard array literals" comes mainly from static arrays, where, possibly unlike to other custom arrays, it is very common to need small literal arrays with constructors as performant as possible. Indeed StaticArrays is all about the performance of small matrices where stuff like construction, indexing, etc. (and their optimization/inlining) tends to be as much important as matrix algebra and iteration. Also, array literals (excluding concatenations and comprehension) are used almost always for small matrices!

Many discussions have been taken about a more lightweight syntax for custom array literals (notice that array elements don't have to be literal numbers). I referred some in #23519, and another important one happens JuliaArrays/StaticArrays.jl#24 .

The syntax change I'm proposing is a very low-hanging fruit for "something reasonabe".
Having it will readily allow StaticArrays users to simply prefix arrays/vectors/matrices literals with @SArray (or something like @SA) and instantly get small static arrays with types and sizes resolved at compile time solely from the literal.
Maybe, StaticArrays developers, @andyferris, @SimonDanisch, and others (I'm merely a user), can also have something to say here.

@cdsousa cdsousa force-pushed the fix23519 branch 2 times, most recently from dfe96c3 to 880aff7 Compare September 22, 2017 09:17
@cdsousa cdsousa changed the title [WIP/RFC] Allow juxtaposition of macro and array literal. fixes #23519 Allow juxtaposition of macro and array literal. fixes #23519 Sep 22, 2017
@cdsousa
Copy link
Contributor Author

cdsousa commented Sep 22, 2017

Rebased.

@cdsousa
Copy link
Contributor Author

cdsousa commented Sep 27, 2017

Some people have shown dislike and some people have shown sympathy for this change, both here and in the issue (#23519). How usually are these cases resolved? I think that it is better to either merge or reject the PR, right?

@andyferris
Copy link
Member

It can sometimes be a slow process, @cdsousa.

If you want resolution, I'll add some labels which means it will be picked up by what's being called the "triage" process.

@andyferris andyferris added needs decision A decision on this change is needed triage This should be discussed on a triage call labels Sep 28, 2017
@andyferris andyferris added this to the 1.0 milestone Sep 28, 2017
@martinholters
Copy link
Member

@ararslan The fact that

@m[x] * v is not the same as @m [x] * v

doesn't seem worse than the fact that @m([x]) * v is not the same as @m ([x]) * v to me.

@JeffBezanson JeffBezanson removed triage This should be discussed on a triage call needs decision A decision on this change is needed labels Sep 28, 2017
@JeffBezanson
Copy link
Member

Needs news, then I will merge this.

@JeffBezanson JeffBezanson removed the needs news A NEWS entry is required for this change label Oct 2, 2017
@JeffBezanson JeffBezanson merged commit 6c4e30e into JuliaLang:master Oct 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrays [a, r, r, a, y, s] macros @macros parser Language parsing and surface syntax
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants