Generic details 9: default impl #1034

josh11b · 2022-01-19T21:05:24Z

When an interface requires another interface, using impl as or extends, this proposal allows a default implementation to be provided inline, as in:

interface TotalOrder {
  default impl as PartialOrder {
    fn PartialLess[me: Self](right: Self) -> Bool {
      return me.TotalLess(right);
    }
  }
}

The reason to use this instead of blanket impls is how they get prioritized differently.

…pl-default

docs/design/generics/details.md

zygoloid · 2022-01-31T21:44:30Z

docs/design/generics/details.md

+  // As if followed by *internal* impl of Equatable:
+  // impl as Equatable {
+  //   fn Equals[me: Self](rhs: Self) -> bool {
+  //     return me.Hash() == rhs.Hash();
+  //   }
+  // }


Does it actually matter if the impl as Equatable is internal? If both of the prerequisite conditions are met, the Hashable interface contains all members of the Equatable interface, so I'd expect Song to have the same interface here regardless of whether we have an internal or external implementation of Equatable.

If I'm not missing something, I think this rule would be equivalent: "A default impl is always external. Note: Its interface will be available in the type if the original interface is implemented internally and extends the type for which a default impl is provided."

I don't necessarily think that alternative presentation is any better; I think the rules are clear either way, and we want to spell out the consequences either way. I'm mostly just checking that there's not some subtlety here that I've overlooked.

I think your approach makes sense. I've updated the text, though I'm not super happy about how the wording came out. I might need to establish some terminology to talk about the two different interfaces involved.

zygoloid · 2022-01-31T21:47:56Z

docs/design/generics/details.md

