syntax: Support parentheses around trait bounds #41077

petrochenkov · 2017-04-04T23:43:28Z

petrochenkov · 2017-04-05T08:22:09Z

src/libsyntax/parse/parser.rs

                        self.bump(); // `+`
                        let pt = PolyTraitRef::new(Vec::new(), path.clone(), lo.to(self.prev_span));
                        let mut bounds = vec![TraitTyParamBound(pt, TraitBoundModifier::None)];
                        bounds.append(&mut self.parse_ty_param_bounds()?);
                        TyKind::TraitObject(bounds)
                    }
+                    TyKind::TraitObject(ref bounds) if maybe_bounds && bounds.len() == 1 => {


FIXME: Need to reject (Trait +) + Trait.

petrochenkov · 2017-04-06T19:50:51Z

src/libsyntax/parse/parser.rs

@@ -4048,17 +4059,24 @@ impl<'a> Parser<'a> {
    // Parse bounds of a type parameter `BOUND + BOUND + BOUND` without trailing `+`.
    // BOUND = TY_BOUND | LT_BOUND
    // LT_BOUND = LIFETIME (e.g. `'a`)
-    // TY_BOUND = [?] [for<LT_PARAM_DEFS>] SIMPLE_PATH (e.g. `?for<'a: 'b> m::Trait<'a>`)
+    // TY_BOUND = TY_BOUND_NOPAREN | (TY_BOUND_NOPAREN)
+    // TY_BOUND_NOPAREN = [?] [for<LT_PARAM_DEFS>] SIMPLE_PATH (e.g. `?for<'a: 'b> m::Trait<'a>`)


@nikomatsakis
You wanted a grammar, here it is.

Thanks. Sorry for my high lag, traveling this week.

nikomatsakis · 2017-04-12T17:09:41Z

OK, so, I'm not sure I'm 100% happy with these rules around parens, but honestly I'm ready to stop thinking about it. =) And besides if anything I would want to accept more, so it seems ok to accept less for now.

carols10cents · 2017-04-17T14:16:10Z

Soooo do i hear an r, @nikomatsakis? What are the next steps here?

nikomatsakis · 2017-04-18T14:08:07Z

@carols10cents heh, thanks for keeping me on my toes. Got distracted before deciding what the next step was -- in particular, whether to consult with @rust-lang/lang or what. I think I'm inclined to r+ regardless, let me do another quick review though, since I think that this can only be a subset of the behavior we would eventually want. (And maybe it's fine to just stop here for now.)

nikomatsakis

Left a few comments.

nikomatsakis · 2017-04-18T14:26:42Z

src/libsyntax/parse/parser.rs

                        self.bump(); // `+`
                        let pt = PolyTraitRef::new(Vec::new(), path.clone(), lo.to(self.prev_span));
                        let mut bounds = vec![TraitTyParamBound(pt, TraitBoundModifier::None)];
                        bounds.append(&mut self.parse_ty_param_bounds()?);
                        TyKind::TraitObject(bounds)
                    }
+                    TyKind::TraitObject(ref bounds)


What is an example where this case gets hit?

