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

Nightly regression. #32301

Closed
dpc opened this Issue Mar 17, 2016 · 5 comments

Comments

Projects
None yet
5 participants
@dpc
Copy link
Contributor

dpc commented Mar 17, 2016

Mioco works on nightly, but recent nightly (around two day ago) does not compile mioco anymore.

I have minimized code to:

https://gist.github.com/dpc/a6bd306b8187651ec700

This code compiles on stable, but breaks on nightly, so I believe this is a regression.

dpc@futex ~/t/mioco-break (master) [I]> multirust override nightly ; make
multirust: using existing install for 'nightly'
multirust: override toolchain for '/home/dpc/tmp/mioco-break' set to 'nightly'
cargo build
   Compiling mioco-break v0.1.0 (file:///home/dpc/tmp/mioco-break)
udp.rs:11:5: 13:6 error: duplicate definitions with name `try_read`: [E0201]
udp.rs:11     pub fn try_read(&mut self, buf: &mut [u8]) -> io::Result<Option<(usize, SocketAddr)>> {
udp.rs:12         Ok(None)
udp.rs:13     }
udp.rs:11:5: 13:6 help: run `rustc --explain E0201` to see a detailed explanation
lib.rs:14:5: 16:6 note: conflicting definition is here:
lib.rs:14     pub fn try_read(&mut self, buf: &mut [u8]) -> io::Result<Option<usize>> {
lib.rs:15         Ok(None)
lib.rs:16     }
error: aborting due to previous error
       error Could not compile `mioco-break`.

To learn more, run the command again with --verbose.
Makefile:2: recipe for target 'all' failed
make: *** [all] Error 101
dpc@futex ~/t/mioco-break (master) [2] [I]> multirust override stable ; make
multirust: using existing install for 'stable'
multirust: override toolchain for '/home/dpc/tmp/mioco-break' set to 'stable'
cargo build
   Compiling mioco-break v0.1.0 (file:///home/dpc/tmp/mioco-break)
udp.rs:11:5: 13:6 warning: method is never used: `try_read`, #[warn(dead_code)] on by default
udp.rs:11     pub fn try_read(&mut self, buf: &mut [u8]) -> io::Result<Option<(usize, SocketAddr)>> {
udp.rs:12         Ok(None)
udp.rs:13     }
udp.rs:11:32: 11:35 warning: unused variable: `buf`, #[warn(unused_variables)] on by default
udp.rs:11     pub fn try_read(&mut self, buf: &mut [u8]) -> io::Result<Option<(usize, SocketAddr)>> {
                                         ^~~
lib.rs:14:32: 14:35 warning: unused variable: `buf`, #[warn(unused_variables)] on by default
lib.rs:14     pub fn try_read(&mut self, buf: &mut [u8]) -> io::Result<Option<usize>> {
                                         ^~~
@dpc

This comment has been minimized.

Copy link
Contributor Author

dpc commented Mar 17, 2016

Just to comment on the code: mio::udp::UdpSocket does not implement mio::Evented so impl<MT> MioAdapter<MT> where MT: mio::Evented + mio::TryRead + 'static should not be applied.

dpc added a commit to dpc/mioco.pre-0.9 that referenced this issue Mar 17, 2016

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Mar 17, 2016

The problem here is that we started applying the coherence rules to inherent impls -- the fact that we did not do so before was a bug. The coherence rules are somewhat more restrictive about negative reasoning. For example here, you are not allowed to rely on the fact that UdpSocket does not implement Evented because --- in principle, at least --- mio might add an impl of Evented for UdpSocket and that is not supposed to be a breaking change. (That is, in general, adding an impl of a trait for a type is not a breaking change -- nothing specific to mio). But it would in fact break your code given those two impls.

We probably ought to have had a warning period for this change, however.

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Mar 17, 2016

cc @aturon

@nagisa

This comment has been minimized.

Copy link
Contributor

nagisa commented Mar 17, 2016

Duplicate of #32247 I think?

@aturon

This comment has been minimized.

Copy link
Member

aturon commented Mar 17, 2016

Yes, this is effectively a dupe. I'm planning to land a change ASAP to produce a warning instead of an error for now, though we (at the moment) intend to go forward with this change.

@aturon aturon closed this Mar 17, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.