From 9df8d683cbe77d719fef9d720a74eb2864e4a635 Mon Sep 17 00:00:00 2001 From: Josh L Date: Wed, 19 Jan 2022 13:05:17 -0800 Subject: [PATCH 01/23] Filling out template with PR 1034 --- proposals/p1034.md | 62 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 proposals/p1034.md diff --git a/proposals/p1034.md b/proposals/p1034.md new file mode 100644 index 0000000000000..555777a806957 --- /dev/null +++ b/proposals/p1034.md @@ -0,0 +1,62 @@ +# Generic details 9: default impl + + + +[Pull request](https://github.com/carbon-language/carbon-lang/pull/1034) + + + +## Table of contents + +- [Problem](#problem) +- [Background](#background) +- [Proposal](#proposal) +- [Details](#details) +- [Rationale based on Carbon's goals](#rationale-based-on-carbons-goals) +- [Alternatives considered](#alternatives-considered) + + + +## Problem + +TODO: What problem are you trying to solve? How important is that problem? Who +is impacted by it? + +## Background + +TODO: Is there any background that readers should consider to fully understand +this problem and your approach to solving it? + +## Proposal + +TODO: Briefly and at a high level, how do you propose to solve the problem? Why +will that in fact solve it? + +## Details + +TODO: Fully explain the details of the proposed solution. + +## Rationale based on Carbon's goals + +TODO: How does this proposal effectively advance Carbon's goals? Rather than +re-stating the full motivation, this should connect that motivation back to +Carbon's stated goals for the project or language. This may evolve during +review. Use links to appropriate goals, for example: + +- [Community and culture](/docs/project/goals.md#community-and-culture) +- [Language tools and ecosystem](/docs/project/goals.md#language-tools-and-ecosystem) +- [Performance-critical software](/docs/project/goals.md#performance-critical-software) +- [Software and language evolution](/docs/project/goals.md#software-and-language-evolution) +- [Code that is easy to read, understand, and write](/docs/project/goals.md#code-that-is-easy-to-read-understand-and-write) +- [Practical safety and testing mechanisms](/docs/project/goals.md#practical-safety-and-testing-mechanisms) +- [Fast and scalable development](/docs/project/goals.md#fast-and-scalable-development) +- [Modern OS platforms, hardware architectures, and environments](/docs/project/goals.md#modern-os-platforms-hardware-architectures-and-environments) +- [Interoperability with and migration from existing C++ code](/docs/project/goals.md#interoperability-with-and-migration-from-existing-c-code) + +## Alternatives considered + +TODO: What alternative solutions have you considered? From b680c26783c9ce00e1ff9e8c02b3cd5286e630af Mon Sep 17 00:00:00 2001 From: Josh L Date: Thu, 20 Jan 2022 09:37:58 -0800 Subject: [PATCH 02/23] Start proposal text --- proposals/p1034.md | 46 +++++++++++++++++++++++++++++++++++----------- 1 file changed, 35 insertions(+), 11 deletions(-) diff --git a/proposals/p1034.md b/proposals/p1034.md index 555777a806957..6cb9c3c187722 100644 --- a/proposals/p1034.md +++ b/proposals/p1034.md @@ -15,7 +15,6 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception - [Problem](#problem) - [Background](#background) - [Proposal](#proposal) -- [Details](#details) - [Rationale based on Carbon's goals](#rationale-based-on-carbons-goals) - [Alternatives considered](#alternatives-considered) @@ -23,22 +22,47 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception ## Problem -TODO: What problem are you trying to solve? How important is that problem? Who -is impacted by it? +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 -TODO: Is there any background that readers should consider to fully understand -this problem and your approach to solving it? +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 -TODO: Briefly and at a high level, how do you propose to solve the problem? Why -will that in fact solve it? - -## Details - -TODO: Fully explain the details of the proposed solution. +We allow an interface to provide a default implementation when it requires or +extends another interface. This is described in detail in +[the "FIXME" section of `docs/design/generics/details.md`](docs/design/generics/details.md#FIXME). ## Rationale based on Carbon's goals From c0770647a14ca1788de042297fafff176eff1fdb Mon Sep 17 00:00:00 2001 From: Josh L Date: Wed, 19 Jan 2022 13:05:17 -0800 Subject: [PATCH 03/23] Filling out template with PR 1034 --- proposals/p1034.md | 62 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 proposals/p1034.md diff --git a/proposals/p1034.md b/proposals/p1034.md new file mode 100644 index 0000000000000..555777a806957 --- /dev/null +++ b/proposals/p1034.md @@ -0,0 +1,62 @@ +# Generic details 9: default impl + + + +[Pull request](https://github.com/carbon-language/carbon-lang/pull/1034) + + + +## Table of contents + +- [Problem](#problem) +- [Background](#background) +- [Proposal](#proposal) +- [Details](#details) +- [Rationale based on Carbon's goals](#rationale-based-on-carbons-goals) +- [Alternatives considered](#alternatives-considered) + + + +## Problem + +TODO: What problem are you trying to solve? How important is that problem? Who +is impacted by it? + +## Background + +TODO: Is there any background that readers should consider to fully understand +this problem and your approach to solving it? + +## Proposal + +TODO: Briefly and at a high level, how do you propose to solve the problem? Why +will that in fact solve it? + +## Details + +TODO: Fully explain the details of the proposed solution. + +## Rationale based on Carbon's goals + +TODO: How does this proposal effectively advance Carbon's goals? Rather than +re-stating the full motivation, this should connect that motivation back to +Carbon's stated goals for the project or language. This may evolve during +review. Use links to appropriate goals, for example: + +- [Community and culture](/docs/project/goals.md#community-and-culture) +- [Language tools and ecosystem](/docs/project/goals.md#language-tools-and-ecosystem) +- [Performance-critical software](/docs/project/goals.md#performance-critical-software) +- [Software and language evolution](/docs/project/goals.md#software-and-language-evolution) +- [Code that is easy to read, understand, and write](/docs/project/goals.md#code-that-is-easy-to-read-understand-and-write) +- [Practical safety and testing mechanisms](/docs/project/goals.md#practical-safety-and-testing-mechanisms) +- [Fast and scalable development](/docs/project/goals.md#fast-and-scalable-development) +- [Modern OS platforms, hardware architectures, and environments](/docs/project/goals.md#modern-os-platforms-hardware-architectures-and-environments) +- [Interoperability with and migration from existing C++ code](/docs/project/goals.md#interoperability-with-and-migration-from-existing-c-code) + +## Alternatives considered + +TODO: What alternative solutions have you considered? From bc9146605ec1c805d3241c8f6dfbd0dba0b4ee8e Mon Sep 17 00:00:00 2001 From: Josh L Date: Thu, 20 Jan 2022 09:37:58 -0800 Subject: [PATCH 04/23] Start proposal text --- proposals/p1034.md | 46 +++++++++++++++++++++++++++++++++++----------- 1 file changed, 35 insertions(+), 11 deletions(-) diff --git a/proposals/p1034.md b/proposals/p1034.md index 555777a806957..6cb9c3c187722 100644 --- a/proposals/p1034.md +++ b/proposals/p1034.md @@ -15,7 +15,6 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception - [Problem](#problem) - [Background](#background) - [Proposal](#proposal) -- [Details](#details) - [Rationale based on Carbon's goals](#rationale-based-on-carbons-goals) - [Alternatives considered](#alternatives-considered) @@ -23,22 +22,47 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception ## Problem -TODO: What problem are you trying to solve? How important is that problem? Who -is impacted by it? +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 -TODO: Is there any background that readers should consider to fully understand -this problem and your approach to solving it? +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 -TODO: Briefly and at a high level, how do you propose to solve the problem? Why -will that in fact solve it? - -## Details - -TODO: Fully explain the details of the proposed solution. +We allow an interface to provide a default implementation when it requires or +extends another interface. This is described in detail in +[the "FIXME" section of `docs/design/generics/details.md`](docs/design/generics/details.md#FIXME). ## Rationale based on Carbon's goals From 94a4d092df3c2cd460c2747b203184af2177d931 Mon Sep 17 00:00:00 2001 From: Josh L Date: Wed, 26 Jan 2022 16:14:10 -0800 Subject: [PATCH 05/23] Alternatives --- proposals/p1034.md | 75 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 74 insertions(+), 1 deletion(-) diff --git a/proposals/p1034.md b/proposals/p1034.md index 6cb9c3c187722..65f4effb54bd6 100644 --- a/proposals/p1034.md +++ b/proposals/p1034.md @@ -17,6 +17,9 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception - [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) @@ -83,4 +86,74 @@ review. Use links to appropriate goals, for example: ## Alternatives considered -TODO: What alternative solutions have you 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, +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. +- FIXME: Need a way to prioritized impls provided by default against + overlapping +- 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). + +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. + +### Final impls in interfaces + +There would definitely be use cases for declaring an `impl` in an interface as +`final`, to ensure that the implementations of the two interfaces are consistent +with each other. For example, an interface for types with a total order might +want to provide a final implementation for a partial order interface. + +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 define a final impl for another interface +would have additional restrictions, which seemed surprising and awkward to use. +It is possible that we might find a way to lift these restrictions in this case +by using the fact that these declarations must be in the same library as the +interface being implemented. From ac9d249c924d16fb53545caf4010dbb87c0b43ec Mon Sep 17 00:00:00 2001 From: Josh L Date: Thu, 27 Jan 2022 11:16:35 -0800 Subject: [PATCH 06/23] Start reorganization --- docs/design/generics/details.md | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/docs/design/generics/details.md b/docs/design/generics/details.md index 39de9c5b21f4a..d59d0f2b5a702 100644 --- a/docs/design/generics/details.md +++ b/docs/design/generics/details.md @@ -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) @@ -4344,6 +4345,15 @@ interface Iterator { } ``` +**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 + Carbon does **not** support providing a default implementation of a required interface. @@ -4379,13 +4389,6 @@ 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`. -**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. - ### `final` members As an alternative to providing a definition of an interface member as a default, From c74f819dd096e055a710a99bd589a70ccd2606b7 Mon Sep 17 00:00:00 2001 From: Josh L Date: Fri, 28 Jan 2022 17:24:09 -0800 Subject: [PATCH 07/23] Checkpoint progress. --- docs/design/generics/details.md | 104 +++++++++++++++++++++++++++++--- 1 file changed, 94 insertions(+), 10 deletions(-) diff --git a/docs/design/generics/details.md b/docs/design/generics/details.md index 6e2a055287aff..ae0028fac30e9 100644 --- a/docs/design/generics/details.md +++ b/docs/design/generics/details.md @@ -4359,15 +4359,18 @@ Rust has found them valuable. #### Default implementation of required interface -Carbon does **not** support providing a default implementation of a required -interface. +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); } @@ -4375,13 +4378,74 @@ interface TotalOrder { } ``` -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); +// } +// } +``` + +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. + +``` +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(); + // } + // } +} +``` + +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 +[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 { @@ -4391,8 +4455,28 @@ 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 T as PartialOrder`, which has a higher priority than the blanket impl's +type structure of `impl ? as PartialOrder`. + +FIXME: prioritization in `match_first` blocks + +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. Carbon has +[a recursion limit to prevent this from defining an infinite collection of implementations](#termination-rule), +as would happen in this case: + +``` +interface Infinite(T:! Type) { + impl as Infinite(T*) { } +} +``` + +Implementations of required interfaces may not be marked `final`. Use `final` +blanket impls instead. ### `final` members From 8cd76da3cebee25dd671655facdba5bea49317e6 Mon Sep 17 00:00:00 2001 From: Josh L Date: Fri, 28 Jan 2022 17:39:23 -0800 Subject: [PATCH 08/23] Checkpoint progress. --- proposals/p1034.md | 34 ++++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/proposals/p1034.md b/proposals/p1034.md index 65f4effb54bd6..7512138f5a294 100644 --- a/proposals/p1034.md +++ b/proposals/p1034.md @@ -141,19 +141,33 @@ similar priority as the reversed impl was provided explicitly. ### Final impls in interfaces -There would definitely be use cases for declaring an `impl` in an interface as -`final`, to ensure that the implementations of the two interfaces are consistent -with each other. For example, an interface for types with a total order might -want to provide a final implementation for a partial order interface. - 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 define a final impl for another interface -would have additional restrictions, which seemed surprising and awkward to use. -It is possible that we might find a way to lift these restrictions in this case -by using the fact that these declarations must be in the same library as the -interface being implemented. +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. From 5332e4c690983f03b73a92fd5185dde83bd44c21 Mon Sep 17 00:00:00 2001 From: Josh L Date: Mon, 31 Jan 2022 09:18:28 -0800 Subject: [PATCH 09/23] Finish first pass on proposal text --- proposals/p1034.md | 46 +++++++++++++++++++++++++--------------------- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/proposals/p1034.md b/proposals/p1034.md index 7512138f5a294..3d6b227131c0f 100644 --- a/proposals/p1034.md +++ b/proposals/p1034.md @@ -20,6 +20,7 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception - [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) @@ -65,24 +66,20 @@ to the open questions in the design. We allow an interface to provide a default implementation when it requires or extends another interface. This is described in detail in -[the "FIXME" section of `docs/design/generics/details.md`](docs/design/generics/details.md#FIXME). +[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 -TODO: How does this proposal effectively advance Carbon's goals? Rather than -re-stating the full motivation, this should connect that motivation back to -Carbon's stated goals for the project or language. This may evolve during -review. Use links to appropriate goals, for example: - -- [Community and culture](/docs/project/goals.md#community-and-culture) -- [Language tools and ecosystem](/docs/project/goals.md#language-tools-and-ecosystem) -- [Performance-critical software](/docs/project/goals.md#performance-critical-software) -- [Software and language evolution](/docs/project/goals.md#software-and-language-evolution) -- [Code that is easy to read, understand, and write](/docs/project/goals.md#code-that-is-easy-to-read-understand-and-write) -- [Practical safety and testing mechanisms](/docs/project/goals.md#practical-safety-and-testing-mechanisms) -- [Fast and scalable development](/docs/project/goals.md#fast-and-scalable-development) -- [Modern OS platforms, hardware architectures, and environments](/docs/project/goals.md#modern-os-platforms-hardware-architectures-and-environments) -- [Interoperability with and migration from existing C++ code](/docs/project/goals.md#interoperability-with-and-migration-from-existing-c-code) +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. ## Alternatives considered @@ -91,17 +88,18 @@ review. Use links to appropriate goals, for example: [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, -and so we have attempted to answer the open questions about how this feature -would work: +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. -- FIXME: Need a way to prioritized impls provided by default against - overlapping +- 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). + [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 @@ -171,3 +169,9 @@ 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. From f3d9cae0b18f0d8d25f6699bc7e06043339ce22d Mon Sep 17 00:00:00 2001 From: Josh L Date: Mon, 31 Jan 2022 11:16:55 -0800 Subject: [PATCH 10/23] Finish details --- docs/design/generics/details.md | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/docs/design/generics/details.md b/docs/design/generics/details.md index ae0028fac30e9..5c65437f5e85a 100644 --- a/docs/design/generics/details.md +++ b/docs/design/generics/details.md @@ -4457,17 +4457,15 @@ external impl [T:! TotalOrder] T 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 T as PartialOrder`, which has a higher priority than the blanket impl's +`impl Song as PartialOrder`, which has a higher priority than the blanket impl's type structure of `impl ? as PartialOrder`. -FIXME: prioritization in `match_first` blocks - 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. Carbon has -[a recursion limit to prevent this from defining an infinite collection of implementations](#termination-rule), -as would happen in this case: +declared in the triggering interface. There is a a recursion limit to prevent +this from defining an infinite collection of implementations, like +[with parameterized impls](#termination-rule), as would happen in this case: ``` interface Infinite(T:! Type) { @@ -4475,6 +4473,9 @@ interface Infinite(T:! Type) { } ``` +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. @@ -4650,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) From 5d5d7e3a97d6bc44b6b6b39f8677a1bba3ae9bba Mon Sep 17 00:00:00 2001 From: josh11b Date: Mon, 31 Jan 2022 14:52:26 -0800 Subject: [PATCH 11/23] Apply suggestions from code review Co-authored-by: Richard Smith --- docs/design/generics/details.md | 18 ++++++++++-------- proposals/p1034.md | 2 +- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/docs/design/generics/details.md b/docs/design/generics/details.md index 5c65437f5e85a..f4e94b88bd55b 100644 --- a/docs/design/generics/details.md +++ b/docs/design/generics/details.md @@ -4387,12 +4387,14 @@ using the default definition, unless there has already been an implementation of 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); -// } -// } +\``` +Behaves as if it is followed by: +\``` +external impl Song as PartialOrder { + fn PartialLess[me: Self](right: Self) -> Bool { + return me.TotalLess(right); + } +} ``` The resulting impl definition must be legal where it is instantiated, for @@ -4402,7 +4404,7 @@ 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. +- the type implements the original interface internally. ``` interface Hashable { @@ -4463,7 +4465,7 @@ type structure of `impl ? as PartialOrder`. 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 +declared in the triggering interface. There is a recursion limit to prevent this from defining an infinite collection of implementations, like [with parameterized impls](#termination-rule), as would happen in this case: diff --git a/proposals/p1034.md b/proposals/p1034.md index 3d6b227131c0f..609f2dd526514 100644 --- a/proposals/p1034.md +++ b/proposals/p1034.md @@ -135,7 +135,7 @@ 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. +same priority as if the reversed impl was provided explicitly. ### Final impls in interfaces From 1fddebed556c3ebe6b00c1ea25efc1c6f859dd32 Mon Sep 17 00:00:00 2001 From: Josh L Date: Mon, 31 Jan 2022 15:54:07 -0800 Subject: [PATCH 12/23] Fix --- docs/design/generics/details.md | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/docs/design/generics/details.md b/docs/design/generics/details.md index f4e94b88bd55b..c5dfa4134e2fb 100644 --- a/docs/design/generics/details.md +++ b/docs/design/generics/details.md @@ -4387,9 +4387,11 @@ using the default definition, unless there has already been an implementation of external impl Song as TotalOrder { fn TotalLess[me: Self](right: Self) -> Bool { ... } } -\``` +``` + Behaves as if it is followed by: -\``` + +``` external impl Song as PartialOrder { fn PartialLess[me: Self](right: Self) -> Bool { return me.TotalLess(right); @@ -4465,8 +4467,8 @@ type structure of `impl ? as PartialOrder`. 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 recursion limit to prevent -this from defining an infinite collection of implementations, like +declared in the triggering interface. There is a recursion limit to prevent this +from defining an infinite collection of implementations, like [with parameterized impls](#termination-rule), as would happen in this case: ``` From 39e52c761364f10dcc4258b46645730925ef7932 Mon Sep 17 00:00:00 2001 From: Josh L Date: Thu, 3 Feb 2022 09:38:28 -0800 Subject: [PATCH 13/23] Checkpoint progress. --- docs/design/generics/details.md | 105 +++++++++++++++++++++++++++----- 1 file changed, 91 insertions(+), 14 deletions(-) diff --git a/docs/design/generics/details.md b/docs/design/generics/details.md index c5dfa4134e2fb..db04f55a174b1 100644 --- a/docs/design/generics/details.md +++ b/docs/design/generics/details.md @@ -95,6 +95,11 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception - [Interface members with definitions](#interface-members-with-definitions) - [Interface defaults](#interface-defaults) - [Default implementation of required interface](#default-implementation-of-required-interface) + - [Overriding the default](#overriding-the-default) + - [Constraint weaker than default impl](#constraint-weaker-than-default-impl) + - [Comparison to blanket impl](#comparison-to-blanket-impl) + - [Default impls for other types](#default-impls-for-other-types) + - [Multiple default impls](#multiple-default-impls) - [`final` members](#final-members) - [Future work](#future-work) - [Dynamic types](#dynamic-types) @@ -4378,10 +4383,9 @@ interface TotalOrder { } ``` -This means that any implementation of `TotalOrder` for a type `Song` behaves as +This means that an 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. +using the default definition. ``` external impl Song as TotalOrder { @@ -4399,18 +4403,20 @@ external impl Song as PartialOrder { } ``` -The resulting impl definition must be legal where it is instantiated, for +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 original interface internally. +The resulting impl will be [external](#external-impl), but it is possible for it +is still possible for its names to be available on the type. For example, if the +interface requirement uses `extends` instead of `impl as`, then the names from +the default impl will also be members of the requiring interfaces, and will be +available if that interface is implemented internally. ``` interface Hashable { fn Hash[me: Self]() -> u64; + // `extends` means `Equals` is a member of `Hashable` extends Equatable { fn Equals[me: Self](rhs: Self) -> bool { return me.Hash() == rhs.Hash(); @@ -4419,18 +4425,26 @@ interface Hashable { } class Song { + // Since `Song` implements `Hashable` internally, it has + // all the members of `Hashable`, including `Equals`. 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(); - // } - // } } ``` +##### Overriding the default + +The default definition is skipped if there has already been an implementation of +`PartialOrder` for `Song` declared earlier. + +``` +external impl Song as PartialOrder { ... } +external impl Song as TotalOrder { ... } +// Default implementation of `PartialOrder` for `Song` +// is ignored since one is already defined. +``` + Explicitly implementing `PartialOrder` for `Song` after it has been given a default definition is an error. @@ -4443,6 +4457,47 @@ external impl Song as TotalOrder { external impl Song as PartialOrder { ... } ``` +The default definition can be suppressed by an implementation of the interface +itself: + +``` +interface BothOrders(T:! Type, U:! Type) { + fn F[me: Self](x: T, y: U); + impl as BothOrders(U, T) { + fn F[me: Self](x: U, y: T) { + me.(BothOrders(T, U).F)(y, x); + } + } +} + +// Default not triggered when T == U +external impl bool as BothOrder(i32, i32) { ... } +``` + +The default definition can also be suppressed by an earlier instantiation of a +default, for example if the required interface doesn't use all of the parameters +of the requiring interface: + +``` +interface NoParam { } +interface WithParam(T:! Type) { + impl as NoParam { } +} + +external impl bool as WithParam(bool) { } +// Triggers implementation of `bool as NoParam` + +external impl bool as WithParam(i32) { } +// `bool` already implements `NoParam`, so default +// implementation skipped. +``` + +##### Constraint weaker than default impl + +FIXME + +##### Comparison to blanket impl + You can achieve a similar effect as a default impl by using a [blanket impl](#blanket-impls) instead: @@ -4464,6 +4519,28 @@ 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`. +A parameterized implementation of an interface with a default impl will result +in a parameterized impl. + +``` +external impl [T:! TotalOrder] Vector(T) as TotalOrder { ... } +``` + +Behaves as if it is followed by: + +``` +external impl [T:! TotalOrder] Vector(T) as PartialOrder { ... } +``` + +Note that this is still more specific than, and therefore given higher priority +than, the blanket impl approach. + +##### Default impls for other types + +FIXME + +##### Multiple default impls + 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 From 6bb20bcc93e7592f3e42ca52aca90ac27cca699a Mon Sep 17 00:00:00 2001 From: Josh L Date: Thu, 3 Feb 2022 10:23:40 -0800 Subject: [PATCH 14/23] Add to proposal --- proposals/p1034.md | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/proposals/p1034.md b/proposals/p1034.md index 609f2dd526514..04625eb5a8602 100644 --- a/proposals/p1034.md +++ b/proposals/p1034.md @@ -21,6 +21,8 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception - [Weak impls](#weak-impls) - [Final impls in interfaces](#final-impls-in-interfaces) - [Explicit prioritization of default impls](#explicit-prioritization-of-default-impls) + - [Separate default impls from requirements](#separate-default-impls-from-requirements) + - [Allow part of a default impl to be defined out of line](#allow-part-of-a-default-impl-to-be-defined-out-of-line) @@ -172,6 +174,21 @@ 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. +We did not include a method of explicitly prioritizing default impls for +simplicity. If the current approach is shown to have problems in practice, we +may add a way to indicate how they are prioritized explicitly in a `match_first` +block. + +### Separate default impls from requirements + +Right now, the syntax for defining a default impl also defines a matching +requirement at the same time. A workaround for this is provided +[in the proposal](/docs/design/generics/details.md#constraint-weaker-than-default-impl), +but it is surprising and requires a lot of boilerplate. A future proposal may +make changes to address this use case if it is found to be important. + +### Allow part of a default impl to be defined out of line + +You might want to define part of a default impl out of line for readability or +to break a cycle. This proposal does not address that use case. This should be +addressed in a future proposal if we determine it is needed. From ed04e06022cc07427f367332e24a632841782991 Mon Sep 17 00:00:00 2001 From: Josh L Date: Thu, 3 Feb 2022 11:16:59 -0800 Subject: [PATCH 15/23] Both interfaces in the same library --- proposals/p1034.md | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/proposals/p1034.md b/proposals/p1034.md index 04625eb5a8602..5b0760fe5b70e 100644 --- a/proposals/p1034.md +++ b/proposals/p1034.md @@ -103,6 +103,34 @@ feature would work: [internal](/docs/design/generics/terminology.md#internal-impl) based on whether the interface uses `impl as` or `extends`. +If the two interfaces were allowed to be in different libraries, for example + +``` +package P library "LibA" api; + +interface A { } +``` + +and + +``` +package P library "LibB" api; +import P library "LibA"; + +interface B { + impl as P.A { } +} + +// Not allowed to define an impl of `A` for +// `i32` in this library due to the orphan rule. +external impl i32 as B { } +``` + +Then implementing the interface with the default would typically be forbidden by +[the orphan rule](/docs/design/generics/details.md#orphan-rule). Otherwise, a +query to see if the type `i32` implements interface `A` would get a different +answer depending on whether library `LibB` had been imported. + 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 From d9dd9a890522b80edb21e81c04218a2df6f2c9d7 Mon Sep 17 00:00:00 2001 From: Josh L Date: Thu, 3 Feb 2022 13:12:14 -0800 Subject: [PATCH 16/23] Weaker constraints --- docs/design/generics/details.md | 40 ++++++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/docs/design/generics/details.md b/docs/design/generics/details.md index db04f55a174b1..8ae82111694f4 100644 --- a/docs/design/generics/details.md +++ b/docs/design/generics/details.md @@ -4494,7 +4494,45 @@ external impl bool as WithParam(i32) { } ##### Constraint weaker than default impl -FIXME +When specifying a default implementation, associated constants and types are +assigned using a `where` clause. Those assignments are also the requirements for +any type implementing the interface. + +``` +interface Required { + let A:! Type; +} +impl WithDefault { + // Types implementing `WithDefault` must implement + // `Required` with `Required.A` set to `i32`, which + // is also what the default implementation does. + impl as Required where .A = i32 { } +} +``` + +It is possible to assign associated constants and types in the default +implementation without also making them requirements by using +[an adapter](#use-case-defining-an-impl-for-use-by-other-types), as in this +example: + +``` +interface Required { + let A:! Type; + let B:! Type; +} +interface FewerConstraints; +adapter ImplOfRequired(T:! FewerConstraints) for T; +interface FewerConstraints { + // Types implementing `FewerConstraints` are required to + // implement `Required` with `Required.A` set to `i32`. + impl as Required where .A = i32 = ImplOfRequired(Self); +} +adapter ImplOfRequired(T:! FewerConstraints) for T { + // The default implementation of `Required` for + // `FewerConstraints` also sets `Required.B` to `bool`. + impl as Required where .A = i32 and .B = bool { } +} +``` ##### Comparison to blanket impl From dfedac7955394c2f11fc5e54b74ff0f8c916cf2d Mon Sep 17 00:00:00 2001 From: Josh L Date: Thu, 3 Feb 2022 15:41:41 -0800 Subject: [PATCH 17/23] Defaults for other types --- docs/design/generics/details.md | 48 ++++++++++++++++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) diff --git a/docs/design/generics/details.md b/docs/design/generics/details.md index 8ae82111694f4..b3aba892011ad 100644 --- a/docs/design/generics/details.md +++ b/docs/design/generics/details.md @@ -4575,7 +4575,53 @@ than, the blanket impl approach. ##### Default impls for other types -FIXME +An interface define a default impl for a type other than `Self` by writing the +name of that type between `impl` and `as`. Inside the definition of the default, +`Self` continues to refer to the type implementing the outer interface, not the +type the default is an implementation for. + +``` +interface CompareOp(T:! Type) { + fn Compare[me: Self](x: T) -> CompareResult; + impl T as CompareOp(Self) { + // `Self` means outer `Self` not `T` + fn Compare[me: T](x: Self) -> CompareResult { + return ReverseCompareResult(x.Compare(me)); + } + } +} + +external impl i32 as CompareOp(u32) { + fn Compare[me: Self](x: T) -> CompareResult { ... } +} +``` + +Behaves as if it is followed by: + +``` +external impl u32 as CompareOp(i32) { + fn Compare[me: u32](x: i32) -> CompareResult { + return ReverseCompareResult(x.Compare(me)); + } +} +``` + +Observe that in this example, the default will be suppressed for any impl of +`CompareOp` where `T` and `Self` are the same. If they are different, though, +the default will always be used because it can not be overridden once the +default is instantiated. + +Note that _if_ there was no default implementation, the _requirement_ that `T` +implement `CompareOp(Self)` could be written in the interface's parameter list: + +``` +interface CompareOp(T:! CompareOp(Self)) { + fn Compare[me: Self](x: T) -> CompareResult; +} +``` + +FIXME: Would this be allowed or is it invalid to use `CompareOp` as a parameter +constraint in the definition of `CompareOp` itself? ##### Multiple default impls From a16f02f64f710a5a6797110e6492b418f4aa5ca8 Mon Sep 17 00:00:00 2001 From: Josh L Date: Thu, 3 Feb 2022 16:04:30 -0800 Subject: [PATCH 18/23] `observe` alternative --- proposals/p1034.md | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/proposals/p1034.md b/proposals/p1034.md index 5b0760fe5b70e..d12ba004165c6 100644 --- a/proposals/p1034.md +++ b/proposals/p1034.md @@ -23,6 +23,7 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception - [Explicit prioritization of default impls](#explicit-prioritization-of-default-impls) - [Separate default impls from requirements](#separate-default-impls-from-requirements) - [Allow part of a default impl to be defined out of line](#allow-part-of-a-default-impl-to-be-defined-out-of-line) + - [Require `observe` declarations to find required implementations](#require-observe-declarations-to-find-required-implementations) @@ -220,3 +221,37 @@ make changes to address this use case if it is found to be important. You might want to define part of a default impl out of line for readability or to break a cycle. This proposal does not address that use case. This should be addressed in a future proposal if we determine it is needed. + +### Require `observe` declarations to find required implementations + +In +[this discussion in the #typesystem channel on Discord](https://discord.com/channels/655572317891461132/708431657849585705/938154940931657758), +we considered whether the ability to define +[default impls for other types](/docs/design/generics/details.md#default-impls-for-other-types) +made type checking much more difficult. In this example, + +``` +class Y {} +interface OtherInterface(T:! Type) {} +interface X { impl Y as OtherInterface(Self) {} } +fn F[T:! X](a: T) { + // Can we assume `Y` implements `OtherInterface(T)` here? +} +``` + +we would like to use the fact that the type parameter `T` implements the +interface `X` to conclude that the type `Y` implements `OtherInterface(T)`. Part +of the concern here is that just looking at the query "does `Y` implement +`OtherInterface(T)`?", it isn't clear that the answer can be found by looking at +the definition of `X`. Furthermore, we might have to recursively look into the +requirements in `X` for multiple levels to find the fact that we are looking +for. + +We +[considered](https://discord.com/channels/655572317891461132/708431657849585705/938162240811585566) +saying that the compiler would only look at a depth of one instead of performing +a recursive search. The user would be able to then write explicit +[`observe` declarations](/docs/design/generics/details.md#observe-declarations) +to identify other facts that could be used in type checking. We eventually +concluded that this issue was still present with ordinary interface requirements +and was not specific to this proposal. From fafd2bd0b462f361b82f0c7e83861984ef1fd091 Mon Sep 17 00:00:00 2001 From: Josh L Date: Mon, 7 Feb 2022 14:13:42 -0800 Subject: [PATCH 19/23] Interface members with defaults use `default` keyword --- docs/design/generics/details.md | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/docs/design/generics/details.md b/docs/design/generics/details.md index b3aba892011ad..f6b1ad2599b25 100644 --- a/docs/design/generics/details.md +++ b/docs/design/generics/details.md @@ -4287,8 +4287,9 @@ differences between the Carbon and Rust plans: Interfaces may provide definitions for members, such as a function body for an associated function or method or a value for an associated constant. If these -definitions may be overridden in implementations, they are called "defaults." -Otherwise they are called "final members." +definitions may be overridden in implementations, they are called "defaults" and +marked with the `default` keyword. Otherwise they are called "final members" and +marked with the `final` keyword. ### Interface defaults @@ -4300,7 +4301,7 @@ interface Vector { fn Add[me: Self](b: Self) -> Self; fn Scale[me: Self](v: f64) -> Self; // Default definition of `Invert` calls `Scale`. - fn Invert[me: Self]() -> Self { + default fn Invert[me: Self]() -> Self { return me.Scale(-1.0); } } @@ -4321,7 +4322,7 @@ types, and interface parameters, using the `= ` syntax. ``` interface Add(Right:! Type = Self) { - let Result:! Type = Self; + default let Result:! Type = Self; fn DoAdd[me: Self](right: Right) -> Result; } @@ -4351,7 +4352,7 @@ More generally, default expressions may reference other associated types or ``` interface Iterator { let Element:! Type; - let Pointer:! Type = Element*; + default let Pointer:! Type = Element*; } ``` @@ -4375,7 +4376,7 @@ interface TotalOrder { // for `Self`, but provides a default definition. // `TotalOrder` and `PartialOrder` must be defined in the // same library. - impl as PartialOrder { + default impl as PartialOrder { fn PartialLess[me: Self](right: Self) -> Bool { return me.TotalLess(right); } @@ -4417,7 +4418,7 @@ available if that interface is implemented internally. interface Hashable { fn Hash[me: Self]() -> u64; // `extends` means `Equals` is a member of `Hashable` - extends Equatable { + default extends Equatable { fn Equals[me: Self](rhs: Self) -> bool { return me.Hash() == rhs.Hash(); } @@ -4463,7 +4464,7 @@ itself: ``` interface BothOrders(T:! Type, U:! Type) { fn F[me: Self](x: T, y: U); - impl as BothOrders(U, T) { + default impl as BothOrders(U, T) { fn F[me: Self](x: U, y: T) { me.(BothOrders(T, U).F)(y, x); } @@ -4481,7 +4482,7 @@ of the requiring interface: ``` interface NoParam { } interface WithParam(T:! Type) { - impl as NoParam { } + default impl as NoParam { } } external impl bool as WithParam(bool) { } @@ -4506,7 +4507,7 @@ impl WithDefault { // Types implementing `WithDefault` must implement // `Required` with `Required.A` set to `i32`, which // is also what the default implementation does. - impl as Required where .A = i32 { } + default impl as Required where .A = i32 { } } ``` @@ -4525,7 +4526,7 @@ adapter ImplOfRequired(T:! FewerConstraints) for T; interface FewerConstraints { // Types implementing `FewerConstraints` are required to // implement `Required` with `Required.A` set to `i32`. - impl as Required where .A = i32 = ImplOfRequired(Self); + default impl as Required where .A = i32 = ImplOfRequired(Self); } adapter ImplOfRequired(T:! FewerConstraints) for T { // The default implementation of `Required` for @@ -4583,7 +4584,7 @@ type the default is an implementation for. ``` interface CompareOp(T:! Type) { fn Compare[me: Self](x: T) -> CompareResult; - impl T as CompareOp(Self) { + default impl T as CompareOp(Self) { // `Self` means outer `Self` not `T` fn Compare[me: T](x: Self) -> CompareResult { return ReverseCompareResult(x.Compare(me)); @@ -4634,7 +4635,7 @@ from defining an infinite collection of implementations, like ``` interface Infinite(T:! Type) { - impl as Infinite(T*) { } + default impl as Infinite(T*) { } } ``` From 4b2af9500a449cfd9412edf86799223f84e9305d Mon Sep 17 00:00:00 2001 From: Josh L Date: Mon, 7 Feb 2022 14:54:05 -0800 Subject: [PATCH 20/23] Finer control over defaults --- docs/design/generics/details.md | 98 ++++++++++++++++++++++++++++++--- 1 file changed, 89 insertions(+), 9 deletions(-) diff --git a/docs/design/generics/details.md b/docs/design/generics/details.md index f6b1ad2599b25..7fd238821298e 100644 --- a/docs/design/generics/details.md +++ b/docs/design/generics/details.md @@ -4404,6 +4404,10 @@ external impl Song as PartialOrder { } ``` +Note that it is the definition of a `TotalOrder` implementation that triggers +the instantiation of `PartialOrder` for `Song`, a forward declaration of the +implementation does not. + 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. @@ -4434,16 +4438,22 @@ class Song { } ``` +Implementations of required interfaces may not be marked `final`. Use `final` +blanket impls instead. + ##### Overriding the default The default definition is skipped if there has already been an implementation of `PartialOrder` for `Song` declared earlier. ``` -external impl Song as PartialOrder { ... } +// Forward declaration of `PartialOrder` is sufficient, +// but a definition of `PartialOrder` would also suppress +// it being generated as a default. +external impl Song as PartialOrder; external impl Song as TotalOrder { ... } // Default implementation of `PartialOrder` for `Song` -// is ignored since one is already defined. +// is ignored since one has already been declared. ``` Explicitly implementing `PartialOrder` for `Song` after it has been given a @@ -4622,16 +4632,89 @@ interface CompareOp(T:! CompareOp(Self)) { ``` FIXME: Would this be allowed or is it invalid to use `CompareOp` as a parameter -constraint in the definition of `CompareOp` itself? +constraint in the definition of `CompareOp` itself? Awkward since we aren't even +done defining the parameter list yet, which would normally be required in a +forward declaration. A forward declaration can't be declared for the same +reason. ##### Multiple default impls 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 recursion limit to prevent this -from defining an infinite collection of implementations, like -[with parameterized impls](#termination-rule), as would happen in this case: +declared in the triggering interface. + +``` +interface A { + fn F() -> i32; +} +interface B { + default impl as A { + fn F() -> i32 { return 0; } + } +} +interface C { + default impl as A { + fn F() -> i32 { return 1; } + } +} +interface D { + default impl as B { } + default impl as C { } +} + +external impl i32 as D { } +``` + +Behaves as if it is followed by: + +``` +// `B` is listed before `C` in `D`, so the +// default for `B` is defined first. +external impl i32 as B { } +// Depth-first order means, the default for +// `A` from `B` will be used before `C`. +external impl i32 as A { + fn F() -> i32 { return 0; } +} +// `C` is after `B` in `D`. +external impl i32 as C { } +// The default for `A` from `C` is ignored +// since we already got a definition for `A` +// from `B`. +``` + +An implementation of an interface can opt in to using a default defined by that +interface using the `= default`, even if that default would otherwise be +suppressed due to an earlier forward declaration. This is an error if a matching +implementation has already been defined. + +``` +// Forward declaration suppresses default +// implementations of `A`. +external impl bool as A; +// Does not trigger default definition of `A`. +external impl bool as B { } +external impl bool as C { + // Trigger default definition of `A`. + impl as A = default; +} +``` + +FIXME: How to get the same behavior as `bool` of using the default for `A` from +`C` when defining an implementation for `D`? + +``` +external impl f32 as D { + // What do I put here to get C's A instead of B's? + // Otherwise would like to use the defaults given + // by D for both B and C. +} +``` + +There is a recursion limit to prevent this from defining an infinite collection +of implementations, like [with parameterized impls](#termination-rule), as would +happen in this case: ``` interface Infinite(T:! Type) { @@ -4642,9 +4725,6 @@ interface Infinite(T:! Type) { 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 As an alternative to providing a definition of an interface member as a default, From 3f90eec304bd74e7ceea12bf8f616fc661e19e05 Mon Sep 17 00:00:00 2001 From: Josh L Date: Tue, 8 Feb 2022 14:22:14 -0800 Subject: [PATCH 21/23] Out-of-line defaults --- docs/design/generics/details.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/docs/design/generics/details.md b/docs/design/generics/details.md index 7fd238821298e..aac751bca2723 100644 --- a/docs/design/generics/details.md +++ b/docs/design/generics/details.md @@ -4310,6 +4310,19 @@ interface Vector { An impl of that interface for a type may omit a definition of `Invert` to use the default, or provide a definition to override the default. +**FIXME:** A default function or method may also be defined out of line: + +``` +interface Vector { + fn Add[me: Self](b: Self) -> Self; + fn Scale[me: Self](v: f64) -> Self; + default fn Invert[me: Self]() -> Self; +} +fn Vector.Invert[me: Self]() -> Self { + return me.Scale(-1.0); +} +``` + Interface defaults are helpful for [evolution](#evolution), as well as reducing boilerplate. Defaults address the gap between the minimum necessary for a type to provide the desired functionality of an interface and the breadth of API that From 8a5d2c7f19d4f32c4afdf6d22e9b914726309110 Mon Sep 17 00:00:00 2001 From: josh11b Date: Wed, 9 Feb 2022 09:35:25 -0800 Subject: [PATCH 22/23] Apply suggestions from code review Co-authored-by: Richard Smith --- docs/design/generics/details.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/design/generics/details.md b/docs/design/generics/details.md index aac751bca2723..e853f634378f5 100644 --- a/docs/design/generics/details.md +++ b/docs/design/generics/details.md @@ -4425,8 +4425,8 @@ 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), but it is possible for it -is still possible for its names to be available on the type. For example, if the +The resulting impl will be [external](#external-impl), but it is possible for +its names to be available on the type. For example, if the interface requirement uses `extends` instead of `impl as`, then the names from the default impl will also be members of the requiring interfaces, and will be available if that interface is implemented internally. @@ -4599,7 +4599,7 @@ than, the blanket impl approach. ##### Default impls for other types -An interface define a default impl for a type other than `Self` by writing the +An interface can define a default impl for a type other than `Self` by writing the name of that type between `impl` and `as`. Inside the definition of the default, `Self` continues to refer to the type implementing the outer interface, not the type the default is an implementation for. From dffdd16fef693424ed11ce4f77dfa5345d196043 Mon Sep 17 00:00:00 2001 From: Josh L Date: Wed, 9 Feb 2022 13:36:03 -0800 Subject: [PATCH 23/23] Fix --- docs/design/generics/details.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/docs/design/generics/details.md b/docs/design/generics/details.md index e853f634378f5..00fec9b124ccc 100644 --- a/docs/design/generics/details.md +++ b/docs/design/generics/details.md @@ -4426,10 +4426,10 @@ example it must respect the [orphan rule](#orphan-rule), or the triggering impl is invalid. The resulting impl will be [external](#external-impl), but it is possible for -its names to be available on the type. For example, if the -interface requirement uses `extends` instead of `impl as`, then the names from -the default impl will also be members of the requiring interfaces, and will be -available if that interface is implemented internally. +its names to be available on the type. For example, if the interface requirement +uses `extends` instead of `impl as`, then the names from the default impl will +also be members of the requiring interfaces, and will be available if that +interface is implemented internally. ``` interface Hashable { @@ -4599,10 +4599,10 @@ than, the blanket impl approach. ##### Default impls for other types -An interface can define a default impl for a type other than `Self` by writing the -name of that type between `impl` and `as`. Inside the definition of the default, -`Self` continues to refer to the type implementing the outer interface, not the -type the default is an implementation for. +An interface can define a default impl for a type other than `Self` by writing +the name of that type between `impl` and `as`. Inside the definition of the +default, `Self` continues to refer to the type implementing the outer interface, +not the type the default is an implementation for. ``` interface CompareOp(T:! Type) {