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

combined trait/impl syntax #3048

Closed
Dretch opened this issue Jul 29, 2012 · 18 comments
Closed

combined trait/impl syntax #3048

Dretch opened this issue Jul 29, 2012 · 18 comments
Labels
A-type-system Area: Type system C-enhancement Category: An issue proposing an enhancement or a PR with one.

Comments

@Dretch
Copy link
Contributor

Dretch commented Jul 29, 2012

Rust used to support iface-less impls. These were good because they avoided duplicating method signatures.

Since the iface -> trait conversion these no longer seem to be supported. Instead, it seems, a trait-less impl can only apply to class types and is equivalent to declaring the methods inside the class body.

IMO the iface/trait-less impl was really useful. Having to declare the iface/trait even though there will only ever be one impl seems redundant.

Is there any chance this feature could return?

@lkuper
Copy link
Contributor

lkuper commented Jul 29, 2012

It's still possible to have trait-less implementations that apply to non-class types; that hasn't changed. For an example of one currently in use in the compiler, search for impl methods for @fn_ctxt in https://github.com/mozilla/rust/blob/incoming/src/rustc/middle/typeck/check.rs. The receiver type, fn_ctxt, is defined in that file, and isn't a class. There are several other examples in the compiler. Am I misunderstanding the issue?

@nikomatsakis
Copy link
Contributor

It is only possible for nominal types, strictly speaking, which basically means classes or enums.

@Dretch
Copy link
Contributor Author

Dretch commented Jul 29, 2012

@lkuper thanks for the pointer. I think something has changed, but slightly differently to how I suggested.

This is my current understanding:

Before, you could define an iface-less impl on any type and the definition could be in any scope. These impls behaved just like normal impls (that had ifaces).

Currently, you can define a trait-less impl on any nominal (or pointer-to nominal?) type so long as the definition has to be in the same scope as the type. Going by the error messages I have seen, i am guessing these impls have a different semantics to normal impls - they are somehow "inherent" methods that are more attached to the type than regular trait methods are.

My guess is that methods defined inside class bodies are going to be removed and replaced with trait-less impls defined in the same scope. This would allow defining "inherent" methods on enums, something that was not possible before.

I found the previous semantics of iface/trait-less impls really nice. It is no fun to have to define a trait and then directly below it duplicate all the method signatures in the single impl of that trait.

As a solution to this problem, could inherent methods be defined only within class bodies and enum bodies? Currently you cant defined methods inside enums, but why not? This would allow the trait-less impl syntax to have the semantics that iface-less impls had before.

@nikomatsakis
Copy link
Contributor

You can already define an impl without a trait for enums/classes, but you must do so within the same module (or submodule). I don't see how allowing one to define methods inside an enum body would help the problem I thought you were facin. Actually defining methods within a class body is deprecated in any case.

@nikomatsakis
Copy link
Contributor

Maybe I misunderstood what you wanted? I figured you wanted to define new trait-less methods from some other place besides the definition of the receiver type.

@Dretch
Copy link
Contributor Author

Dretch commented Jul 29, 2012

I figured you wanted to define new trait-less methods from some other place besides the definition of the receiver type.

That is correct. Sorry for not being clear.

I am just trying to suggest a way to allow trait-less impls to have the semantics that they did before - by introducing different ways to implement the semantics that they have now. But I have made some assumptions that probably are not correct, confusing matters.

@lkuper
Copy link
Contributor

lkuper commented Jul 29, 2012

Before, you could define an iface-less impl on any type and the definition could be in any scope. These impls behaved just like normal impls (that had ifaces).
Currently, you can define a trait-less impl on any nominal (or pointer-to nominal?) type so long as the definition has to be in the same scope as the type. Going by the error messages I have seen, i am guessing these impls have a different semantics to normal impls - they are somehow "inherent" methods that are more attached to the type than regular trait methods are.

@Dretch Could it be that you're butting heads with the new instance coherence enforcement? (See discussion of instance coherence on the development roadmap, and also here.)

I found the previous semantics of iface/trait-less impls really nice. It is no fun to have to define a trait and then directly below it duplicate all the method signatures in the single impl of that trait.

I think you shouldn't have to do that once default methods are allowed in traits, which is a feature that's on the way.

@Dretch
Copy link
Contributor Author

Dretch commented Jul 30, 2012

@Dretch Could it be that you're butting heads with the new instance coherence enforcement? (See discussion of instance coherence on the development roadmap, and also here.)

Perhaps, but I am hopeful that the trait-less impls (where the trait to implement is kind-of anonymous and is created automatically by declaring the impl - that is how I understood the old iface-less impls) are compatible with the new coherence rules.

I think you shouldn't have to do that once default methods are allowed in traits, which is a feature that's on the way.

As I understand if the default/provided methods in a trait would only be allowed to call other (required/provided) methods in that trait, so they are not equivalent to trait-less impls, which can call any methods/functions defined for the type.

@lkuper
Copy link
Contributor

lkuper commented Jul 30, 2012

@Dretch Can you gist a concrete piece of code that used to work and doesn't now, so I can get a clear idea of the problem? Thanks.

@Dretch
Copy link
Contributor Author

Dretch commented Jul 30, 2012

@Dretch Can you gist a concrete piece of code that used to work and doesn't now, so I can get a clear idea of the problem? Thanks.

Before, I could write (this is in a util module in a project of mine):

impl option_extensions for option<~str> {

    /// if the specified option is some(msg), then fail with msg
    fn fail_if_some () {
        for self.each |msg| { fail msg }
    }

}

But now I have to write:

trait option_extensions {

    fn fail_if_some();    

}

impl option_extensions of option_extensions for option<~str> {

    /// if the specified option is some(msg), then fail with msg
    fn fail_if_some () {
        for self.each |msg| { fail msg }
    }

}

It is not that it doesn't work any more - it works fine - the problem (IMHO) is that it requires more duplication/boilerplate.

@pcwalton
Copy link
Contributor

Would it be fine to have a syntax that declared a trait and an impl in one go?

@Dretch
Copy link
Contributor Author

Dretch commented Jul 31, 2012

Would it be fine to have a syntax that declared a trait and an impl in one go?

I think it would, yes please!

@lkuper
Copy link
Contributor

lkuper commented Aug 1, 2012

So, for the record: yes, the issue is caused by instance coherence enforcement, and no, adding default methods to traits won't solve it. But @pcwalton's suggestion, of a combined trait/impl syntax, would. Maybe it should just look like this (to go back to @Dretch's example):

trait option_extensions for option<~str> {

    /// if the specified option is some(msg), then fail with msg
    fn fail_if_some () {
        for self.each |msg| { fail msg }
    }

}

And this wouldn't be possible with merely traits + default methods, because parametricity requires that provided methods can only use other provided/required methods.

So, tentatively changing the title of this bug to "combined trait/impl syntax"...

@graydon
Copy link
Contributor

graydon commented Aug 2, 2012

+1 on this, I just hit it again. not a huge deal but it'll keep coming up. it's primarily how you extend a base trait with purely-derived methods.

@brson
Copy link
Contributor

brson commented Aug 8, 2012

I am going to put this on the 0.4 milestone because I think it's a fairly big papercut issue.

@brson
Copy link
Contributor

brson commented Aug 15, 2012

Perhaps we can use a syntax extension

@graydon
Copy link
Contributor

graydon commented Apr 25, 2013

nominating for backwards-compatible milestone

@graydon
Copy link
Contributor

graydon commented Apr 25, 2013

consensus of team is closing WONTFIX.

@graydon graydon closed this as completed Apr 25, 2013
bors pushed a commit to rust-lang-ci/rust that referenced this issue May 15, 2021
lists: Detect block comment by starting from the end.
RalfJung pushed a commit to RalfJung/rust that referenced this issue Sep 3, 2023
add '--skip-children' to rustfmt invocation

This finally fixes the issue that we format the same file many times (and `./miri fmt --check` shows duplicate diffs). :)
celinval pushed a commit to celinval/rust-dev that referenced this issue Jun 4, 2024
Upgrades the Rust toolchain to `nightly-2024-02-25`. The Rust compiler
PRs that triggered changes in this upgrades are:
 * rust-lang#121209
 * rust-lang#121309
 * rust-lang#120863
 * rust-lang#117772
 * rust-lang#117658

With rust-lang#121309 some intrinsics
became inlineable so their names became qualified. This made our `match`
on the intrinsic name to fail in those cases, leaving them as
unsupported constructs as in this example:

```
warning: Found the following unsupported constructs:
             - _RNvNtCscyGW2MM2t5j_4core10intrinsics8unlikelyCs1eohKeNmpdS_5arith (3)
             - caller_location (1)
             - foreign function (1)
         
         Verification will fail if one or more of these constructs is reachable.
         See https://model-checking.github.io/kani/rust-feature-support.html for more details.

[...]

Failed Checks: _RNvNtCscyGW2MM2t5j_4core10intrinsics8unlikelyCs1eohKeNmpdS_5arith is not currently supported by Kani. Please post your example at https://github.com/model-checking/kani/issues/new/choose
 File: "/home/ubuntu/.rustup/toolchains/nightly-2024-02-18-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/num/mod.rs", line 25, in core::num::<impl i8>::checked_add
```

We use `trimmed_name()` to work around this, but that may include type
arguments if the intrinsic is defined on generics. So in those cases, we
just take the first part of the name so we can keep the rest as before.

Resolves rust-lang#3044
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.
Projects
None yet
Development

No branches or pull requests

6 participants