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

proc bounds and closure lifetimes are parsed incorrectly #10553

Closed
Kimundi opened this issue Nov 18, 2013 · 15 comments · Fixed by #13268
Closed

proc bounds and closure lifetimes are parsed incorrectly #10553

Kimundi opened this issue Nov 18, 2013 · 15 comments · Fixed by #13268
Milestone

Comments

@Kimundi
Copy link
Member

Kimundi commented Nov 18, 2013

<kimundi> If you have a function that takes a reference to a closure the lifetime specifier becomes confusing
<kimundi> fn foo(&|T| -> U), fn foo(&'a |T| -> U) and fn foo(&'a 'a |T| -> U) are all valid, and in the second one you can't tell which one of both the lifetimes is supposed to belong to
<nmatsakis> kimundi: this is parser bug, 'a |T| shouldn't parse
<nmatsakis> it should be |T|:'a

<nmatsakis> the parser may well be wrong
<nmatsakis> maybe got overlooked

<kimundi> well, if I change to that syntax I get compile errors
<kimundi> this works: impl<'a> StringPrefixMatch for 'a |$self_anon| -> bool
<kimundi> this doesn't: impl<'a> StringPrefixMatch for |$self_anon|:'a -> bool
<kimundi> I get a "'static is the only valid region allowed here" error

<nmatsakis> kimundi: I think it's just a parser bug
<nmatsakis> kimundi: yes, I can understand why that would occur
<kimundi> why?

<nmatsakis> kimundi: basically because some cleanup is needed
<nmatsakis> we are interpreting |...|:K with K as a list of bounds , which is correct,
<nmatsakis> but we currently haven't implemented support for arbitrary lifetimes in bounds,
<nmatsakis> which is wrong,
<nmatsakis> and instead we have special case treatment for closures,
<kimundi> Ah, so it only accepts the 'static bound there
<nmatsakis> which kind of fit ok with the old syntax,
<nmatsakis> but in the newer one we really should be just using general lifetimes in bounds
@Kimundi
Copy link
Member Author

Kimundi commented Nov 18, 2013

cc @nikomatsakis

@pnkfelix
Copy link
Member

pnkfelix commented Dec 2, 2013

cc me

@pnkfelix
Copy link
Member

Note in particular that attempting to make 'a |T| -> U a type expression leads to ambiguities when parsing a generic instantiation where you are trying to pass a closure type as the type parameter, like Foo<'a |T| -> U>, because the parser will eagerly treat the Foo<'a prefix as a lifetime parameter being passed to Foo, and thus it expects a comma next.

(An easy work-around for the type being is to use a type declaration like
type closure<'a, T, U> = 'a |T| -> U;
and then you can instantiate Foo on the type closure<'a, T, U>.)

@flaper87
Copy link
Contributor

cc @flaper87

@nikomatsakis
Copy link
Contributor

Another parser issue: something like proc:Send doesn't parse

@nikomatsakis
Copy link
Contributor

Nominating.

@pnkfelix
Copy link
Member

Assigning 1.0, P-backcompat-lang

@pnkfelix pnkfelix added this to the 1.0 milestone Feb 27, 2014
@emberian
Copy link
Member

cc me

@nikomatsakis
Copy link
Contributor

For reference, I believe the syntax ought to be:

T = '|' ( id : T ) * '|' [ ':' K ] [ '->' T ]
  | 'proc' '(' ( id : T ) * ')' [ ':' K ] [ '->' T ]
  | ...
K = TraitReference | Lifetime 

@nikomatsakis
Copy link
Contributor

Except of course that parameter lists are comma-separated

@alexcrichton
Copy link
Member

I'm assuming that with that syntax you can also have multiple K values +-separate? Would I be correct in these translations:

'a || => ||: 'a
||: 'static => ||: 'static
'a ||: Send => ||: 'a + Send

@nikomatsakis
Copy link
Contributor

yes, except that ||: 'a + Send is kind of inherently contradictory. I imagine Send should imply 'static, but I'm not quite sure what needs to be done in the code to make that happen. cc #10296

@alexcrichton
Copy link
Member

Hm, so one thing I just realized is that the lifetimes don't necessarily translate to a bound on the environment, but they could also serve to connect arguments together. For example:

||: 'a // environment is bounded by 'a
<'a> |&'a Foo| -> &'a int // 'a used to connect parameters

Currently, the lifetime introduction syntax (<'a>) causes problems in generics because << is one token instead of two. This could be fixed in the parser, but I wanted to ensure that this was the syntax that we wanted before modifying the parser.

@nikomatsakis
Copy link
Contributor

@alexcrichton right, I expect that <'a> can be used in the way you show. You can of course combine these: <'a> |&'a Foo|: 'b -> &'a int

@nikomatsakis
Copy link
Contributor

the problem of parsing << incorrectly is something we also have for && and >> -- we've hacked it for >>

alexcrichton added a commit to alexcrichton/rust that referenced this issue Apr 4, 2014
In summary these are some example transitions this change makes:

    'a ||       => ||: 'a
    proc:Send() => proc():Send

The intended syntax for closures is to put the lifetime bound not at the front
but rather in the list of bounds. Currently there is no official support in the
AST for bounds that are not 'static, so this case is currently specially handled
in the parser to desugar to what the AST is expecting. Additionally, this moves
the bounds on procedures to the correct position, which is after the argument
list.

The current grammar for closures and procedures is:

    procedure := 'proc' [ '<' lifetime-list '>' ] '(' arg-list ')'
                        [ ':' bound-list ] [ '->' type ]
    closure := [ 'unsafe' ] ['<' lifetime-list '>' ] '|' arg-list '|'
                        [ ':' bound-list ] [ '->' type ]
    lifetime-list := lifetime | lifetime ',' lifetime-list
    arg-list := ident ':' type | ident ':' type ',' arg-list
    bound-list := bound | bound '+' bound-list
    bound := path | lifetime

This does not currently handle the << ambiguity in `Option<<'a>||>`, I am
deferring that to a later patch. Additionally, this removes the support for the
obsolete syntaxes of ~fn and &fn.

Closes rust-lang#10553
Closes rust-lang#10767
Closes rust-lang#11209
Closes rust-lang#11210
Closes rust-lang#11211
bors added a commit that referenced this issue Apr 5, 2014
In summary these are some example transitions this change makes:

    'a ||       => ||: 'a
    proc:Send() => proc():Send

The intended syntax for closures is to put the lifetime bound not at the front
but rather in the list of bounds. Currently there is no official support in the
AST for bounds that are not 'static, so this case is currently specially handled
in the parser to desugar to what the AST is expecting. Additionally, this moves
the bounds on procedures to the correct position, which is after the argument
list.

The current grammar for closures and procedures is:

    procedure := 'proc' [ '<' lifetime-list '>' ] '(' arg-list ')'
                        [ ':' bound-list ] [ '->' type ]
    closure := [ 'unsafe' ] ['<' lifetime-list '>' ] '|' arg-list '|'
                        [ ':' bound-list ] [ '->' type ]
    lifetime-list := lifetime | lifetime ',' lifetime-list
    arg-list := ident ':' type | ident ':' type ',' arg-list
    bound-list := bound | bound '+' bound-list
    bound := path | lifetime

This does not currently handle the << ambiguity in `Option<<'a>||>`, I am
deferring that to a later patch. Additionally, this removes the support for the
obsolete syntaxes of ~fn and &fn.

Closes #10553
Closes #10767 
Closes #11209
Closes #11210
Closes #11211
bors added a commit that referenced this issue Apr 6, 2014
In summary these are some example transitions this change makes:

    'a ||       => ||: 'a
    proc:Send() => proc():Send

The intended syntax for closures is to put the lifetime bound not at the front
but rather in the list of bounds. Currently there is no official support in the
AST for bounds that are not 'static, so this case is currently specially handled
in the parser to desugar to what the AST is expecting. Additionally, this moves
the bounds on procedures to the correct position, which is after the argument
list.

The current grammar for closures and procedures is:

    procedure := 'proc' [ '<' lifetime-list '>' ] '(' arg-list ')'
                        [ ':' bound-list ] [ '->' type ]
    closure := [ 'unsafe' ] ['<' lifetime-list '>' ] '|' arg-list '|'
                        [ ':' bound-list ] [ '->' type ]
    lifetime-list := lifetime | lifetime ',' lifetime-list
    arg-list := ident ':' type | ident ':' type ',' arg-list
    bound-list := bound | bound '+' bound-list
    bound := path | lifetime

This does not currently handle the << ambiguity in `Option<<'a>||>`, I am
deferring that to a later patch. Additionally, this removes the support for the
obsolete syntaxes of ~fn and &fn.

Closes #10553
Closes #10767 
Closes #11209
Closes #11210
Closes #11211
flip1995 pushed a commit to flip1995/rust that referenced this issue Apr 6, 2023
In uninit checking, add fallback for polymorphic types

After rust-lang#10520, we always assumed that polymorphic types do not allow to be left uninitialized. But we can do better, by peeking into polymorphic types and adding a few special cases for going through tuples, arrays (because the length may be polymorphic) and blanket allowing all unions (like MaybeUninit).

fixes rust-lang#10551

changelog: [uninit_vec]: fix false positive for polymorphic types
changelog: [uninit_assumed_init]: fix false positive for polymorphic types
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 a pull request may close this issue.

6 participants