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

Add support for angle bracket ⟨ ⟩ #27712

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

innerlee
Copy link
Contributor

@innerlee innerlee commented Jun 21, 2018

This is part of #27697, in which we decided to add brackets one by one.

On this pr:

julia> dump(:( ⟨a,b⟩ ))
Expr
  head: Symbol anglebracket
  args: Array{Any}((2,))
    1: Symbol a
    2: Symbol b

Note, this pr implements the angle bracket as function call anglebracket. No other functionality is implemented. Especially,

  • do not support ⟨1; 2⟩
  • do not support ⟨1 2⟩
  • do not support ⟨1 2; 3 4⟩
  • do not support f⟨x⟩
  • did not add any pre-defined methods for it

@MasonProtter
Copy link
Contributor

Should Base even define an anglebracket function? Why not just have

julia> dump(:( ⟨a,b⟩ ))
Expr
  head: Symbol call
  args: Array{Any}((3,))
    1: Symbol anglebracket
    2: Symbol a
    3: Symbol b

and then if someone tries to evaluate that, they get and UndefVarError anglebracket not defined? With the current way, it's type piracy if people add methods to anglebracket for types they don't own, whereas if we just make people define their own anglebracket function the type piracy is avoided.

Copy link
Member

@simeonschaub simeonschaub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems reasonable and useful to me, I think this was just forgotten. Are you still interested in this?

@test Meta.lower(@__MODULE__, :(⟨a;⟩)) == Expr(:error, "unexpected semicolon inside ⟨ ⟩")
@test_throws ParseError Meta.parse("⟨a b⟩")
@test_throws ParseError Meta.parse("⟨1 2;3 4⟩")
@test Meta.parse("f⟨x⟩") == :(f * $(Expr(:anglebracket, :x)))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder whether it would make more sense to give this its own expression head, like we already do with vect/ref, braces/curly, or [typed_]h/vcat, instead of parsing this as juxtaposition.

Copy link
Contributor

@MasonProtter MasonProtter Oct 30, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's a good idea. :anglecall? :anglegetindex?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, anglecall probably seems most natural to me.

Copy link
Contributor

@MasonProtter MasonProtter Oct 30, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One reason we might want to call it :anglegetindex is because it seems natural to also want a :anglesetindex for the expression f⟨x⟩ = 1, but I don't have a strong feeling either way.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't it be called angleref then? With this PR, your example should still be valid surface syntax for use in macros. I don't think we want to do anything special for this in lowering though, at least for now, since it's not really clear what this should do.

@simeonschaub simeonschaub added feature Indicates new feature / enhancement requests compiler:lowering Syntax lowering (compiler front end, 2nd stage) parser Language parsing and surface syntax labels Oct 30, 2020
@simeonschaub
Copy link
Member

I agree with @MasonProtter that anglebracket should not be defined in Base. I think it would make the most sense to lower this syntax to just anglebracket instead of (top anglebracket).

@innerlee
Copy link
Contributor Author

Yes definitely has interests and I need some instructions.
On the functional side: do we want to support [ ]-like operations ⟨1; 2⟩, ⟨1 2⟩, ⟨1 2; 3 4⟩, f⟨x⟩?
On the implementation side: what is the best approach?
And the naming :D

@simeonschaub
Copy link
Member

simeonschaub commented Oct 31, 2020

Yes definitely has interests and I need some instructions.

Ok, awesome!

On the functional side: do we want to support [ ]-like operations ⟨1; 2⟩, ⟨1 2⟩, ⟨1 2; 3 4⟩, f⟨x⟩?

I am not so sure whether angle brackets should behave more like regular parentheses or square brackets, so I think it's better to do what we currently do for curly brackets, in that we don't allow whitespace-sensitive parsing or semicolons, at least for now. (Edit: I just realized that we do allow bracescat expressions, so maybe we should allow anglecat as well?) f⟨x⟩ seems fine to me, the only thing to decide would be whether to call this anglecall, angleref, or something else and I would then just make this an error in lowering for now.

On the implementation side: what is the best approach?

Your current implementation looks good to me overall, you probably want to have a look at the function parse-call-chain for how to implement the parsing for f⟨x⟩.

And the naming :D

Yeah, that's always the hardest part! 😆 I think anglebracket is probably fine. I thought about calling it just angle, but we can't use that for the function name this syntax is lowered to, since there already is a function angle in Base.

@antoine-levitt
Copy link
Contributor

By not defining anglebracket in base though it means that packages exporting it will conflict with each other. It seems to me that anglebracket(a,b) should default to dot(a,b), no? Is there any other standard use for this notation?

@MasonProtter
Copy link
Contributor

MasonProtter commented Nov 2, 2020

By not defining anglebracket in base though it means that packages exporting it will conflict with each other.

Then they should coordinate with each other via a lightweight ____base package that defines the meaning they want for the angled brackets.

It seems to me that anglebracket(a,b) should default to dot(a,b), no? Is there any other standard use for this notation?

That seems like a huge waste of this syntax, if it’s only purpose would be to be a other way to spell \cdot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler:lowering Syntax lowering (compiler front end, 2nd stage) feature Indicates new feature / enhancement requests parser Language parsing and surface syntax
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants