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

Generic details 9: default impl #1034

Closed
wants to merge 26 commits into from
Closed
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
121 changes: 105 additions & 16 deletions docs/design/generics/details.md
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
- [Comparison to Rust](#comparison-to-rust)
- [Interface members with definitions](#interface-members-with-definitions)
- [Interface defaults](#interface-defaults)
- [Default implementation of required interface](#default-implementation-of-required-interface)
- [`final` members](#final-members)
- [Future work](#future-work)
- [Dynamic types](#dynamic-types)
Expand Down Expand Up @@ -4349,29 +4350,102 @@ interface Iterator {
}
```

Carbon does **not** support providing a default implementation of a required
interface.
**Comparison with other languages:** Rust supports specifying defaults for
[methods](https://doc.rust-lang.org/book/ch10-02-traits.html#default-implementations),
[interface parameters](https://doc.rust-lang.org/book/ch19-03-advanced-traits.html#default-generic-type-parameters-and-operator-overloading),
and
[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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


Carbon supports providing a default implementation of a
[required interface](#interface-requiring-other-interfaces) in an interface, as
long as the required interface is defined in the same library.

```
interface TotalOrder {
fn TotalLess[me: Self](right: Self) -> Bool;
// ❌ Illegal: May not provide definition
// for required interface.
impl PartialOrder {
// `TotalOrder` requires `PartialOrder` to be implemented
// for `Self`, but provides a default definition.
// `TotalOrder` and `PartialOrder` must be defined in the
// same library.
impl as PartialOrder {
fn PartialLess[me: Self](right: Self) -> Bool {
return me.TotalLess(right);
}
}
}
```

The workaround for this restriction is to use a [blanket impl](#blanket-impls)
instead:
This means that any implementation of `TotalOrder` for a type `Song` behaves as
if it is immediately followed by an implementation of `PartialOrder` for `Song`
using the default definition, unless there has already been an implementation of
`PartialOrder` for `Song` declared earlier.

```
external impl Song as TotalOrder {
fn TotalLess[me: Self](right: Self) -> Bool { ... }
}
// as if followed by:
// external impl Song as PartialOrder {
// fn PartialLess[me: Self](right: Self) -> Bool {
// return me.TotalLess(right);
// }
// }
josh11b marked this conversation as resolved.
Show resolved Hide resolved
```

The resulting impl definition must be legal where it is instantiated, for
example it must respect the [orphan rule](#orphan-rule), or the triggering impl
is invalid.

The resulting impl will be [external](#external-impl) unless both:

- the interface requirement uses `extends` instead of `impl as`, and
- the type implements the type type `Song` implements `TotalOrder` internally.
josh11b marked this conversation as resolved.
Show resolved Hide resolved

```
interface Hashable {
fn Hash[me: Self]() -> u64;
extends Equatable {
fn Equals[me: Self](rhs: Self) -> bool {
return me.Hash() == rhs.Hash();
}
}
}

class Song {
impl as Hashable {
fn Hash[me: Self]() -> u64 { ... }
}
// As if followed by *internal* impl of Equatable:
// impl as Equatable {
// fn Equals[me: Self](rhs: Self) -> bool {
// return me.Hash() == rhs.Hash();
// }
// }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}
```

Explicitly implementing `PartialOrder` for `Song` after it has been given a
default definition is an error.

```
external impl Song as TotalOrder {
fn TotalLess[me: Self](right: Self) -> Bool { ... }
}
// ❌ Illegal: `PartialOrder` already implemented for `Song`
// using default definition from `Song as TotalOrder`.
external impl Song as PartialOrder { ... }
```

You can achieve a similar effect as a default impl by using a
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

@zygoloid zygoloid Feb 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

[blanket impl](#blanket-impls) instead:

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

external impl [T:! TotalOrder] T as PartialOrder {
Expand All @@ -4381,15 +4455,29 @@ external impl [T:! TotalOrder] T as PartialOrder {
}
```

Note that by the [orphan rule](#orphan-rule), this blanket impl must be defined
in the same library as `PartialOrder`.
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`.
Comment on lines +4579 to +4582
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. That we still get a type structure that's more precise than a blanket impl, but one that's still parameterized.
  2. What happens if the default impl doesn't use all of the parameters of the outer interface (plus Self).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


**Comparison with other languages:** Rust supports specifying defaults for
[methods](https://doc.rust-lang.org/book/ch10-02-traits.html#default-implementations),
[interface parameters](https://doc.rust-lang.org/book/ch19-03-advanced-traits.html#default-generic-type-parameters-and-operator-overloading),
and
[associated constants](https://doc.rust-lang.org/reference/items/associated-items.html#associated-constants-examples).
Rust has found them valuable.
If an interface provides multiple default impl definitions, or a default impl
definition triggers another default impl to be instantiated, the default impls
are instantiated in depth-first order following the order the default impls were
declared in the triggering interface. There is a a recursion limit to prevent
josh11b marked this conversation as resolved.
Show resolved Hide resolved
this from defining an infinite collection of implementations, like
[with parameterized impls](#termination-rule), as would happen in this case:

```
interface Infinite(T:! Type) {
impl as Infinite(T*) { }
}
```

Default impls are prioritized immediately after the triggering impl in a
`match_first` block, in the same order they are instantiated.

Implementations of required interfaces may not be marked `final`. Use `final`
blanket impls instead.

### `final` members

Expand Down Expand Up @@ -4563,3 +4651,4 @@ be included in the declaration as well.
- [#983: Generic details 7: final impls](https://github.com/carbon-language/carbon-lang/pull/983)
- [#990: Generics details 8: interface default and final members](https://github.com/carbon-language/carbon-lang/pull/990)
- [#1013: Generics: Set associated constants using where constraints](https://github.com/carbon-language/carbon-lang/pull/1013)
- [#1034: Generic details 9: default impl](https://github.com/carbon-language/carbon-lang/pull/1034)
177 changes: 177 additions & 0 deletions proposals/p1034.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,177 @@
# Generic details 9: default impl

<!--
Part of the Carbon Language project, under the Apache License v2.0 with LLVM
Exceptions. See /LICENSE for license information.
SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-->

[Pull request](https://github.com/carbon-language/carbon-lang/pull/1034)

<!-- toc -->

## Table of contents

- [Problem](#problem)
- [Background](#background)
- [Proposal](#proposal)
- [Rationale based on Carbon's goals](#rationale-based-on-carbons-goals)
- [Alternatives considered](#alternatives-considered)
- [Status quo using blanket impls](#status-quo-using-blanket-impls)
- [Weak impls](#weak-impls)
- [Final impls in interfaces](#final-impls-in-interfaces)
- [Explicit prioritization of default impls](#explicit-prioritization-of-default-impls)

<!-- tocstop -->

## Problem

We want there to be a convenient way to implement two impls together in a
consistent way. For example, given two types `T1` and `T2` that are equality
comparable to each other, we would like to get the same result no matter which
type appears on the left side of the equal sign.

For equality comparison, it would be reasonable to require that it be defined in
both directions if it is defined for one. In other cases, the order of the
arguments to an operator will matter for some types but not others. For example,
addition is commutative for integers but not strings. We'd like to make it
convenient to specify that addition is commutative for specific pairs of types
without requiring that addition is always commutative.

## Background

In addition to equality and order comparisons, the `CommonType` interface from
[proposal #911: "conditional expressions"](https://github.com/carbon-language/carbon-lang/pull/911)
should be defined symmetrically.

[Rejected proposal #1027: "weak impls"](https://github.com/carbon-language/carbon-lang/pull/1027)
attempted to address these use cases, but in practice that approach
[did not result in the correct prioritization of impls](https://discord.com/channels/655572317891461132/708431657849585705/931740599600709692).
The symptom would be that switching the argument order could result in a
different specialization being selected. This problem arose since the
implementation for one order came from a general blanket impl with low priority,
instead of both argument orders having similar priority.

[Proposal #990](https://github.com/carbon-language/carbon-lang/pull/990) defined
interface defaults, but specifically excluded letting interfaces give defaults
for other interfaces it required. At the time that feature seemed to have too
much overlap with what could be done with blanket impls and was not worth the
additional complexity it would introduce and questions that would need to be
resolved. However, once weak impls were found to be an inadequate solution for
their intended use cases, it was discovered that default implementations would
be able to solve this problem better. These use cases also helped give answers
to the open questions in the design.

## Proposal

We allow an interface to provide a default implementation when it requires or
extends another interface. This is described in detail in
[the "Default implementation of required interface" section of `docs/design/generics/details.md`](/docs/design/generics/details.md#default-implementation-of-required-interface).

## Rationale based on Carbon's goals

This proposal is important for using the intended specialization, particularly
for overloaded operators. Specialization is part of Carbon's
[performance story](/docs/project/goals.md#performance-critical-software).

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.
Comment on lines +80 to +85
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made that an alternative.


## Alternatives considered

### Status quo using blanket impls

[The original proposal that added interface defaults](https://github.com/carbon-language/carbon-lang/pull/990)
[considered and rejected allowing interfaces to proved implementations of required interfaces](p0990.md#allow-default-implementations-of-required-interfaces).
That proposal recommended using blanket impls instead. Since then we have come
to understand that blanket impls don't address an important use case as well as
we want, and so we have attempted to answer the open questions about how this
feature would work:

- Requiring both interfaces to be defined in the same library addresses
incoherence concerns.
Comment on lines +98 to +99
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

- We prioritize impls provided by default just after the impl that triggered
its instantiation.
- We resolved rules determining when the default impl would be
[external](/docs/design/generics/terminology.md#external-impl) or
[internal](/docs/design/generics/terminology.md#internal-impl) based on
whether the interface uses `impl as` or `extends`.

We also had specific concerns that there would be cases where a default
implementation is prioritized over an explicit implementation in a way that
would be surprising to users. The main concern is how a default impl is not
visible in the source in the same places as other impls. This is how defaults
work generally, and so seemed like something users would be able to understand,
but is something to be on the lookout for once we gain experience with this
feature.

### Weak impls

We considered allowing declarations in the same library as an interface to mark
some implementations as `weak` and use constraints restricted to the non-`weak`
implementations manually written by users, in
[rejected proposal #1027](https://github.com/carbon-language/carbon-lang/pull/1027).

The
[main problem with this approach](https://discord.com/channels/655572317891461132/708431657849585705/931740599600709692)
is that the weak impls provided by the interface's library in the envisioned use
cases were prioritized incorrectly. For example, with these definitions:

```
interface CommonType(T:! Type) {
let Result:! Type;
}
weak impl [T:! Type, U:! CommonTypeWith(T)]
T as CommonTypeWith(U) where .Result = U.Result {}
impl Optional(T) as CommonType(T) where .Result = Optional(T) {}
impl T* as CommonType(Optional(T*)) where .Result = Nullable(T*) {}
```

then the common type of `Optional(T*)` and `T*` would either be `Optional(T*)`
or `Nullable(T*)` depending on the order they were tested. This is because the
blanket `weak` impl is very broad and therefore is given low priority. To get a
symmetric answer, we need the impls corresponding to the two orders to have the
similar priority as the reversed impl was provided explicitly.
josh11b marked this conversation as resolved.
Show resolved Hide resolved

### Final impls in interfaces

Under the rules of this proposal, a type can implement an interface that has a
default implementation for another interface only if it would be legal to write
the default implementation explicitly. The
[rules for final impls outside of interfaces](/docs/design/generics/details.md#libraries-that-can-contain-final-impls)
from [proposal #983](https://github.com/carbon-language/carbon-lang/pull/983)
restrict which libraries are allowed to declare a `final` impl. Combining these
rules would mean that interfaces that provide a definition for a required impl
of another interface would have additional restrictions, which seemed surprising
and awkward to use.

Furthermore, these restrictions are essential, otherwise you could make a copy
of an interface to bypass the restrictions on `final`:

```
interface I {
fn F...();
}
// Defined in the same library as `I`.
interface FinalI {
fn F...();
final impl as I { fn F...() { FinalI.F(); } }
}
```

With this definition of `FinalI`, you could get around the rules for final impls
by implementing `FinalI` instead of `I`.

The use cases for `final` impls are well served by `final` blanket impls. The
motivation for default impls do not apply here since `final` prevents higher
priority impls from being defined.

### Explicit prioritization of default impls

If the current approach of prioritizing default impls is shown to have problems
in practice, we may add a way to indicate how they are prioritized explicitly in
a `match_first` block.