I guess it occurs with (for<'a> Foo)?

I guess it occurs with (for<'a> Foo)?

Exactly

nikomatsakis · 2017-04-18T14:28:17Z

src/libsyntax/parse/parser.rs

@@ -4070,6 +4088,9 @@ impl<'a> Parser<'a> {
                    TraitBoundModifier::None
                };
                bounds.push(TraitTyParamBound(poly_trait, modifier));
+                if has_parens {
+                    self.expect(&token::CloseDelim(token::Paren))?;
+                }
            } else {
                break


So, we consumed an open-paren at the start of the loop, don't we want to consume the close paren here somewhere?

(In general the handling of parens here feels complex.)

This breaks happens only if the code is already syntactically incorrect, something like

(Trait) + (Trait) + ($*&^#&*%%a;ch

so there may be no closing paren.
I can move expect(closing_paren) below else { ... } though, it should look a bit nicer.

@petrochenkov can you add some comments about this, at minimum? moving the expect(closing_patern) also sounds good.

Oh wait, the break is actually wrong in presence of ( or ?.
fn f<T: ?>(){} in particular compiles on all Rust versions since 1.0 😆

I'll fix this too.

petrochenkov · 2017-04-20T21:36:17Z

Updated.

nikomatsakis · 2017-04-22T10:08:12Z

@bors r+

bors · 2017-04-22T10:08:13Z

📌 Commit 8838cd1 has been approved by nikomatsakis

@nikomatsakis

…akis syntax: Support parentheses around trait bounds An implementation for rust-lang#39318 (comment) r? @nikomatsakis

Rollup of 3 pull requests - Successful merges: #41077, #41355, #41450 - Failed merges:

bors · 2017-04-22T16:46:48Z

⌛ Testing commit 8838cd1 with merge 9cc77d7...

rust-highfive assigned nikomatsakis Apr 4, 2017

petrochenkov commented Apr 5, 2017

View reviewed changes

petrochenkov force-pushed the boundparen branch from 27d1859 to 60ec1ac Compare April 6, 2017 19:48

petrochenkov commented Apr 6, 2017

View reviewed changes

nikomatsakis requested changes Apr 18, 2017

View reviewed changes

carols10cents added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 18, 2017

petrochenkov force-pushed the boundparen branch 3 times, most recently from b01f7b3 to 45a7ef2 Compare April 20, 2017 21:34

petrochenkov added 3 commits April 21, 2017 21:32

syntax: Support parentheses around trait bounds

e038f58

Fix issue with single question mark or paren

6e75def

Move parse_remaining_bounds into a separate function

8838cd1

petrochenkov force-pushed the boundparen branch from 45a7ef2 to 8838cd1 Compare April 21, 2017 19:15

nikomatsakis approved these changes Apr 22, 2017

View reviewed changes

frewsxcv added a commit to frewsxcv/rust that referenced this pull request Apr 22, 2017

Rollup merge of rust-lang#41077 - petrochenkov:boundparen, r=nikomats…

5e1351b

…akis syntax: Support parentheses around trait bounds An implementation for rust-lang#39318 (comment) r? @nikomatsakis

frewsxcv mentioned this pull request Apr 22, 2017

Rollup of 3 pull requests #41464

Merged

bors added a commit that referenced this pull request Apr 22, 2017

Auto merge of #41464 - frewsxcv:rollup, r=frewsxcv

9cc77d7

Rollup of 3 pull requests - Successful merges: #41077, #41355, #41450 - Failed merges:

bors merged commit 8838cd1 into rust-lang:master Apr 22, 2017

kennytm mentioned this pull request May 15, 2017

fn foo<T:?>() {} compiles despite the weird trait bound #42004

Closed

petrochenkov deleted the boundparen branch August 26, 2017 00:13

petrochenkov mentioned this pull request Dec 24, 2017

mini-RFC: Finalize syntax of impl Trait and dyn Trait with multiple bounds rust-lang/rfcs#2250

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

syntax: Support parentheses around trait bounds #41077

syntax: Support parentheses around trait bounds #41077

petrochenkov commented Apr 4, 2017 •

edited

Loading

petrochenkov Apr 5, 2017

petrochenkov Apr 6, 2017

nikomatsakis Apr 8, 2017

nikomatsakis commented Apr 12, 2017

carols10cents commented Apr 17, 2017

nikomatsakis commented Apr 18, 2017

nikomatsakis left a comment

nikomatsakis Apr 18, 2017

nikomatsakis Apr 18, 2017

petrochenkov Apr 19, 2017

nikomatsakis Apr 18, 2017

nikomatsakis Apr 18, 2017

petrochenkov Apr 19, 2017

nikomatsakis Apr 20, 2017

petrochenkov Apr 20, 2017

petrochenkov Apr 20, 2017

petrochenkov commented Apr 20, 2017

nikomatsakis commented Apr 22, 2017

bors commented Apr 22, 2017

bors commented Apr 22, 2017

syntax: Support parentheses around trait bounds #41077

syntax: Support parentheses around trait bounds #41077

Conversation

petrochenkov commented Apr 4, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nikomatsakis commented Apr 12, 2017

carols10cents commented Apr 17, 2017

nikomatsakis commented Apr 18, 2017

nikomatsakis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

petrochenkov commented Apr 20, 2017

nikomatsakis commented Apr 22, 2017

bors commented Apr 22, 2017

bors commented Apr 22, 2017

petrochenkov commented Apr 4, 2017 •

edited

Loading