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

Refactoring in support of higher-ranked traitbounds #18743

Merged
merged 5 commits into from
Nov 9, 2014

Conversation

nikomatsakis
Copy link
Contributor

Various miscellaneous changes pushing towards HRTB support:

  1. Update parser and adjust ast to support for<'a,'b> syntax, both in closures and trait bounds. Warn on the old syntax (not error, for stage0).
  2. Refactor TyTrait representation to include a TraitRef.
  3. Purge once_fns feature gate and once keyword.

r? @pcwalton

This is a [breaking-change]:

  • The once_fns feature is now officially deprecated. Rewrite using normal closures or unboxed closures.
  • The new for-based syntax now issues warnings (but not yet errors):
    • fn<'a>(T) -> U becomes for<'a> fn(T) -> U
    • <'a> |T| -> U becomes for<'a> |T| -> U

@rust-highfive
Copy link
Collaborator

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!

lifetime_defs
/// Parses `[ 'for' '<' lifetime_defs '>' ]'
fn parse_legacy_lifetime_defs(&mut self,
lifetime_defs: Vec<ast::LifetimeDef>)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: weird indentation

@pcwalton
Copy link
Contributor

pcwalton commented Nov 7, 2014

r=me with the nit

@nikomatsakis
Copy link
Contributor Author

@pcwalton added one more commit, 9be076d, which purges once_fns

…naturally carries over to object types.

I wanted to embed an `Rc<TraitRef>`, but I was foiled by the current
static rules, which prohibit non-Sync values from being stored in
static locations. This means that the constants for `ty_int` and so
forth cannot be initialized.
@nikomatsakis
Copy link
Contributor Author

rebased, merged a bit

@pcwalton
Copy link
Contributor

pcwalton commented Nov 7, 2014

r=me

bors added a commit that referenced this pull request Nov 9, 2014
Various miscellaneous changes pushing towards HRTB support:

1. Update parser and adjust ast to support `for<'a,'b>` syntax, both in closures and trait bounds. Warn on the old syntax (not error, for stage0).
2. Refactor TyTrait representation to include a TraitRef.
3. Purge `once_fns` feature gate and `once` keyword.

r? @pcwalton 

This is a [breaking-change]:

- The `once_fns` feature is now officially deprecated. Rewrite using normal closures or unboxed closures.
- The new `for`-based syntax now issues warnings (but not yet errors):
  - `fn<'a>(T) -> U` becomes `for<'a> fn(T) -> U`
  - `<'a> |T| -> U` becomes `for<'a> |T| -> U`
@bors bors closed this Nov 9, 2014
@bors bors merged commit cf4e53e into rust-lang:master Nov 9, 2014
@pnkfelix
Copy link
Member

@nikomatsakis I only noticed now that rust-lang/rfcs#387 only discusses changing the syntax for fn types, but not for fn definitions themselves.

Concrete example:

fn bar1(g: fn<'a>(x: &'a int) -> &'a int) { g(&3i); }
//           ^~~~
//
// Definition of `bar1` now causes rustc to issue a deprecation warning.

fn bar2(g: for<'a> fn(x: &'a int) -> &'a int) { g(&3i); }
//         ^~~~~~~
//
// You need to use this syntax instead.

// ... BUT ...

fn foo1<'a>(x: &'a int) -> &'a int { x }
//     ^~~~
// This is the only syntax accepted for fn definition forms...

for<'a> fn foo2(x: &'a int) -> &'a int { x } /*
^~~~~~~                                       *
                                              *
...because this syntax is rejected by rustc.  *
                                              */

Is this a problem, in terms of introducing (unnecessary?) deviation between the type and term forms?

@pnkfelix
Copy link
Member

( adding reference to the HRTB tracking issue: #18639 )

@nikomatsakis nikomatsakis deleted the hrtb-refactor-2 branch March 30, 2016 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants