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

Autoderef does not find methods on traits in scope #13264

Closed
jdm opened this issue Apr 2, 2014 · 7 comments · Fixed by #19780
Closed

Autoderef does not find methods on traits in scope #13264

jdm opened this issue Apr 2, 2014 · 7 comments · Fixed by #19780
Labels
E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added.

Comments

@jdm
Copy link
Contributor

jdm commented Apr 2, 2014

Testcase: https://gist.github.com/jdm/9936609

Note how AddChild is resolved, since it's defined in an impl for JSRef itself, whereas RemoveChild is not resolved since it's a trait method.

RUST_LOG=rustc::middle::typeck::check::method output: https://gist.github.com/jdm/9936635

@alexcrichton
Copy link
Member

Compiling your example, I get:

<anon>:61:5: 61:23 error: this function takes 1 parameter but 0 parameters were supplied
<anon>:61     root.RemoveChild();
              ^~~~~~~~~~~~~~~~~~
<anon>:62:5: 62:24 error: this function takes 1 parameter but 0 parameters were supplied
<anon>:62     jsref.RemoveChild();
              ^~~~~~~~~~~~~~~~~~~
error: aborting due to 2 previous errors

Which I believe is working as intended. Currently inherent methods shadow trait methods.

@jdm
Copy link
Contributor Author

jdm commented Apr 2, 2014

@alexcrichton If I comment out the Deref impl, it compiles without issue.

@alexcrichton
Copy link
Member

Ah, don't mind me, I think I see what's going on.

@jdm
Copy link
Contributor Author

jdm commented Apr 2, 2014

[11:44:27] <nmatsakis> jdm: the reason you're seeing an error there is because we always favor inherent methods over trait methods
[11:44:31] <nmatsakis> jdm: currently, at least.

@nikomatsakis
Copy link
Contributor

The reason for this rule is to permit impls like impl Trait for ~Trait { ... }, since otherwise they result in infinite recursion cycles.

@ghost
Copy link

ghost commented Nov 11, 2014

Updated test case:

struct Root {
    jsref: JSRef
}

impl Deref<JSRef> for Root {
    fn deref<'a>(&'a self) -> &'a JSRef {
        &self.jsref
    }
}

struct JSRef {
    node: *const Node
}

impl Deref<Node> for JSRef {
    fn deref<'a>(&'a self) -> &'a Node {
        self.get()
    }
}

trait INode {
    fn RemoveChild(&self);
}

impl INode for JSRef {
    fn RemoveChild(&self) {
        self.get().RemoveChild(0)
    }
}

impl JSRef {
    fn AddChild(&self) {
        self.get().AddChild(0);
    }

    fn get<'a>(&'a self) -> &'a Node {
        unsafe {
            &*self.node
        }
    }
}

struct Node;

impl Node {
    fn RemoveChild(&self, _a: uint) {
    }

    fn AddChild(&self, _a: uint) {
    }
}

fn main() {
    let n = Node;
    let jsref = JSRef { node: &n };
    let root = Root { jsref: jsref };

    root.AddChild();
    jsref.AddChild();

    root.RemoveChild();
    jsref.RemoveChild();
}

E-needstest.

@ghost ghost added the E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. label Nov 11, 2014
@nikomatsakis
Copy link
Contributor

For more context: we changed the priorities so that inherent methods only override extension methods at a given autoderef level, which I think is close enough to the behavior that @jdm wanted for this test case.

bors added a commit that referenced this issue Dec 18, 2014
Closes #5988.
Closes #10176.
Closes #10456.
Closes #12744.
Closes #13264.
Closes #13324.
Closes #14182.
Closes #15381.
Closes #15444.
Closes #15480.
Closes #15756.
Closes #16822.
Closes #16966.
Closes #17351.
Closes #17503.
Closes #17545.
Closes #17771.
Closes #17816.
Closes #17897.
Closes #17905.
Closes #18188.
Closes #18232.
Closes #18345.
Closes #18389.
Closes #18400.
Closes #18502.
Closes #18611.
Closes #18783.
Closes #19009.
Closes #19081.
Closes #19098.
Closes #19127.
Closes #19135.
matthiaskrgr pushed a commit to matthiaskrgr/rust that referenced this issue Oct 11, 2022
…ykril

Ensure at least one trait bound in `TyKind::DynTy`

One would expect `TyKind::DynTy` to have at least one trait bound, but we may produce a dyn type with no trait bounds at all. This patch prevents it by returning `TyKind::Error` in such cases.

An "empty" dyn type would have caused panic during method resolution without rust-lang#13257. Although already fixed, I think an invariant to never produce such types would help prevent similar problems in the future.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants