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

complete default methods for traits #2794

Closed
lkuper opened this issue Jul 3, 2012 · 18 comments
Closed

complete default methods for traits #2794

lkuper opened this issue Jul 3, 2012 · 18 comments
Labels
A-type-system Area: Type system C-enhancement Category: An issue proposing an enhancement or a PR with one. metabug Issues about issues themselves ("bugs about bugs")

Comments

@lkuper
Copy link
Contributor

lkuper commented Jul 3, 2012

Implement the first two parts of this proposal (the third part, dealing with instance coherence, is orthogonal).

@lkuper
Copy link
Contributor Author

lkuper commented Jul 13, 2012

fc9c4c3 is the beginning of support for default methods in traits.

@lkuper
Copy link
Contributor Author

lkuper commented Aug 9, 2012

Default methods get through typeck as of 293f371.

@lkuper
Copy link
Contributor Author

lkuper commented Aug 31, 2012

Per IRC discussions, it seems that a big part of the remaining work (possibly the big part) is monomorphization. This is now @msullivan's bug, since I'm back at school and, alas, unlikely to work on it any time soon, and he's better equipped to deal with it than I am. :)

@catamorphism
Copy link
Contributor

One specific thing that's not finished is that default methods don't work cross-crate.

catamorphism added a commit that referenced this issue Sep 7, 2012
…rate

But note that default methods still don't work cross-crate (see #2794) --
this just makes it so that when a method is missing in a cross-crate impl,
the right error message gets printed.

Closes #3344
@burg
Copy link

burg commented Sep 10, 2012

Default methods are important for Servo (layout, display list). I will deliver a Free Beverage to whomever fixes this (two beverages if documentation written!).

@nikomatsakis
Copy link
Contributor

It would be very useful to have a "compiler_context" trait or some such that looks kind of like:

trait compiler_context {
    fn tcx() -> ty::ctxt;

    fn ty_to_str(t: ty::t) -> ~str { ty_to_str(self.tcx(), t) }
    fn expr_to_str(expr: @ast::expr) -> ~str { fmt!("expr(%?: %s)", expr.id, expr_to_str(expr, self.tcx().sess.intr()) }
    ...
}

@nikomatsakis
Copy link
Contributor

This is basically blocked and/or possibly a subpart of #3446

@nikomatsakis
Copy link
Contributor

Deferring to 0.5

@catamorphism
Copy link
Contributor

I think this is done; file more specific bugs if necessary.

@lkuper
Copy link
Contributor Author

lkuper commented Dec 7, 2012

Huzzah!

@catamorphism
Copy link
Contributor

@lkuper I spoke a bit too soon, but still, there will be separate more specific bugs for any remaining issues :-)

@graydon
Copy link
Contributor

graydon commented Jun 12, 2013

Re-opening and promoting to metabug. Sub-bugs include: #4099 #4396 #4102 #4103 #4377 #4350 #3694

No idea how much work there is to do here, but it needs to work. We've been stumbling over the absence of this feature in library work all year, keep deferring fixing due to "higher priority" work.

@graydon graydon reopened this Jun 12, 2013
@msullivan
Copy link
Contributor

About to file pull request that fixes #4099, #4350.

@catamorphism
Copy link
Contributor

@msullivan - where does this stand for 0.7?

@brson
Copy link
Contributor

brson commented Jul 3, 2013

Bumping from 0.7

@msullivan
Copy link
Contributor

@catamorphism - They are a lot more solid than they used to be, and could /maybe/ be taken out from behind the flag. Still a bunch of known problems, though.

@msullivan
Copy link
Contributor

This is basically done and working. Couple minor problems, but they are solidly usable.

@lkuper
Copy link
Contributor Author

lkuper commented Aug 24, 2013

Serious round of applause for @msullivan and everyone else who's been working on this over the last year.

RalfJung pushed a commit to RalfJung/rust that referenced this issue Feb 26, 2023
celinval pushed a commit to celinval/rust-dev that referenced this issue Jun 4, 2024
… also include ZSTs (rust-lang#2794)

This change considers another case for the code generation of scalar data.
In particular, when the expected type is an ADT, we previously
considered two cases: either there is no field or there is one. But in
cases where the ADT includes a ZST, the ADT will contain multiple
fields: one associated to the scalar data, and other fields associated
to the ZSTs. In those cases, we ended up crashing as in rust-lang#2680:

```rust
error: internal compiler error: Kani unexpectedly panicked at panicked at cprover_bindings/src/goto_program/expr.rs:804:9:
                                assertion failed: `(left == right)`
                                  left: `2`,
                                 right: `1`: Error in struct_expr; mismatch in number of fields and values.
                                    StructTag("tag-_161424092592971241517604180463813633314")
                                    [Expr { value: IntConstant(0), typ: Unsignedbv { width: 8 }, location: None, size_of_annotation: None }].

thread 'rustc' panicked at cprover_bindings/src/goto_program/expr.rs:804:9:
assertion failed: `(left == right)`
  left: `2`,
 right: `1`: Error in struct_expr; mismatch in number of fields and values.
	StructTag("tag-_161424092592971241517604180463813633314")
	[Expr { value: IntConstant(0), typ: Unsignedbv { width: 8 }, location: None, size_of_annotation: None }]
```

With the changes in this PR, that's no longer the case.

Resolves rust-lang#2364 
Resolves rust-lang#2680
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-type-system Area: Type system C-enhancement Category: An issue proposing an enhancement or a PR with one. metabug Issues about issues themselves ("bugs about bugs")
Projects
None yet
Development

No branches or pull requests

7 participants