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

Interface implementation ("impl") syntax #575

Closed
josh11b opened this issue Jun 11, 2021 · 1 comment
Closed

Interface implementation ("impl") syntax #575

josh11b opened this issue Jun 11, 2021 · 1 comment
Labels
leads question A question for the leads team

Comments

@josh11b
Copy link
Contributor

josh11b commented Jun 11, 2021

There are a number of options discussed in this document. In open discussions, we have been converging towards these choices:

External impl

Task: The type Song implements the interface Comparable. The methods of Comparable may not be used as unqualified members of Song. The implementation is allowed to live either in the library with Song or Comparable. Another use case is the "blanket impl" case, where we provide an implementation of the Printable interface for any type implementing the ToString interface, or an implementation of Comparable for Vector(T) anytime T implements Comparable. There are two possibilities under consideration:

out-of-line impl...as choice:

impl Song as Comparable {
  method (me: Self) Less(rhs: Self) -> Bool;
}
impl (T:$ ToString) as Printable { ... }
impl Vector(C:$ Comparable) as Comparable { ... }

Advantages:

  • More concise, so less to read and write.

out-of-line external impl...as choice:

external impl Song as Comparable {
  method (me: Self) Less(rhs: Self) -> Bool;
}
external impl (T:$ ToString) as Printable { ... }
external impl Vector(C:$ Comparable) as Comparable { ... }

Advantages:

  • More explicit to clarify that the the methods of this impl definition are not contributing to unqualified API of the type.
  • Matches how you would describe this kind of impl, especially when contrasting with "inline impl" (see below).

Note that this is a divergence from the Rust approach which uses for instead of as and puts the interface before the type (impl Comparable for Song). In discussions we have been favoring this as approach for two main reasons:

  • Song as Comparable is the actual name of the facet type we are defining the implementation of.
  • It seems more natural to express the parameters to the interface in terms of the parameters and associated items of the type than the other way around.

Conditional inline impl

Task: A Vector(T) doesn't implement the Printable interface or have a method Print unless T implements Printable:

struct Vector(T:$ Type) {
  // data and methods ...
  impl Vector(P:$ Printable) as Printable {
    method (me: Self) Print() { ... }
  }
}

Inline impl

Task: The type Song implements the interface Printable, and the methods of Printable may be used as unqualified members of Song. To preserve the "unqualified members all come (transitively) from declarations inside the type's definition" property, this means an impl block inside the definition of Song, but there are still two possibilities under consideration.

inline impl choice:

struct Song {
  // data and methods ...
  impl Printable {
    method (me: Self) Print() { ... }
  }
}

Advantages:

  • More concise, so less to read and write.

inline impl as choice:

struct Song {
  // data and methods ...
  impl as Printable {
    method (me: Self) Print() { ... }
  }
}

Advantages:

  • Consistency with the other uses of impl, which always put the interface name after as.
  • Matches the conditional impl syntax if you make the type between impl and as optional and default to Self.

Questions

Is this syntax good, or are there changes you would like to see? Do you prefer external for the out-of-line cases? Do you prefer to always use as for the inline cases?

@chandlerc
Copy link
Contributor

@zygoloid and I are both happy keeping as in the inline impl. It seems a small cost for consistency.

Both of us felt like the question around external was a bit of a bikeshed. My painter opinion is to keep it for now. If the annoyance ends up growing to the point that it outweighs the utility, we can remove it. But at least some folks have found the explicit marking to be useful.

I think we have consensus among the leads on this so closing as well. Happy to re-open if needed though!

@jonmeow jonmeow added the leads question A question for the leads team label Aug 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
leads question A question for the leads team
Projects
None yet
Development

No branches or pull requests

3 participants