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

Tracking issue for Reflect stabilization #27749

Closed
aturon opened this issue Aug 12, 2015 · 48 comments
Closed

Tracking issue for Reflect stabilization #27749

aturon opened this issue Aug 12, 2015 · 48 comments
Labels
B-unstable Blocker: Implemented in the nightly compiler and unstable. final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. T-lang Relevant to the language team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@aturon
Copy link
Member

aturon commented Aug 12, 2015

The Reflect trait is currently unstable. It was intended to narrow the use of things like Any for runtime introspection, to help retain parametricity properties. However, with impl specialization these properties would go away anyhow, so it's not clear how useful the trait would be.

Thus, stabilization should wait at least until specialization is settled.

cc @reem @nikomatsakis @pnkfelix

@aturon aturon added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. B-unstable Blocker: Implemented in the nightly compiler and unstable. labels Aug 12, 2015
@reem
Copy link
Contributor

reem commented Aug 13, 2015

I am in favor of it going away, I've had no use for it (and have heard of none) other than as necessary boilerplate whenever I want to do downcasting. I know of absolutely no types either in std or in the ecosystem which opt out of Reflect (and I can't really think of a good reason to do so).

@glaebhoerl
Copy link
Contributor

I know of absolutely no types either in std or in the ecosystem which opt out of Reflect (and I can't really think of a good reason to do so).

This is intentional. The point is precisely to prevent downcasting of opaque type parameters.

@nikomatsakis
Copy link
Contributor

I agree that the outcome of specialization is the key question.

@alexcrichton
Copy link
Member

Nominating for 1.6 discussion

@pnkfelix
Copy link
Member

pnkfelix commented Nov 4, 2015

@alexcrichton I do not understand, isn't the situation still :

Thus, stabilization should wait at least until specialization is settled.

(And, to my knowledge, specialiation is not settled?)

@alexcrichton
Copy link
Member

That was part of the discussion I'd like to have :)

With nonparametric dropck I wanted to check in on this and see if we could deprecate/remove, but if it still has to do with specialization then I think we'd just keep punting.

@aturon
Copy link
Member Author

aturon commented Nov 4, 2015

@alexcrichton Yeah, it's tied to specialization. The change to dropck was a conservative one that will make room for specialization but doesn't imply that we give up on parametricity.

@alexcrichton
Copy link
Member

Alex has been taught the error of his ways, so this issue is not moving into FCP this cycle.

@nrc nrc added I-nominated T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Aug 18, 2016
@nikomatsakis
Copy link
Contributor

nikomatsakis commented Aug 22, 2016

We discussed this in the @rust-lang/lang meeting. We would like to move this issue to final-comment period with the intention to deprecate and remove the Reflect trait. This represents a decision to move away from parametricity; we started down this road when accepting the specialization RFC (where there was much discussion). This is also relevant to mem::discriminant (rust-lang/rfcs#1696).

Note: This final-comment-period lasts for (roughly) the current release cycle, which began on Aug 18, 2016.

@nikomatsakis nikomatsakis added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed I-nominated labels Aug 22, 2016
@nikomatsakis
Copy link
Contributor

On the general topic of parametricity, there have been numerous comments. In an effort to summarize the conversation, I wanted to link to some of the most salient exchanges:

@nikomatsakis
Copy link
Contributor

Reviewing this thread did reveal one interesting thing to me: the opt-in proposal still relied on the Reflect trait being discussed here, but it made the trait into an implicitly present bound, much like Sized, as opposed to the current bound (i.e., one would have to write T: ?Reflect to avoid a T: Reflect bound). The intention was that it could be introduced backwards compatibly.

@nikomatsakis
Copy link
Contributor

nikomatsakis commented Aug 22, 2016

Also, @eddyb has pointed out a few times that the current Reflect trait has the interesting property that individual types can, in principle, "opt-out" of reflection, though that has no effect on specialization.

@eddyb
Copy link
Member

eddyb commented Aug 22, 2016

@nikomatsakis Do you mean specialization can "remove" the requirement? Because that's unsound (or rather, it would be for Send and Sync).

@nikomatsakis
Copy link
Contributor

@eddyb

Do you mean specialization can "remove" the requirement? Because that's unsound (or rather, it would be for Send and Sync).

What I meant is that the specialization RFC does not mention the Reflect trait -- hence one can "downcast" during monomorphization without regard for whether code has opted in or out of Reflect.

I am not sure of what relationship you see between Reflect and Send and Sync though, can you elaborate? Am I forgetting about something?

@eddyb
Copy link
Member

eddyb commented Aug 23, 2016

@nikomatsakis Ah, sure, I was talking about a Reflect bound, e.g. on Any - specialization shouldn't be able to work around the fact that I denied it for my type.
Now the usefulness of that feature is orthogonal, but it is there.

@withoutboats
Copy link
Contributor

This represents a decision to move away from parametricity; we started down this road when accepting the specialization RFC (where there was much discussion).

I'm curious what this means in the larger picture, not specific to this tracking issue.

@aturon
Copy link
Member Author

aturon commented Sep 2, 2016

@withoutboats In a nutshell, it means that Rust will not attempt to ensure traditional parametricity for type arguments (i.e. that generic code behaves "equivalently" when instantiated with different types). This is already formally the case due to the ability to get the size of a type parameter, but we're opening the door to observable, behavioral differences under different type arguments.

That means we will permit language features that let you write a function like:

fn print<T>(t: &T) { ... }

which e.g. prints its argument using Display if available, otherwise Debug if available, otherwise prints a generic message. Whereas in today's Rust, you might assume that such a function must behave more "uniformly" for all T (and in fact that it couldn't interact meaningfully with its argument).

The tradeoff is:

  • Benefit: additional expressiveness, of course. In particular, in the specialization thread I and others argued that this kind of "static reflection" is essential for reaching true zero-cost abstraction. A possible counter-argument: it could be treated as an unsafe feature, where marking as safe requires ensuring that there's no "observable" difference in behavior, just better performance. But of course there are use cases like print above where different behavior is the whole point. See the specialization thread for more.
  • Downside: the signature of a function tells you less about that function's behavior. That is, you can make certain assumptions about a signature like print in Rust today, e.g. that it cannot access anything "inside" its argument -- and that reasoning would be invalidated. Counter-argument: in almost all cases you need a contract for a function that goes beyond what the types say anyway; if this kind of lack of reflection is important, just make it an explicit part of the function's contract. In particular, the vast majority of generic functions will still behave parametrically, and those that don't are likely to be fairly obvious or otherwise marked.

Note that all of the above is specific to type parameters. Lifetime parameters, on the other hand, are intended to remain parametric -- e.g. you can't change the behavior of your function depending on whether a given lifetime is 'static or not. That's important for functions like Ref::map.

@withoutboats
Copy link
Contributor

withoutboats commented Sep 2, 2016

That means we will permit language features that let you write a function ... which e.g. prints its argument using Display if available, otherwise Debug if available, otherwise prints a generic message.

In my opinion, strict parametricity is not so important, but explicit, algorithmic reflection almost always is a band-aid over a wrong abstraction which results in increasingly unmaintainable spaghetti. Having automatically determined aparametricity during monomorphization (so specialization) is fine, in my opinion, but making explicit downcasting a normal and commonly used part of the language would be a very negative change. The fact that its so hard to do this is one of the language's key selling points to me.

That is, I'm comfortable with this:

default fn print<T>(t: &T) { ... }
default fn print<T: Debug>(t: &T) { ... }
default fn print<T: Display>(t: &T) { ... }
fn print<T: Debug + Display>(t: &T) { ... }

But I'm uncomfortable with something like this:

fn print<T>(t: &T) {
    if T::implements::<Display>() { ... }
    else if T::implements::<Debug>() { ... }
    else { ... }
} 

Partly this is an intuitive judgment, but some obvious advantages of the first:

  1. The compiler can automatically check your specialization hierarchy - you have to think explicitly about "what if T: Debug + Display?" and "what if T: !Debug + !Display?" or your code won't compile.
  2. No conditional thickets.

@naasking
Copy link

I admit I have no horse in this race since I haven't gotten to use Rust for any projects, but I do follow the theoretical side of Rust. This discussion of introducing typecase puzzles me, particularly because Rust already supports ad-hoc overloading via traits, so why introduce yet another way to overload, and via a mechanism that is more limited to boot, ie. typecase is closed where traits are open?

With impl specialization seemingly merged, zero-cost open overloading already seems available, so what's the real use for typecase, aside from reducing a little typing by avoiding trait and impls?

@eddyb
Copy link
Member

eddyb commented Oct 14, 2016

Any is dynamic, while impl specialization is fully statically dispatched.

@bluss
Copy link
Member

bluss commented Oct 14, 2016

You can already use TypeId for compile time ad hoc special casing depending on type.

@naasking
Copy link

@eddyb, if that was a reply to me, then:

  1. dynamic type checks seem to violate the desired zero-cost abstraction principle
  2. dynamic type class dispatch is possible in Haskell via existentials, so I can't imagine Rust isn't able to express something similar

@eddyb
Copy link
Member

eddyb commented Oct 14, 2016

Rust supporting dynamic multidispatch and specialization at the language level (2) would cause 1.
At this moment Any is a library construct based on a static TypeId primitive.
There is no dynamic type checking in the language, only barebones trait objects (existentials over Self).
You shouldn't use something like Any in Rust as anything other than a last resort.

@naasking
Copy link

Yes (2) implies (1). An if-chain discriminating on TypeId also implies (1), but at least a) you didn't introduce a new means of overloading, b) the fact that the behaviour is overloaded is now visible in the trait constraint, c) the specialization is now extensible to other types besides the "blessed" closed family of types.

