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

Rustc asks for Reflect, which is unstable, when it could be asking for Any which is stable #33807

Closed
tinco opened this issue May 23, 2016 · 18 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@tinco
Copy link
Contributor

tinco commented May 23, 2016

Hi,

I had a latenight coding session and ran into this error:

src/components/mod.rs:35:69: 35:83 error: the trait `core::marker::Reflect` is not implemented for the type `T` [E0277]
src/components/mod.rs:35    let component_list : &'static SharedMutex<T> = *component_list_any.downcast_ref().expect("Invalid type for components list");
                                                                                               ^~~~~~~~~~~~~~

Seems like an easy fix, add Reflect to the type constraint of the T, but that yields this exception:

src/components/mod.rs:12:5: 12:25 error: use of unstable library feature 'reflect_marker': requires RFC and more experience (see issue #27749)
src/components/mod.rs:12 use std::marker::Reflect;
                             ^~~~~~~~~~~~~~~~~~~~

That #27749 looks like it won't soon be solved and probably #31844 will supersede it in some way. So this advice from the compiler of adding Reflect is not very nice.

Perhaps we could have it recommend implementing Any instead? Or at least mention Any would do it as well? Most rookies won't know that Any is basically Reflect+'static.

Here's my forum question about this topic with the code that threw the error.

@durka
Copy link
Contributor

durka commented May 26, 2016

There is supposed to be a note:

rust/src/libcore/marker.rs

Lines 456 to 457 in 34fd686

#[rustc_on_unimplemented = "`{Self}` does not implement `Any`; \
ensure all type parameters are bounded by `Any`"]

But it's not triggering. It used to work on stable, but is broken on beta. cc @GuillaumeGomez

@GuillaumeGomez
Copy link
Member

@durka: @arielb1 removed my implementation. I'll try to fix this case.

@GuillaumeGomez GuillaumeGomez added the regression-from-stable-to-beta Performance or correctness regression from stable to beta. label May 26, 2016
@GuillaumeGomez
Copy link
Member

@tinco: Do you have a shorter code sample by any chance?

@sanxiyn sanxiyn added the A-diagnostics Area: Messages for errors, warnings, and lints label Jun 23, 2016
@jonas-schievink
Copy link
Contributor

Reproduction:

use std::any::Any;

fn log<T>(value: &T) {
    let value_any = value as &Any;

    // try to convert our value to a String.  If successful, we want to
    // output the String's length as well as its value.  If not, it's a
    // different type: just print it out unadorned.
    match value_any.downcast_ref::<String>() {
        Some(as_string) => {
        }
        None => {
        }
    }
}

fn main() {}
<anon>:4:21: 4:26 error: the trait bound `T: std::marker::Reflect` is not satisfied [E0277]
<anon>:4     let value_any = value as &Any;
                             ^~~~~
<anon>:4:21: 4:26 help: see the detailed explanation for E0277
<anon>:4:21: 4:26 help: consider adding a `where T: std::marker::Reflect` bound
<anon>:4:21: 4:26 note: required for the cast to the object type `std::any::Any + 'static`
error: aborting due to previous error

Looks like it does mention Any.

@GuillaumeGomez
Copy link
Member

That's what I got as well so I concluded it wasn't what he did. But maybe this is already solved in nightly?

@durka
Copy link
Contributor

durka commented Jun 27, 2016

It mentions Any but the help says to add a T: Reflect bound which is unstable and the subject of this issue. There's a rustc_on_unimplemented note about it that isn't getting printed (see my source link above).

@durka
Copy link
Contributor

durka commented Jun 27, 2016

This was tagged regression-from-stable-to-beta but that was a cycle ago so it is now a regression-from-stable-to-stable (albeit a quite minor one).

rustc 1.8.0 prints this in response to @jonas-schievink's code:

foo.rs:4:21: 4:26 error: the trait `core::marker::Reflect` is not implemented for the type `T` [E0277]
foo.rs:4     let value_any = value as &Any;
                             ^~~~~
foo.rs:4:21: 4:26 help: run `rustc --explain E0277` to see a detailed explanation
foo.rs:4:21: 4:26 note: `T` does not implement `Any`; ensure all type parameters are bounded by `Any`
foo.rs:4:21: 4:26 note: required for the cast to the object type `core::any::Any + 'static`
error: aborting due to previous error

The fifth line (first note) is not printed by 1.9.0 or later versions.

@GuillaumeGomez GuillaumeGomez added regression-from-stable-to-stable Performance or correctness regression from one stable version to another. and removed regression-from-stable-to-beta Performance or correctness regression from stable to beta. labels Jun 27, 2016
@GuillaumeGomez
Copy link
Member

@durka: I updated the tag.

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

nrc commented Aug 18, 2016

@rust-lang/compiler and/or @durka is this likely to get fixed or should we give up on this? Does seem a shame to live with this regression, but given that it's been on stable for a while (and Reflect will probably disappear anyway) perhaps it is not worth fixing?

@eddyb
Copy link
Member

eddyb commented Aug 18, 2016

@nrc It's good idea to track the underlying diagnostics regression (@jonathandturner may be interested).

@durka
Copy link
Contributor

durka commented Aug 18, 2016

Reflect is going to disappear but there is some underlying problem here with the #[rustc_on_unimplemented] getting dropped on the floor.

@nikomatsakis
Copy link
Contributor

Do we have any code that actually reproduces the problem?

@arielb1
Copy link
Contributor

arielb1 commented Aug 18, 2016

current error message:

error[E0277]: the trait bound `T: std::marker::Reflect` is not satisfied
 --> <anon>:6:21
  |
6 |     let value_any = value as &Any;
  |                     ^^^^^
  |
  = help: consider adding a `where T: std::marker::Reflect` bound
  = note: required because of the requirements on the impl of `std::any::Any` for `T`
  = note: required for the cast to the object type `std::any::Any + 'static`

@nikomatsakis
Copy link
Contributor

Conclusion for @rust-lang/compiler meeting: current behavior appears to be correct, so we believe problem is fixed, can call this I-needs-test.

@nikomatsakis
Copy link
Contributor

triage: P-low

@rust-highfive rust-highfive added P-low Low priority and removed I-nominated labels Aug 18, 2016
@nikomatsakis nikomatsakis added I-nominated E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. and removed P-low Low priority labels Aug 18, 2016
@nikomatsakis
Copy link
Contributor

Actually, this was misunderstood. The problem is that at some point we decided (perhaps incorrectly) that this kind of message we see now is higher priority that #[rustc_on_unimplemented] messages, and that is why we do not see the custom message anymore. I am not convinced this is correct, but it was deliberate.

@durka
Copy link
Contributor

durka commented Aug 18, 2016

OK, that's weird to me (why not print both), but if it was deliberate then this can be closed.

@brson
Copy link
Contributor

brson commented Sep 13, 2016

Closing per previous comments.

@brson brson closed this as completed Sep 13, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests