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

ICE due to ambiguity between impl and where-clause #22110

Closed
nikomatsakis opened this issue Feb 9, 2015 · 6 comments
Closed

ICE due to ambiguity between impl and where-clause #22110

nikomatsakis opened this issue Feb 9, 2015 · 6 comments
Labels
A-traits Area: Trait system I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️

Comments

@nikomatsakis
Copy link
Contributor

Example:

use std::vec::Vec;
use std::iter::Iterator;
use std::slice::{AsSlice, Split, SliceExt};

pub struct Path {
    repr: Vec<u8>, // assumed to never be empty or contain NULs
}

pub type Components<'a> = Split<'a, u8, fn(&u8) -> bool>;

pub fn is_sep_byte(u: &u8) -> bool {
    true
}

impl Path {
    fn components<'a>(&'a self) -> Components<'a>
        where fn(&u8) -> bool : FnMut(&u8) -> bool
    {
        let v = &self.repr[1..];
        let is_sep_byte: fn(&u8) -> bool = is_sep_byte; // coerce to fn ptr
        let mut ret = v.split(is_sep_byte);
        if v.is_empty() {
            ret.next();
        }
        ret
    }
}

fn main() {
}

fails with



<anon>:21:25: 21:43 error: internal compiler error: coherence failed to report ambiguity: cannot locate the impl of the trait `for<'r> core::ops::FnMut<(&'r u8,)>` for the type `fn(&u8) -> bool`
<anon>:21         let mut ret = v.split(is_sep_byte);
                                  ^~~~~~~~~~~~~~~~~~
note: the compiler unexpectedly panicked. this is a bug.
note: we would appreciate a bug report: http://doc.rust-lang.org/complement-bugreport.html
note: run with `RUST_BACKTRACE=1` for a backtrace
thread 'rustc' panicked at 'Box<Any>', /home/rustbuild/src/rust-buildbot/slave/nightly-dist-rustc-linux/build/src/libsyntax/diagnostic.rs:129


playpen: application terminated with error code 101
Program ended.

This is because the compiler considers both the bridging impl from Fn to FnMut is an option and the where-clause to be valid options. Choosing between where-clauses and impls seems to be tricky.

At minimum, the code that checks for overlap between the where-clause and the impl seems to have the subtyping relationship backwards. However, I am not sure whether it's a good idea to check anything, or maybe better to simply always prefer where-clauses.

@nikomatsakis nikomatsakis added the A-traits Area: Trait system label Feb 9, 2015
@nikomatsakis
Copy link
Contributor Author

A more standalone version of that test:

use std::vec::Vec;
use std::iter::Iterator;
use std::slice::{AsSlice, Split, SliceExt};

struct Path;

trait Foo<A> {
    fn foo(&self, a: A);
}

impl<A,F:Fn(A)> Foo<A> for F {
    fn foo(&self, a: A) { }
}

fn baz<F:for<'a> Foo<(&'a Path,)>>(f: F) { }

impl Path {
    fn components<T>(&self, t: fn(&Path))
        where fn(&Path) : for<'a> Foo<(&'a Path,)>,
    {
        baz(t)
    }
}

fn main() {
}

@nikomatsakis
Copy link
Contributor Author

That version only works because we don't seem to be enforcing the rules against where-clauses with no type parameters fully. This alternate version also fails to compile but obeys those rules:

use std::vec::Vec;
use std::iter::Iterator;
use std::slice::{AsSlice, Split, SliceExt};

struct Path;

trait Foo<A> {
    fn foo(&self, a: A);
}

impl<A,F:Fn(A)> Foo<A> for F {
    fn foo(&self, a: A) { }
}

fn baz<A,F:for<'a> Foo<(&'a A,)>>(f: F) { }

fn components<T,A>(t: fn(&A))
    where fn(&A) : for<'a> Foo<(&'a A,)>,
{
    baz(t)
}

fn main() {
}

but the error is different:

traits-issue-22110.rs:34:5: 34:8 error: unable to infer enough type information about `_`; type annotations required [E0282]

@nikomatsakis
Copy link
Contributor Author

A related example:

use std::slice::Iter;

fn foo<'a,T>(v: Iter<'a,T>) -> Option<T>
    where Iter<'a,T> : Iterator
{
    v.next()
}

fn main() { }

Here the explicit where clause matches, interfering with later attempts to project the type Item, and hence we get:

<anon>:6:5: 6:13 error: mismatched types:
 expected `core::option::Option<T>`,
    found `core::option::Option<<core::slice::Iter<'a, T> as core::iter::Iterator>::Item>`
(expected type parameter,
    found associated type) [E0308]
<anon>:6     v.next()
             ^~~~~~~~
error: aborting due to previous error
playpen: application terminated with error code 101

@jdm jdm added the I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ label Feb 9, 2015
@jroesch
Copy link
Member

jroesch commented Feb 9, 2015

cc me

1 similar comment
@ghost
Copy link

ghost commented Feb 9, 2015

cc me

@nikomatsakis
Copy link
Contributor Author

The fix in #22407 is simply to always allow where clauses to override impls. This seems like a conservative step -- everything where clauses tell you must be derived from an impl, after all. In he future, I'd like to improve the engine so that it can take advantage of impls as well to possibly glean more information than the where-clause is telling us, but it's tricky, because impls have preconditions.

bors added a commit that referenced this issue Feb 18, 2015
…soc-types, r=nikomatsakis

Take 2. This PR includes a bunch of refactoring that was part of an experimental branch implementing [implied bounds]. That particular idea isn't ready to go yet, but the refactoring proved useful for fixing #22246. The implied bounds branch also exposed #22110 so a simple fix for that is included here. I still think some more refactoring would be a good idea here -- in particular I think most of the code in wf.rs is kind of duplicating the logic in implicator and should go, but I decided to post this PR and call it a day before diving into that. I'll write a bit more details about the solutions I adopted in the various bugs. I patched the two issues I was concerned about, which was the handling of supertraits and HRTB (the latter turned out to be fine, so I added a comment explaining why.)

r? @pnkfelix (for now, anyway)
cc @aturon 

[implied bounds]: http://smallcultfollowing.com/babysteps/blog/2014/07/06/implied-bounds/
alexcrichton added a commit to alexcrichton/rust that referenced this issue Feb 19, 2015
…imes-of-assoc-types

Take 2. This PR includes a bunch of refactoring that was part of an experimental branch implementing [implied bounds]. That particular idea isn't ready to go yet, but the refactoring proved useful for fixing rust-lang#22246. The implied bounds branch also exposed rust-lang#22110 so a simple fix for that is included here. I still think some more refactoring would be a good idea here -- in particular I think most of the code in wf.rs is kind of duplicating the logic in implicator and should go, but I decided to post this PR and call it a day before diving into that. I'll write a bit more details about the solutions I adopted in the various bugs. I patched the two issues I was concerned about, which was the handling of supertraits and HRTB (the latter turned out to be fine, so I added a comment explaining why.)

r? @pnkfelix (for now, anyway)
cc @aturon

[implied bounds]: http://smallcultfollowing.com/babysteps/blog/2014/07/06/implied-bounds/
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-traits Area: Trait system I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️
Projects
None yet
Development

No branches or pull requests

3 participants