What am I missing?

@eddyb
Copy link
Member

eddyb commented Oct 14, 2016

I don't understand, are you arguing against specialization as a whole?
You can use specialization to hide any trait constraint you want.
Including, uhm, Reflect itself, so someone can make their own unconstrained Any.

brson pushed a commit to brson/rust that referenced this issue Oct 19, 2016
@e-oz
Copy link

e-oz commented Nov 10, 2016

Will it affect rustc-serialize?

@bluss
Copy link
Member

bluss commented Nov 10, 2016

@e-oz Can you elaborate the question? Reflection in Rust allows some dynamic type checks, but it does not have any structural information like enumeration of struct fields or so, which I imagine could be what you mean?

@e-oz
Copy link

e-oz commented Nov 10, 2016

@bluss I didn't know it and thought rustc-serialize does use reflection. Thanks for clarification :)

@alexcrichton
Copy link
Member

Ah the deprecation here has hit stable, so this issue is resolved!

eddyb added a commit to eddyb/rust that referenced this issue Nov 11, 2016
…chton

vec: Write the .extend() specialization in cleaner style

As far as possible, use regular `default fn` specialization in favour of
ad-hoc conditionals.

No intentional functional change. Code quality was validated against the same
benchmarks that were used in initial trusted len development.

This change is prompted by taking impressions from
rust-lang#27749 (comment)
eddyb added a commit to eddyb/rust that referenced this issue Nov 12, 2016
…chton

