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

Replace InterfaceType with FacetType #4499

Merged
merged 58 commits into from
Nov 11, 2024
Merged

Conversation

josh11b
Copy link
Contributor

@josh11b josh11b commented Nov 7, 2024

This does a few things:

  • Replaces the single TypeId in the FacetTypeInfo struct with a vector of InterfaceId, SpecificId pairs (sorted in id order) representing the set of interface requirements of the facet type. This will later be used to support facet types with multiple interface requirements (as in I & J or I where .Self impls J).
  • Replace InterfaceType instructions (used as the type of an InterfaceDecl instruction) with FacetType instructions (introduced in Add facet type values and an instruction that produces them #4460) with a (newly introduced) FacetTypeFromInterface() function.
  • Replace code that consumed InterfaceType values with code that consumed FaceType values. I've generally left the assumption in the code that it is dealing with a single interface, using the (newly introduced) FacetTypeInfo::TryAsSingleInterface, and producing an error otherwise. There isn't yet support for the & operator or where .Self impls, so this is generally a good assumption for now, except you can get a facet type with no associated interfaces from a type where... expression. In some cases, the facet type value is pulled from the evaluation of an InterfaceDecl instruction, where the single interface assumption will hold permanently.
  • Some related cleans up: nicer stringification and formatting of facet types, suppression of some errors when there already was an error.

There is still a lot left to do, including:

  • Type type should be a facet type with a reserved id, replacing the built-in instruction.
  • Code using TryAsSingleInterface should generally be upgraded to handle more than (or less than) one interface. Name lookup should be particularly exciting.
  • Operator & should be defined on facet types, unioning their interface and other requirements.
  • Requirements from a where clause don't do anything yet.
  • Impls and impl lookup need to resolve facet types, and do things like determine if all the associated constants are given values.

toolchain/sem_ir/facet_type_info.h Outdated Show resolved Hide resolved
toolchain/sem_ir/formatter.cpp Outdated Show resolved Hide resolved
toolchain/check/context.cpp Show resolved Hide resolved
toolchain/check/context.cpp Outdated Show resolved Hide resolved
toolchain/check/eval.cpp Show resolved Hide resolved
toolchain/check/import_ref.cpp Outdated Show resolved Hide resolved
toolchain/check/import_ref.cpp Outdated Show resolved Hide resolved
toolchain/check/member_access.cpp Show resolved Hide resolved
@@ -8,19 +8,39 @@
// TIP: To dump output, run:
// TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/impl/fail_impl_bad_interface.carbon

// CHECK:STDERR: fail_impl_bad_interface.carbon:[[@LINE+10]]:1: error: semantics TODO: `impl as non-interface` [SemanticsTodo]
Copy link
Contributor

Choose a reason for hiding this comment

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

FWIW, losing this diagnostic seems like a regression to me, since it's the clearest explanation of what's wrong with this code, but we can presumably circle back to that later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The diagnostic was not particularly targeted before. It would print this anytime there was any error in the expression after the as.

@josh11b josh11b added this pull request to the merge queue Nov 11, 2024
Merged via the queue into carbon-language:trunk with commit a69c263 Nov 11, 2024
8 checks passed
@josh11b josh11b deleted the nointerface branch November 11, 2024 18:19
github-merge-queue bot pushed a commit that referenced this pull request Nov 12, 2024
This was broken by #4499 .

---------

Co-authored-by: Josh L <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants