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

Implement unique types per fn item, rather than having all fn items have fn pointer type #19891

Merged
merged 13 commits into from
Dec 23, 2014

Conversation

nikomatsakis
Copy link
Contributor

This is part of the coercion RFC but also makes sense given unboxed closures. The idea is that every fn item has its own type. This type can be coerced to a fn pointer but is not a fn pointer. This PR does almost all the end-user visible changes, but leaves one thing undone: the representation of an instance of a "fn item type" is still a fn pointer at runtime. A later PR will have to change them to zero-sized values (I'll open an isssue on this).

This is a [breaking-change]. In general, the coercion rules ensure that code which used to take a fn() type continus to work, but there are some patterns that need to be changed to insert explicit coercions.

Example 1:

If/match arms sometimes require explicit types now:

fn foo(x: int) -> int { x }
fn bar(x: int) -> int { x + 1}

// old:
// let x = if some_cond { foo } else { bar };

// new:
let x: fn(int) -> int = if some_cond { foo } else { bar } // coerce to a fn pointer

Example 2:

Generic functions are another place where coercions often fail to trigger. Example:

fn foo(x: int) -> int { x }

fn optional() -> Option<fn(int) -> int> {
    // Old:
    // Some(foo) // here the type argument on `Some` is inferred to be the fn item type

    // New:
    Some(foo as fn(int) -> int)
}

This one may or may not get fixed going forward; inserting coercions in cases like these involves subtle changes to program semantics that may not be appropriate.

r? @nick29581

@nrc nrc self-assigned this Dec 16, 2014

self.unpack_actual_value(b, |sty_b| {
debug!("coerce_from_bare_fn(a={}, b={})",
Copy link
Member

Choose a reason for hiding this comment

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

nit - debug text needs changing

@nrc
Copy link
Member

nrc commented Dec 16, 2014

r=me with nits fixed

@nikomatsakis
Copy link
Contributor Author

@nick29581 I was thinking, perhaps we should have special type rules for this case so that we permit if cond { foo } else { bar }, basically (and, presumably match). Still, that wouldn't affect the majority of cases, so maybe it's pointless. And it introduces a kind of third category of coercion I guess.

@nikomatsakis
Copy link
Contributor Author

Re-examining the fallout, it's clear that the most useful change would be to propagate coercions through Some and friends.

type of a fn item or a fn pointer, which are now differentiated.
Introduce coercion from fn item to fn pointer.
cannot use an `as` expression to coerce, so I used a one-off function
instead (this is a no-op in stage0, but in stage1+ it triggers
coercion from the fn pointer to the fn item type).
post-unboxed-closure-conversion. This requires a fair amount of
annoying coercions because all the `map` etc types are defined
generically over the `F`, so the automatic coercions don't propagate;
this is compounded by the need to use `let` and not `as` due to
stage0. That said, this pattern is to a large extent temporary and
unusual.
…cion

changes, and also stop printing semi-useless inference by-products.
alexcrichton added a commit to alexcrichton/rust that referenced this pull request Dec 22, 2014
Conflicts:
	src/libcore/str.rs
	src/librustc_trans/trans/closure.rs
	src/librustc_typeck/collect.rs
	src/libstd/path/posix.rs
	src/libstd/path/windows.rs
@bors bors merged commit 763152b into rust-lang:master Dec 23, 2014
aturon added a commit to steveklabnik/rustbook that referenced this pull request Dec 29, 2014
@nikomatsakis nikomatsakis deleted the unique-fn-types-3 branch March 30, 2016 16:17
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.

3 participants