+external impl Song as PartialOrder { ... }
+```
+
+You can achieve a similar effect as a default impl by using a


Suggested change

You can achieve a similar effect as a default impl by using a

You can achieve a similar effect to a default impl by using a

Maybe this is a US/UK difference? I checked with my wife and she'd say "as" in this context.

Mm, could be. I know that the "different from" versus "different than"/"different to" is a UK/US thing ("different from" is the only one of those that's correct in formal British English), so a similar divergence for "similar to" would make sense. To my reading, "as" binds to "you" not to "similar", as in "you can travel a similar distance as a pedestrian ..." which makes this construction a little garden-pathy for me. Maybe a different word order would read well to both of us:

Suggested change

You can achieve a similar effect as a default impl by using a

You can achieve an effect similar to a default impl by using a

zygoloid · 2022-01-31T21:51:58Z

docs/design/generics/details.md

+The difference between the two approaches is the prioritization of the resulting
+implementations. The default impl approach results in a type structure of
+`impl Song as PartialOrder`, which has a higher priority than the blanket impl's
+type structure of `impl ? as PartialOrder`.


I would like to see an example of what happens for a default impl if it's generated from a parameterized impl. I think there are two things to demonstrate here:

That we still get a type structure that's more precise than a blanket impl, but one that's still parameterized.

What happens if the default impl doesn't use all of the parameters of the outer interface (plus Self).

I think #2 is just a separate issue with parameterized interfaces independent of parameterization of the impl. I have added text addressing those points separately.

zygoloid · 2022-01-31T21:54:26Z

docs/design/generics/details.md

+[associated constants](https://doc.rust-lang.org/reference/items/associated-items.html#associated-constants-examples).
+Rust has found them valuable.
+
+#### Default implementation of required interface


The examples here are all impl as and extends. For symmetrizing, I think we would also want to support the case where the self type of the default impl differs from the Self type of the enclosing interface (impl T as ComparableWith(Self)). It'd be useful to include an example of that if we intend to support it.

I was going to include that, but I left it for a follow up since I realized I have not yet described that feature for interface requirements (though it is intended and a straightforward generalization).

I've adding it since we have decided that is really part of this feature.

docs/design/generics/details.md

zygoloid · 2022-01-31T22:34:04Z

proposals/p1034.md

+In addition, providing a default implementation of a required interface directly
+in the interface definition rather than in a separate blanket `impl` is expected
+to help make Carbon code
+[easy to read, understand, and write](/docs/project/goals.md#code-that-is-easy-to-read-understand-and-write),
+since that puts related code together and avoids repeating information relating
+those two interfaces together.


Hm. What happens if you want to define some part of a default impl out-of-line? (Or if you have to, in order to break a cycle?) I guess we don't have a syntax for defining a method of a default impl, or even for naming one. Maybe that's a problem to solve in whatever proposal introduces the top-down ordering rule.

Made that an alternative.

zygoloid · 2022-01-31T22:36:34Z

proposals/p1034.md

+-   Requiring both interfaces to be defined in the same library addresses
+    incoherence concerns.


I would like to see a bit more explanation of this point; given that a default impl is effectively rewritten to something that the developer could have written themselves, it's not clear to me why there would be any coherence concerns if we allow an interface in one library to provide a default impl for an interface in another library.

I can think of cases where such permission might be useful. For example, if the Carbon prelude provides only AddableWith, I could define my own SymmetricAddableWith that extends AddableWith and also provides a default impl for the reversed form.

I've written more to explain this point. You are correct that there isn't technically a coherence concern, except that it would be more common for the orphan rule to prevent you from implementing SymmetricAddableWith. I could remove this restriction with that caveat.

proposals/p1034.md

Co-authored-by: Richard Smith <richard@metafoo.co.uk>

docs/design/generics/details.md

zygoloid · 2022-02-04T01:25:44Z

docs/design/generics/details.md

+external impl bool as WithParam(bool) { }
+// Triggers implementation of `bool as NoParam`
+
+external impl bool as WithParam(i32) { }


What happens if I write:

external impl i32 as WithParam(T:! Type) { }

? My guess is that this would be equivalent to:

external impl [T:! Type] i32 as WithParam(T) { }

... and would generate:

external impl [T:! Type] i32 as NoParam { }

... which would be invalid because T is not deducible, and more generally, all wildcard impls of WithParam would be disallowed.

Similarly for Self:

interface Foo(T:! Type) { impl T as NoParam { } } external impl i32 as Foo(i64) { } // Injected: `external impl i64 as NoParam { };` external impl (T:! Type) as Foo(i8) { } // Injected: `external impl [T:! Type] i8 as NoParam { };` // Rejected because that `impl` is invalid.

... and more generally, blanket impls of Foo would be disallowed.

Examples of that would be useful, though I'm not sure if that belongs here or down below where you talk about parameterized implementations of interfaces with default impls.

docs/design/generics/details.md

Co-authored-by: Richard Smith <richard@metafoo.co.uk>

josh11b · 2022-02-11T19:31:45Z

Withdrawing this proposal. Consensus is this has too much overlap with blanket impls, and doesn't fully fix the priority problem that they have.

josh11b added the proposal A proposal label Jan 19, 2022

josh11b requested a review from a team January 19, 2022 21:05

josh11b added 13 commits January 19, 2022 13:05

Filling out template with PR 1034

9df8d68

Start proposal text

b680c26

Filling out template with PR 1034

c077064

Start proposal text

bc91466

Merge branch 'impl-default' of github.com:josh11b/carbon-lang into im…

e8cfd56

…pl-default

Alternatives

94a4d09

Merge remote-tracking branch 'upstream/trunk' into impl-default

a275c40

Start reorganization

ac9d249

Merge remote-tracking branch 'upstream/trunk' into impl-default

139f040

Checkpoint progress.

c74f819

Checkpoint progress.

8cd76da

Finish first pass on proposal text

5332e4c

Finish details

f3d9cae

josh11b marked this pull request as ready for review January 31, 2022 19:20

josh11b requested a review from a team as a code owner January 31, 2022 19:20

github-actions bot added the proposal rfc Proposal with request-for-comment sent out label Jan 31, 2022

zygoloid reviewed Jan 31, 2022

View reviewed changes

josh11b and others added 8 commits January 31, 2022 14:52

Apply suggestions from code review

5d5d7e3

Co-authored-by: Richard Smith <richard@metafoo.co.uk>

Fix

1fddebe

Checkpoint progress.

39e52c7

Add to proposal

6bb20bc

Both interfaces in the same library

ed04e06

Weaker constraints

d9dd9a8

Defaults for other types

dfedac7

observe alternative

a16f02f

zygoloid reviewed Feb 4, 2022

View reviewed changes

josh11b and others added 5 commits February 7, 2022 14:13

Interface members with defaults use default keyword

fafd2bd

Finer control over defaults

4b2af95

Out-of-line defaults

3f90eec

Apply suggestions from code review

8a5d2c7

Co-authored-by: Richard Smith <richard@metafoo.co.uk>

Fix

dffdd16

josh11b closed this Feb 11, 2022

josh11b deleted the impl-default branch February 11, 2022 19:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Generic details 9: default impl #1034

Generic details 9: default impl #1034

josh11b commented Jan 19, 2022 •

edited

Loading

zygoloid Jan 31, 2022

josh11b Feb 2, 2022

zygoloid Jan 31, 2022

josh11b Jan 31, 2022

zygoloid Feb 4, 2022 •

edited

Loading

zygoloid Jan 31, 2022

josh11b Feb 2, 2022

zygoloid Jan 31, 2022

josh11b Jan 31, 2022

josh11b Feb 3, 2022

zygoloid Jan 31, 2022

josh11b Feb 3, 2022

zygoloid Jan 31, 2022

josh11b Feb 3, 2022

zygoloid Feb 4, 2022

josh11b commented Feb 11, 2022

	You can achieve a similar effect as a default impl by using a
	You can achieve a similar effect to a default impl by using a

	You can achieve a similar effect as a default impl by using a
	You can achieve an effect similar to a default impl by using a

		- Requiring both interfaces to be defined in the same library addresses
		incoherence concerns.

Generic details 9: default impl #1034

Generic details 9: default impl #1034

Conversation

josh11b commented Jan 19, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zygoloid Feb 4, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

josh11b commented Feb 11, 2022

josh11b commented Jan 19, 2022 •

edited

Loading

zygoloid Feb 4, 2022 •

edited

Loading