vec: Write the .extend() specialization in cleaner style

As far as possible, use regular `default fn` specialization in favour of
ad-hoc conditionals.

No intentional functional change. Code quality was validated against the same
benchmarks that were used in initial trusted len development.

This change is prompted by taking impressions from
rust-lang#27749 (comment)
@est31
Copy link
Member

est31 commented Jan 14, 2017

I'm a bit confused. feature_gate.rs still lists reflect as "active", aka, non removed. Also, it lists this issue as its tracking issue, but this tracking issue claims that Reflect got removed. What to do?

@est31
Copy link
Member

est31 commented Jan 14, 2017

I'm asking because of issue #39059.

@durka
Copy link
Contributor

durka commented Jan 15, 2017

@est31 the trait itself was deprecated but not removed. As it was never stable, we can remove it in Rust 1.x. And it was deprecated in 1.14, so maybe we can consider removing it in 1.16.

But I think the feature gate stays "active" until then, as you can still mention Reflect if you put #![feature(reflect_marker)] #![allow(deprecated)] in your crate. Someone correct me if I'm wrong.

@withoutboats
Copy link
Contributor

If we're going to remove Reflect and the feature entirely (I support doing that) we should do a crater run to see what impact it has.

@est31 est31 mentioned this issue Jan 15, 2017
@est31
Copy link
Member

est31 commented Jan 15, 2017

@withoutboats I've created a PR to do such a crater run on: #39075

(I don't have any opinion on whether to remove Reflect or not).

@nikomatsakis
Copy link
Contributor

I started a crater run on @est31's PR.

@est31
Copy link
Member

est31 commented Jan 25, 2017

crater report by @brson on the PR above.

@nikomatsakis
Copy link
Contributor

I too am confused on why this issue is closed. I'm going to open it. =) I think we can remove Reflect altogether.

@nikomatsakis nikomatsakis reopened this Jan 25, 2017
bors added a commit that referenced this issue Jan 26, 2017
Remove Reflect

PR for removing the `Reflect` trait. Opened so that a crater run can be done for testing the impact: #27749 (comment)

Fixes #27749
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-unstable Blocker: Implemented in the nightly compiler and unstable. final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. T-lang Relevant to the language team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests