rust-lang / keyword-generics-initiative

Public repository for the Rust keyword generics initiative
https://rust-lang.github.io/keyword-generics-initiative/
Other
96 stars 11 forks source link

Exhaustive implementations (RFC feedback) #36

Open clarfonthey opened 10 months ago

clarfonthey commented 10 months ago

One thing that was rather unclear to me when reading the draft RFC is whether implementations of traits must be exclusive (e.g. you can only implement sync xor async) or whether they can be exhaustive (e.g. you can implement sync and async).

Just taking this initial example:

/// The base implementation
impl Into<Loaf> for Cat {
    fn into(self) -> Loaf {
        self.nap()
    }
}

/// The async implementation
impl async Into<AsyncLoaf> for AsyncCat {
    async fn into(self) -> AsyncLoaf {
        self.async_nap().await
    }
}

It's not immediately clear whether this is presented as a common implementation (someone providing one implementation for each) or two options for an implementation. Note that the former case, at least in the case of const, would require RFC 3352, although there's no such precedent for async and presumably we could do this freely.

It's also worth mentioning that, in terms of effect lowering, the RFC also makes the assumption that effects are simply boolean constant generics as opposed to associated boolean constants, which also would presumably change based upon the choice here.

For something like async, having the ability to do only async or only sync is not an issue, but for const, this is relatively novel, and I'd imagine that an MVP of this would also want to consider at least const, even though stuff like try and unsafe could probably wait until later.

yoshuawuyts commented 10 months ago

Heya, thanks for asking! I tried to cover that in the section Concrete Impls and Coherence:

With the eye on forward-compatibility, and a potential future where types can themselves also be generic over effects, for now types may only implement either the effectful or the base variant of the trait. This ensures that the door is kept open for effect generic implementations later on. As well as ensures that during trait selection the trait variant remains unambiguous. The diagnostics for this case should clearly communicate that only a single trait variant can be implemented per type.

error[E0119]: conflicting implementations of trait `Into` for type `Cat`
 --> src/lib.rs:5:1
  |
4 | impl Into for Cat {}
  | ----------------- first implementation here
5 | impl async Into for Cat {}
  | ^^^^^^^^^^^^^^^^^ conflicting implementation for `Cat`
  |
  | help: types can't both implement the sync and async variant of a trait

TLDR: Only a single impl per type is allowed for now. I should make it more clear that this RFC explicitly does not cover const, since const fn does not mean: "must be const-evaluated" (always semantics) but means: "may be const-evaluated" (maybe semantics). The second RFC in this series is intended to cover effect-polymorphic trait bounds and functions, which will get into more detail about this.

clarfonthey commented 10 months ago

It is unfortunate that the RFC only covers async right now, since const traits are potentially a more anticipated feature than async traits, by my extremely vigorous analysis of guessing and wanting them myself.

I think that having at least one other effect besides async would be good, since it helps make sure the system is flexible enough to cover "weird" effects like const.

Fishrock123 commented 9 months ago

So I've been thinking about this for a couple weeks, and I have felt that non-exhaustive implementations did not sit well with me for some reason(s) I couldn't quite put into words.

A recent article on futures backs up my feeling more concretely so I figure I might as well try to state it here and hope it will be heard.

I think having a language fundamental annotation for trying to make blocking and non-blocking (or other, similar, not necessarily compatible in every direction) functionality be invisible for specific items would be a very, very bad mistake for, well, any programming language. And it certainly doesn't fit well with Rust where what you get and what happens is always supposed to be guaranteed to be explicit in some way or other.

Boats makes a concrete point about this in regarding to the (as the article terms it) "affordances" that is given by the futures paradigm, which normally would allow you to design applications specifically with the task scheduling benefits in mind (assuming library authors are not doing anything bad internally, but at least that's either a bug or it's outright malice). The inverse is of course if you use blocking functionality (cpu bound code and/or especially blocking synchronizations) that can (In reality: almost certainly) causes large problems in non-blocking contexts.

Beyond async, the other current (and future potential) keywords do not seem better off. maybe(const), maybe(try), maybe(no_panic) - To me it just seems that any being "maybe implemented" would lead to ecosystem chaos.

The idea here (non-exhaustive implementations), in essence, makes it easy for library authors to confuse users as to what they should or should not actually be doing. Not very Rust like.

On the flipside, exhaustive implementations has no downside in clarity, you always know that it would select the one which you need. Again this has the caveats are library authors potentially writing problematic code (the reasons may be overarchingly important, but I do not think they are important to if the language should be clear to users or not), which seems like it should be addressed by some other (if perhaps yet unforseen) functionality.

Fishrock123 commented 9 months ago

I feel like I was maybe to harsh in that comment. I do presently feel relatively strongly that this could have significant negative repercussions but I would still like to have a conversation

yoshuawuyts commented 9 months ago

On the blog post

Regarding the blog post, I've just finished skimming it, and it appears to me that the author has based their argument on a number of incorrect assumptions. For example, they assume we're able to objectively classify which code is considered blocking, which we can't. Or that unbounded non-parallel async concurrency necessarily requires tasks [^tasks], which it doesn't. As well as that maybe(async) code would not be able to, or not able to easily support concurrent organization code, which again is not true [^goals].

As a whole I'm not really sure what to do with this post. I might try and sit down another time and try and extract questions from it - but on a first read it doesn't seem to contain anything new or unaccounted for. So my instinct is to continue authoring draft RFCs, in an attempt to better communicate the entire system. We can then take it from there.

[^tasks]: From the blog post: "The first limitation is that it is only possible to achieve a static arity of concurrency with intra-task concurrency. That is, you cannot join (or select, etc) an arbitrary number of futures with intra-task concurrency: the number must be fixed at compile time."

[^goals]: There is no draft RFC for it yet, but it should be covered in no draft no. 4. I also already covered it in my Rustconf talk. And there are even more directions we could explore beyond what I covered there to make it even easier.

Meta: On giving feedback

I should probably find a spot to write this down somewhere, but when giving feedback can I ask people try to ask questions rather than make statements? This is not a general rule, but just how I prefer to receive feedback. A question feels both like an expression of curiosity and an invitation for dialogue. I find it generally easy to engage with it, and it makes for an easier back/forth.

In contrast I find statements a lot harder to engage with. I often find myself wondering which part to engage with, and getting it wrong doesn't really help either party much. I find statements tend to invite debate more than they do dialogue, and so I'm thrust into a position of fact-checker - which is not something I particularly enjoy doing.

I'm sharing this because I'm trying to take your concerns seriously @Fishrock123. I'd rather be clear that I don't understand what you're asking, rather than ignoring your comments and letting it sit. Can I ask you to open a new issue or issues where you rephrase the above as questions? If they're questions we've already accounted for I can point you where they've been answered in the draft RFCs. If we haven't already answered them, I'll try my best to elaborate on the issues, and we can then use that to include as part of the draft RFCs. Thanks!

Fishrock123 commented 9 months ago

they assume we're able to objectively classify which code is considered blocking, which we can't.

I'm not sure I agree with that. In my mind there is some behavior which can't be qualified and some which can (If you are synchronously waiting for another thread, that is definitely what the post refers to as "blocking").

Or that unbounded non-parallel async concurrency necessarily requires tasks 1, which it doesn't.

The same is true for recursive async functions, but that section of the post was about being able to be able to optimize futures to be perfectly-sized, which is no longer true if you are jumping through a pointer (Box/Vec/etc).


Meta: On giving feedback

This reads kinda dismissive, like "you haven't applied the correct formatting to the feedback you are trying to give", but I'll try to re-read everything and give it a shot, although the concern is, essentially, the same as the original post here, just more emphasized regarding concern.

yoshuawuyts commented 9 months ago

This reads kinda dismissive, like "you haven't applied the correct formatting to the feedback you are trying to give"

I'm legitimately trying to engage with you here. I assume you didn't just post here just to make your concerns known, but are also interested in some kind of response? Right now I'm unsure what you're looking for though. I figured reformulating your statements as questions would make it easier to get a dialogue going.

In hindsight I should have probably started off by asking: what kind of response are you looking for? If you just came here to share concerns that's valid of course. In which case: thank you for sharing. But I assume there's more?

Fishrock123 commented 9 months ago

I am in fact misunderstanding; the existing proposal is probably very familiar to those working on it but is rather outwardly confusing.

My issue is present when - as per the following example - A & B are not linked, and it is not clear without extensive reading that they are linked:

#[maybe(async)] // <-- A
trait Into<T>: Sized {
    #[maybe(async)] // <-- B
    fn into(self) -> T;
}

i.e. if you could write what is probably more clear as:

trait Into<T>: Sized {
    #[maybe(async)]
    fn into(self) -> T; // when implemented, what is this? the user has no certainty now
}

Then you have the "non-exhaustiveness" problem I described. I think I will open a new issue regarding the... syntax I suppose.


Addendum:

Another thing I'd like to still mention here: my confusion was also reinforced by how it is very new and strange to define generic bounds with maybe(effect) (it's not clear upon initial reading that this could be used for any bounds at all until explicitly mentioned).