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

Declaration and definition of impls and their associated functions #4672

Open
josh11b opened this issue Dec 12, 2024 · 4 comments
Open

Declaration and definition of impls and their associated functions #4672

josh11b opened this issue Dec 12, 2024 · 4 comments
Labels
leads question A question for the leads team

Comments

@josh11b
Copy link
Contributor

josh11b commented Dec 12, 2024

Summary of issue:

There are three aspects of an impl specified by a developer:

  • The values of non-function associated constants. These determine the signatures of the function members of the interface.
  • The signatures of the associated functions. These must be compatible with the signatures in the interface, but may not be exactly the same. For example, there may be implicit conversions between the corresponding parameter types or return types, or an addr self method in the interface my be implemented by a self method in the impl.
  • The bodies of the associated functions.

The first and last aspects can have defaults specified by the interface if the default modifier is used on that member. Furthermore, in some sense the interface's signatures for associated functions can be thought of as default signatures for the corresponding impl functions.

This leads us to some questions:

  • Which of these aspects are fixed by an impl declaration (one ending in ;)? by an impl definition (with curly braces {...})?
  • If some aspects are not fixed by the impl definition, when & how are they fixed?
  • Can you have an impl declaration in an api file and the definition in the impl file? Is an impl usable without its definition?
  • Can you define the bodies of associated functions outside of an impl definition? Can those be in an impl file even if the impl itself is defined in the api file?

There are some existing answers to these questions in the design, but I believe it is time to re-evaluate given that (a) I understand we have defined a syntax for implementing function bodies outside of the impl definition (mentioned in #3763), and (b) I am working on implementing these concerns in the toolchain.

Details:

There are some desirable traits a solution might have:

  • We'd like the impl signatures to generally be visible to impl consumers, so calls can use that information to minimize unnecessary conversions and avoid going through stubs.
  • We would like to give some flexibility around the ordering of declarations and definitions. The concern is that there may be dependencies and references that mean that some code depends on knowing a type implements an interface before we can specify all the types and values mentioned in the first two aspects (non-function associated constants and the signatures of associated functions).
  • We would like to minimize unnecessary ceremony. For example, it would be nice to minimize how much we repeat the signatures of associated functions in an impl, beyond when we define their bodies.
  • We would like to support interface evolution without breaking impls of that interface. For example, it should be possible to add a default to an existing member.
@josh11b josh11b added the leads question A question for the leads team label Dec 12, 2024
@josh11b
Copy link
Contributor Author

josh11b commented Dec 12, 2024

In discussions we considered a number of possible approaches. There was one family of "definition in the impl file" approaches where an impl is usable by importers as long as it is declared in the api file. The impl declaration would fix the non-function associated constants. The impl definition would be allowed to live in the impl file. There were a number of variations on how the out-of-line associated function bodies would be ordered with respect to the impl definition:

  • In one variation, the out-of-line associated function bodies would be required to come after the definition. The definition would fix the function signatures, acting as a forward declaration of any associated functions that are not defined in the impl definition.
  • In one variation, the out-of-line associated function bodies would be required to come before the definition, and then the checking for completeness and selection of defaults would happen at the end of the impl definition.
  • In another variation, impl definitions are just a way to define function bodies for associated functions. No definition is needed if all of the associated functions have bodies. Conversely, the function bodies could be split among multiple impl definitions. Function bodies are fixed at the end of the impl file.

We didn't like that this family of approaches made it convenient and natural to put the impl's function signatures in the impl file, forcing callers to use the function signatures in the interface in many cases.

@josh11b
Copy link
Contributor Author

josh11b commented Dec 12, 2024

Another approach put the impl declaration and definition are in the api file, but out-of-line associated function bodies could be defined in the impl file. The non-function associated constants would be fixed by the declaration and the function signatures would be fixed by the definition, acting as a forward declaration of any functions whose bodies were not defined in the impl definition. Out-of-line function bodies would be allowed in the impl file.

@josh11b
Copy link
Contributor Author

josh11b commented Dec 12, 2024

The approach we ended up favoring was the "most like classes" approach. Here:

  • The impl declaration and definition would be in the api file.
  • The declaration would not fix anything, but would establish that the type implements the interface.
    • Question: could it specify a subset of the associated constants in a where clause?
  • The non-function associated constants would be fixed at the opening curly { of the impl definition.
  • The associated function signatures would be fixed by the end of the impl definition.
  • Associated functions not mentioned in the impl definition would be given the default signature from the interface.
  • Out-of-line function bodies would be allowed in the impl file.
    • If their signature did not match the signature given by impl definition, but were compatible, a stub function that did the conversion would be generated to bridge between the two versions.
    • An out-of-line definition of a function would subsume a default body in the interface at link time. This was instead of trying to figure out if the default should be used from the impl definition, to allow interfaces to add default function bodies without breaking impls when they were omitted from the impl definition.
    • Missing function body definitions would also be diagnosed at link time.

We liked that this gave some flexibility in ordering, since you could say that a type implemented an interface before you could name all the values of its associated constants or the types mentioned in the associated function signatures (when they were different from the interface's).

We also liked the idea of allowing the definition to be short by not repeating the interface function signatures when they matched.

We thought that an impl could explicitly specify that it was using the default function body from the interface by writing = default in place of a function body in curly braces {...}.

@josh11b
Copy link
Contributor Author

josh11b commented Dec 17, 2024

Further details:

  • (Mentioned as a question above.) We can allow forward declarations of an impl to specify a subset of the constraints, as long as we check that the definition defines a superset. This could be helpful when breaking cycles, if some associated constants can be specified in the forward declaration even if others can't.
  • We could say that a forward impl declaration is a promise that there will be impls that cover an matching query, but I think it would be simpler to require that the forward declaration matches the query and parameterization of the definition. The only differences allowed would be in the where clause to the right of the as. This is intended to treat the parameterized query part of the declaration (forall parameters + type + interface, but not where constraints on the interface) as the analog of the name that other entities have. We would use this name when declaring impls in a match_first/impl_priority block.
  • There is an open question about how this will interact with the extern feature for declaring impls in other files.

@josh11b josh11b mentioned this issue Dec 18, 2024
github-merge-queue bot pushed a commit that referenced this issue Jan 2, 2025
* Change `InterfaceWitness` -> `ImplWitness`
* Include a `SpecificId` in the `ImplWitness`. This allows the
`InstBlock` it contains to have its own identity, allowing it to be
changed as the impl is processed. Evaluation only updates the specific.
* Create the `ImplWitness` at the start of the impl definition. In the
future, this will be populated with the values of non-function
associated constants. For now, it starts full of invalid instruction
ids.
* Implements the model suggested in #4672 .

Note that the non-SemIR testdata changes are to these file:
* `toolchain/check/testdata/impl/lookup/fail_todo_undefined_impl.carbon`
* `toolchain/check/testdata/struct/import.carbon`
* `toolchain/check/testdata/tuple/import.carbon`

The last two are due to an import of generics bug exposed by this PR,
which will be fixed in a follow-on.

---------

Co-authored-by: Josh L <[email protected]>
Co-authored-by: Richard Smith <[email protected]>
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

1 participant