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

Precedence of box is possibly wrong #21192

Closed
nikomatsakis opened this issue Jan 15, 2015 · 21 comments
Closed

Precedence of box is possibly wrong #21192

nikomatsakis opened this issue Jan 15, 2015 · 21 comments
Labels
A-grammar Area: The grammar of Rust C-bug Category: This is a bug. P-low Low priority T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@nikomatsakis
Copy link
Contributor

Now that box is a keyword, and not the ~ operator, I think it's precedence is wrong. It maintains the precedence of a symbolic unary operator but I think keyword unary operators should have lower precedence.

In practical terms, this means that I think:

box 3 + 4

should parse as

box (3+4)

and not

(box 3) + 4

Nominating.

@huonw huonw added the A-grammar Area: The grammar of Rust label Jan 15, 2015
@nikomatsakis
Copy link
Contributor Author

cc @pnkfelix

@brson brson added this to the 1.0 beta milestone Jan 15, 2015
@pnkfelix
Copy link
Member

(subtask of #11779 ) ((well, sort of; depends on your definition of what "placement box" covers.))

@nikomatsakis
Copy link
Contributor Author

Assigning this to @pnkfelix too.

@kjpgit
Copy link
Contributor

kjpgit commented Feb 10, 2015

It would be great if the language would require explicit () in cases like this, for us mortal programmers who have to read code. See this classic: http://lwn.net/Articles/382024/

@pnkfelix
Copy link
Member

Possibly related: currently box e_1 as Box<Trait> parses as box (e_1 as Box<Trait>).

Considering the fallout likely to occur with #22181 (shown in prototype #22086), we probably want to instead parse box E as T as (box E) as T

@nikomatsakis
Copy link
Contributor Author

@pnkfelix yes, this is making me change my mind. Even though my intution is that the precedence of box is too high, it feels like box foo as Box<Trait> really ought to parse...though now that we do implicit coercion I basically never type that anymore.

@pnkfelix
Copy link
Member

still, could box parse with lower precedence than + but higher precedence than as ?

@nikomatsakis
Copy link
Contributor Author

@pnkfelix I think this doesn't work because the precedence of as is higher than +. For example, this program compiles:

fn main() {
    println!("{}", 3_u32 + 4_u64 as u32);
}

(Note also that my intuition is based on some notion that box is a keyword and hence has lower precedence. The current status is also reasonable in that it is the same as other unary operators (pretty tight).)

I think the precedence right now is roughly:

  1. suffix operators (x.foo, x[y], x.foo())
  2. unary operators (box, &, !)
  3. as
  4. binary operators like *
  5. binary operators like +

@pnkfelix
Copy link
Member

box is unstable, so no longer 1.0.

But, I-needs-decision!

@pnkfelix pnkfelix added the I-needs-decision Issue: In need of a decision. label Mar 26, 2015
@pnkfelix pnkfelix removed this from the 1.0 beta milestone Mar 26, 2015
@pnkfelix
Copy link
Member

triage: P-medium

@nixpulvis
Copy link

I agree, just bumped into this. box 1 + 2 feels like it really should be box(1 + 2). Another problem here is that things might be hidden issues, for example consider the following. It's because objects dereference for method resolution automatically, x and y could be either the same or different and the programmer might not notice at first.

let v = vec![1,2,3];
let x =  box v[0];
let y = (box v)[0];

@brson brson added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Aug 25, 2016
@nikomatsakis
Copy link
Contributor Author

triage: P-low

Decided to keep this open as the place to discuss precedence of box specifically, but not that it must be resolved before box is stabilized.

@rust-highfive rust-highfive removed I-needs-decision Issue: In need of a decision. P-medium Medium priority labels Aug 25, 2016
@rust-highfive rust-highfive added the P-low Low priority label Aug 25, 2016
@nixpulvis
Copy link

@kjpgit I'm not sure I completely agree, although I do think Rust is airing in the side of too many levels now. As a programmer that primarily learned using Racket I'd love to throw parenthesis everywhere, but I really don't want to end up with a weird rule like you sometime need parens unless it's a box that is near other operators.

I've always thought that precedence should be a programmer problem solved by education. Personally I've always wanted to see what an editor aware of the grammar of a language could do in terms of alerting programmers to potential issues. A simple fix with an editor would be to show the implicit parens at like 50% opacity.

@Mark-Simulacrum Mark-Simulacrum added the C-bug Category: This is a bug. label Jul 22, 2017
@steveklabnik
Copy link
Member

Triage: IIRC, we're going back the drawing board again with the box syntax stuff. @pnkfelix ?

@nixpulvis
Copy link

nixpulvis commented Sep 24, 2018

If you're going back to the drawing board. What's wrong with box!(...)? Kinda like vec![...]. Just a thought.

@pnkfelix
Copy link
Member

cc @rust-lang/wg-grammar : this remains an open question and it would be great to try to attack it. (It may well tie in with whatever syntax we adopt for await, for that matter...)

@pnkfelix
Copy link
Member

I'll just mention again the potential overlap with await ( #57640 and the associated write-up) ...

at least, I think reasonable people would read 3 + 4.box as parsing as 3 + (box 4), and thus switching from prefix box keyword to a postfix .box syntax would probably resolve our issues here.

@nixpulvis
Copy link

nixpulvis commented May 17, 2019

I agree, and think we should adopt the same style for these kinds of keywords in general. If we decide on prefix with custom precedence then it should be:

box try await e
// or
box(try(await(e)))

Of course try! is a macro because it can be (in some ways the origins of this issue).

Or if it's a macro then:

box!(try!(await!(e)))

Or finally, the proposed postfix method notation.

e.await.try.box

I'm personally a fan of the prefix notation for these, since I like field / tuple access and method call to be what . means, but I've read the other threads, and even voted in the poll, so I understand there's open questions about this still.

@Centril
Copy link
Contributor

Centril commented May 17, 2019

at least, I think reasonable people would read 3 + 4.box as parsing as 3 + (box 4), and thus switching from prefix box keyword to a postfix .box syntax would probably resolve our issues here.

@pnkfelix Yeah I think postfix box is the way to go and I want to thank whoever had the foresight to reserve box as a keyword in these contexts!

@pnkfelix
Copy link
Member

At this point I personally think that postfix .box is a good way to resolve the issues here.

Here is a summary of the various cases that have been raised in this comment thread:

CASE Current parsing Alternative parsing .box of current .box of alt
box 3 + 4 ? (box 3) + 4 box (3 + 4) 3.box + 4 (3 + 4).box
box e_1 as Box<Trait> ? (box e_1) as Box<Trait> box (e_1 as Box<Trait>) e_1.box as Box<Trait> (e_1 as Box<Trait>).box
box v[0] ? box (v[0]) (box v)[0] v[0].box v.box[0]

For the most part I think the postfix .box on the right is as clear or clearer than the prefix versions on the left columns. So that supports the assertion that postfix .box is worth investigating.

However, I also am not personally invested in this matter, and I don't think it is a pressing matter for the compiler or language teams in general.

so I'm going to unassign myself from this issue.

@pnkfelix pnkfelix removed their assignment Jan 14, 2020
@Mark-Simulacrum
Copy link
Member

Given that box syntax is currently permanently unstable and potentially on a deprecation path, I don't think this issue is too useful. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-grammar Area: The grammar of Rust C-bug Category: This is a bug. P-low Low priority T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

10 participants