Implementation of unused pattern bindings #2022, continued.
Whereas previous PR #6460 took care of parsing, and PR #6479 prepared
the stage by using _ in some test cases, this PR has the the actual
implementation, using a simple dataflow analysis.
---------
Co-authored-by: Burak Emir <bqe@google.com>
Co-authored-by: jonmeow <jperkins@google.com>
Currently each interface has a `Self` facet internally that becomes a
binding to every entity inside the interface: associated constants,
functions, and require decls. Each of these has to be independently
generic as a result. This makes is challenging in extended name lookup
to move into an extended scope of an interface, as we have a specific
for the interface, but the names within require a different specific
that includes a `Self` facet value.
We generalize this relationship by adding a second generic to Interface,
called `generic_with_self`. When we want to work with entities inside
the interface, we move from the interface-without-specific to the
interface-with-self specific by adding a Self to the specific. This is
done independently of any particular entity inside the Interface, as
those entities are now all members of the interface-with-self generic.
Associated constants no longer need a generic of their own, as they do
not have separate generic bindings. Functions retain a generic, but if
the function has no generic arguments, it will have no bindings of its
own now.
Require decls retain a generic so that their specific can be
instantiated separately from the interface. Requiring the interface to
be complete does not require the types in a require decl to be complete
unless it is modified by `extend`. So we allow them to be completed
later by keeping them in a separate generic.
Named constraints look like interfaces and gain the additional inner
generic-with-self, with the same relationship to require decls.
This removes the need for name lookup to perform Substitution of a Self
facet into the extended scope instruction. Instead, the
`SpecificConstant` instruction inserted by a `require` decl is part of
the interface-with-self generic. When looking through a FacetType for
extended scopes, for each interface, we push the scope with the specific
for the interface-with-self. Then the constant value of the
`SpecificConstant` is correctly modified by the provided self
automatically through applying that specific.
This gets us a step closer toward resolving TODOs in member access
around facets, by making the lookup into a facet value a "lookup in
base" operation instead of a "lookup in type of base". However the base
given to find scopes in still remains the facet type of the facet, which
is still a TODO.
Then we can simplify the "lookup in type of base" case a bit, with a
single code path doing the name lookup step. But we keep a TODO where if
the type of base is a facet, we change the lookup target to be the facet
type of the facet instead.
This is toward having name lookup into an interface that is extending a
named constraint work correctly with a `Self` in its specific. To
perform that name lookup, we will need to tell name lookup what is the
base, so that it can replace `Self` with the base. This change gets us
in a position where we can correctly provide the base in the `T.F()`
(lookup in facet) and `t.F()` (lookup in type of facet) correctly and
straightforwardly.
We provide a marginally improved diagnostic when looking into a facet
with an incomplete facet type, which will move into
AppendLookupScopesForConstant once we are looking into the facet
directly instead of its type.
Pursuant to recent decisions on #6124, switch `Destroy` to use a
`CustomWitness` for its implementation. Right now this is manufacturing
no-op implementation functions on each lookup, which obviously isn't
ideal but is intended as a first pass. I'm mostly trying to find the
right balance between updating the approach to reflect new decisions,
while still breaking apart work in a way.
The `CoreInterface` logic is intended to build on `CoreIdentifier`
support. We have a number of additional interfaces that require
specialized logic, and that'll extend pretty far with C++ interop, so it
seemed easiest to have a generic function for it. That's what's
replacing the logic inside C++ interop that was doing string comparisons
(which could have already been moved to `CoreIdentifier`, I just missed
it in my first pass).
This adds `CustomWitness` support because the `Destroy` witnesses can be
imported cross-file. `CustomWitness` was previously only used for C++
types, which don't yet support import, which is why that wasn't
previously an issue. The addition of `query_specific_interface_id` is
similarly needed in order to get correct sorting of witness blocks when
imported.
This PR also removes builtin constraint logic (note this is in a
separate commit to help review; it's not a separate PR because it's
difficult to split apart without tests breaking). This had been made
generic with the expectation that destroy, copy, move, and conversions
would all need related support. Under the new decision, we are not going
to do blanket impls and will instead just manufacture a `CustomWitness`
for everything.
A lot of SemIR fingerprints change, but that's probably because the
addition of `Destroy` on core classes is yielding structural changes.
Instead of naming the root namespace `package` (because it's accessed by
the `package` keyword), change it to use the current package name. Note,
buried in the checksum changes,
`toolchain/check/testdata/package_expr/fail_not_found.carbon`:
```
- // CHECK:STDERR: fail_not_found.carbon:[[@LINE+4]]:16: error: member name `x` not found in `package` [MemberNameNotFoundInInstScope]
+ // CHECK:STDERR: fail_not_found.carbon:[[@LINE+4]]:16: error: member name `x` not found in `Main` [MemberNameNotFoundInInstScope]
```
for:
```
// CHECK:STDERR: var y: i32 = package.x;
// CHECK:STDERR: ^~~~~~~~~
```
I'll leave it to you if you prefer this; the alternative I see is to
just rename `IsCorePackage` to `IsImportedCorePackage`, and/or change it
to a helper that takes a `Context` and does the right thing with
`parse_tree` (which, I need for `Destroy`-related reasons and was my
default approach).
Per discussion, makes all symbolic local bindings a TODO. We should
implement them more correctly before making them operable. Right now
things partially work, but because constants behave mostly right in the
symbolic situations under tests. More broadly, it has incorrect behavior
and crashes, thus the TODO.
This converts most tests using `let` to instead using parameters, but
leaves some behind where a conversion either didn't make sense (e.g. in
`let` tests) or a conversion was unclear to me (multi-layer `let`, which
relies more on planned behavior that seems more bespoke to a local
`let`).
In let's `fail_generic.carbon`, there's a "// TODO: Should this be
valid?" that I'm removing because my understanding is the code in
question should be valid (the file is merged into let's
`generic.carbon`).
Refactoring `HandleAnyBindingPattern` a little because there's a TODO to
make it shorter, and it seemed like a reasonable drive-by change (let me
know if you think there's more I should do, or if I should remove said
TODO even though it's still a bit long).
Fixes#5982
This is a prerequisite for support for interop with C++ template names.
No behavior change here, except that it sadly changes the fingerprinting
for a lot of tests.
Give TupleLiteral and StructLiteral a constant value, if their contents
have constant values. Their constant values are TupleValue and
StructValue respectively. This supports their ability to convert to a
constant type (or facet type).
This way when deduce finds a TupleLiteral as the argument to a
_symbolic_ facet type, it can also find a constant value to use for that
argument. This allows deduction to move onto step two, where it can
substitute into the symbolic parameter from previous deduced arguments,
and then perform the conversion from the TupleValue to the desired facet
type.
Allow `PerformBuiltinConversion()` to convert from a canonical
TupleValue or StructValue to `type` instead of only from literals. Then,
also support conversion from a symbolic binding of type TupleType or
StructType to `type`.
#6289 absentmindedly added fields in more places, and this is undoing
that plus further fixes.
This does some cleanup of types with relation to singletons. For
`TypeType` and `ErrorInst`, they're always complete due to a
`SetComplete` call in `file.cpp`. For `CppVoidType`, it's intended to be
incomplete by construction, and so a `TypeId` should be okay. The intent
though on not generally providing these had been that `GetSingletonType`
needs to be called to get a type to be marked as complete.
In the case of `AutoType`, removing `TypeId`does change a small printing
detail. I think that's old legacy that's just been carried forward.
Otherwise, for both `InstType` and `AutoType`, I've added
`GetSingletonType` calls where they were used in order to ensure
completeness is applied correctly. These calls cause small SemIR
permutations.
This causes `AutoType` to be seen by lowering, so I'm adding a
placeholder for it. Also merging two functions that look like they're
identical in intent -- not sure why they're separate.
---------
Co-authored-by: Dana Jansens <danakj@orodu.net>
This resolves a TODO in `expr_info.cpp` by using the inst kind rather
than the bound value to track the binding's category.
Since we're churning all the `bind_name` insts in testdata anyway, I'm
also taking this opportunity to align the inst naming with the design's
terminology, by calling these insts "bindings" (this aspect of the PR is
dependent on #6231 resolving an ambiguity in that terminology). For
consistency we'll need to rename several other insts as well (see the
TODO on `RefBinding`); I'm deferring that to a separate PR to minimize
the review load, but I think those name changes are in-scope for this
review.
The SymbolicBindingType refers to the type value that will be
substituted in for the BindSymbolicName, but holds onto the EntityNameId
from the BindSymbolicName instead of (or in addition to, for now) the
instruction.
The EntityNameId will be used to look in the ScopeStack to find the
witnesses either from the BindSymbolicName instruction, or other
instructions that specify `impls` constraints against the EntityName.
This will allow us to have the `T` in `I(T)` resolve to a `.Self`
reference in the type so that we get type equality with the binding's
type: `T:! I(.Self)`.
The main direction of this change is the edits to `destroy.carbon`
(matching in both prelude and min_prelude).
Previously there was a no-op blanket impl for `Destroy`, which hid all
missing implementations of `Destroy`. This does a few things:
- Sets up builtin aggregate destruction for struct and tuple types as
before, but also adds C++ class types and array types to the same
handling. (all as a TODO for actual implementation)
- Also maybe-unformed destruction, for now at least. (there's a chance I
may try a different approach on this, but the impl lookup wasn't working
as I'd hope in order to write it in code)
- Adds handlers for simple things that are easy to do in code: `type`,
`bool`, pointers. (because these are no-op destruction)
- Redirect `const T` destruction to `T` destruction.
This leaves as future issues:
- `partial T` destruction. (this can't be done similar to `const`
because it only works for non-`final` class types; I think `class`
definitions should just generate what's needed)
- Destruction of other prelude-provided types. (will probably come up as
we implement class destruction, that the adapted builtin type doesn't
implement `Destroy` -- but may end up special-casing that in a way that
moots it)
This moves the `&` operator from `facet_types.carbon` to
`convert.carbon` because more things need to handle type and now that
we're getting separate copy and destroy interfaces. It should be
low-cost (an interface and builtin) so hopefully this is the right
balance for complexity and re-use.
A few tests are also edited in order to focus them more on what they
intend to test, and avoid a `Destroy` dependency.
This is in support of a goal of changing the blanket `destroy` impl to
use (roughly):
```
private fn CanAggregateDestroy() -> type = "type.can_aggregate_destroy";
// Handles aggregate type destruction.
impl forall [AggregateDestroyT:! CanAggregateDestroy()] AggregateDestroyT as Destroy {
fn Op[addr self: Self*]() = "type.aggregate_destroy";
}
```
That isn't done here because there's still other issues that migrating
raises. What this *does* do is add the builtin functions, and in
particular, support to `FacetTypeInfo` to make `CanAggregateDestroy`
work.
The "special requirement" approach in `FacetTypeInfo` allows us to
support restricting a blanket impl under the current approach of impls.
Maybe we'll find a cleaner approach that can work in the future, but
this fits into the current model by propagating similar to other
requirements. I'm using an enum mask because we have a number of similar
things to add (e.g. copy, move) but I'm not sure we need a full vector.
A few alternatives considered were:
- Supporting syntax more like `where .Self impls
TypeCanAggregateDestroy(.Self, SupportedInterface,
UnsupportedInterface)`. I think it'd be a little cleaner, but requires
better compile-time evaluation in order to assess the type of the call.
Right now it's expected to be a `FacetType` too early to make this work,
and I was concerned about pouring too much more time down this route.
- Providing an actual interface, in particular doing name lookup back
into `Core.` for an interface. This would've added name lookup overhead,
and the question of whether an `impl` exists.
- Generating an interface. This avoids the name lookup, but would still
raise the question of whether an `impl` should also be generated. Work
I've previously done generating interfaces for class destruction also
feels complex to both write and understand (an unfortunate issue).
- Still modeling as an `ImplsConstraint`, for example by defining a
special `InterfaceId::CanAggregateDestroy = -2` similar to what we do on
other ids. I was hesitant because of how this expands the number of
modes of `InterfaceId`, and things for consuming code to watch out for,
for what feels like a relatively niche set of use-cases that are only
interface-like.
---------
Co-authored-by: Dana Jansens <danakj@orodu.net>
If it's just `TypeType` then the `BindSymbolicName` appears directly in
type positions, but if it is replaced with another facet value, then we
would need to insert a `FacetAccessType` around it. By giving it a
`FacetType` type, like other `BindSymbolicName`s we make it consistent
and avoid having to introduce extra instructions.
This uses each vector's size as a barrier between lists, to eliminate
the possibility of incidental collisions between entries of different
lists. This is the same as is done inside `AddBlock`.
Decouples associated constants from being special cased in let handlers.
Enforces associated constant grammar restrictions in parsing instead of
checking.
Closes#5411
We add a virtual node (`CompileTimeBindingPatternStart`) as the first
child of `CompileTimeBindingPattern` which holds the identifier
underneath it, so that it is checked just before the type expression of
the `CompileTimeBindingPattern`. When we reach this virtual node during
check, we add `.Self` as a name in the current scope, and when we reach
`CompileTimeBindingPattern` we remove it from scope, which ensures it's
present during only the checking of the type expression for the compile
time pattern.
At the moment the `.Self` has a different type (it's a `TypeType`) than
other `.Self` in the facet type (which are a single `FacetType`), but
the intention is to immediately substitute it out of the facet type
entirely, replacing it with a reference to the compile time binding (a
`BindSymbolicName`) itself. A TODO has been added for this.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
This makes all `.Self` references in a facet type canonically the same
(which will remain true iff they refer to the same `Self` type in the
future), removing the need to do more complex comparisons between them
using the EntityName, interface, and index. This allows the comparison
of types containing `.Self` references to be done correctly regardless
of where the `.Self` appears, as such type expressions will all be
canonically equal if they otherwise equal now, regardless of whether
they are written in the context where `.Self` could have seen different
`Self` facet types.
In order to retain access to constraints on a base `.Self` facet type,
in the case of applying `where` to an existing facet type, we:
- Give the base facet type as a `RequirementBaseFacetType` constraint so
that eval of `WhereExpr` can find and copy all the constraints off of
it.
- Introduce eager/early rewrite constraint resolution, which allows a
constraint to eagerly resolve access to earlier rewrite constraints
(`where .A = () and .B = .A` is eagerly transformed into `where .A = ()
and .B = ()`) before the full constraint resolution step. This allows
use of rewrite constraints in larger type expressions, such as `where .A
= () and .B = C(.A)` and `C` will know that the argument is `()`.
In order to verify rewrite constraints at the end of
`LookupImplWitness()` we need to replace references to associated
constants in the query facet type with values that come from the query's
self. To do this, we find any `ImplWitnessAccess` that is a reference to
`.Self` and replace its witness with the witness found through the impl
lookup process, if the interfaces match. This allows the
`ImplWitnessAccess` to resolve to a concrete value if that witness was
concrete. Then we just need to compare that for each rewrite constraint
the lhs and rhs are the same constant value. If they differ, the self
provided a different value for one side (either through its own facet
value constraints or through an associated impl), or the self did not
provide a value at all.
For now, only .Self references in the top-level facet type are
rewritten. Nested facet types are not, even if they contain a .Self
reference up to the top level facet value. This will be addressed by
adding numbering to the EntityName of of .Self in a BindSymbolicName.
See the third model in
https://docs.google.com/document/d/1Yt-i5AmF76LSvD4TrWRIAE_92kii6j5yFiW-S7ahzlg/edit?tab=t.0
for the plan. For now, there is a TODO addressing this.
Iterate over arrays by producing their elements in the obvious way. We
use `i32` as the cursor type because that's the type that check converts
array indexes to. This may need revisiting if we support arrays with
more than 2Bi elements.
Also includes a fix for an import crash bug that's triggered by this
change, borrowed from #5873.
Change impls from `<interface>.impl` to `<self>.as.<interface>.impl`,
and *member* functions to `<parent scope>.<fn>` (non-member functions
exclude their parent scope). Stop special-casing builtin functions,
given the new naming scheme.
The purpose of this is to make it clearer when a member function is
being accessed and, if so, which member function. In particular, we
often access interface `Op` functions. The builtin function
special-casing was intended to help with that, but we still have lots of
`Op` functions. This particular approach should make the interactions
clearer.
This changes up queueing of block IDs a little because, in particular,
we need to process bodies of entities only after constants finish
processing. But, it should also result in less memory usage during
processing because it means we have less on the insts stack at any given
time, since we track a block rather than all instructions contained by
the block.
This switches from the `CollectNamesInBlock` approach for entities, to
instead traversing entities as they're encountered. For example, when
traversing constants, when a type is found, the entity will have its
block queued for processing.
This leads to a change in the traversal order, which affects
disambiguation done by numeric sequencing (since that's just showing the
traversal order).
This will allow for simpler "name based on name" logic. This is
something I plan to use for:
- impls: `<type>.as.<interface>.impl`
- functions: `<entity>.<member function>`
- Note an impl may be used as the entity for a bound function.
By naming the entities as they're encountered, I'll be able to rely on
the generated names rather than recalculating them.
To assist this, I'm also differentiating between the ambiguous and
disambiguated name. Otherwise, we could end up with things like
`<function>.<disambiguator>.<call>.<other disambiguator>`, where the
repeated disambiguator may not be necessary in order to get full
disambiguation. It's also a smaller delta from the current output.
Note, changing `Name` to a class felt appropriate given its shape. I was
also noticing that parts of its API were unused, and the class helps
detect unused private members.
---------
Co-authored-by: Geoff Romer <gromer@google.com>
In a facet type constraint, you can write `where .Self impls T` for any
facet type `T`, or the constant `type`. It is possible to write `type`
in different ways though, with a `NameRef` instruction appearing on the
RHS instead of `TypeType`. In this case, the canonical constant value's
instruction will still be `TypeType`, so make eval look at the canonical
instruction to see this.
Add a test with an `alias Type = type` which hits this case.
After this change, we only will accept and find one of the following on
the RHS of `impls`:
- `TypeType`
- A facet type
- An error, if the source code had something else there, which will
already be diagnosed. Tested by `fail_right_of_impls_non_type.carbon`
and `fail_right_of_impls_non_facet_type.carbon`.
So we handle these three cases, and drop the implicit handling of other
things which will never appear there.
`AddImportedInstruction` was turning errors in an instruction into a
Runtime constant value instead of an Error, which led to crashes when
importing an instruction that had an error inside it somewhere.
Fixes#5726
Replace the binary `Operation` interfaces with the `OperationWith(T:!
type)` interfaces described in the design, and add a `Result` associated
type for both unary and binary operations. Update the `impl`s in the
prelude for integer types to use the new form, including supporting
implicit conversion of either operand.
I've tried to split this PR up into commits focused on distinct changes
for review convenience. It may be simplest to review it one commit at a
time.
Now that included files can specify `EXTRA-ARGS`, this uses that to
handle min_prelude files. Also moves `As`/`ImplicitAs` out to a shared
file, partly because we duplicate it a few times over, partly just to
show that it works.
Also removes the `min_prelude/` subdirectories because many of these
files were touched by autoupdate regardless. I noticed one conflict for
`impl_thunk.carbon`, so renaming that one to
`impl_thunk_min_prelude.carbon`.
`GetArgReplacements` I simply noticed was unused, so removing it.
Where `--no-dump-sem-ir` is used, change to `--dump-sem-ir-ranges=only`.
Otherwise, add `--dump-sem-ir-ranges=if-present` with a TODO to change
to `only`.
Note, SemIR is affected just because the extra comments change line
numbers in files where splits aren't in use.
Previously we walked the global variables defined by the current file
and emitted an LLVM global variable definition for each of them. Now
instead, when emitting a constant reference to a global variable, we
emit an LLVM global variable declaration, and we then subsequently walk
the global variables defined by the current file and convert each of
them from a declaration to a definition.
In order to make import of names of global variables work, add support
for import of `var`, as well as support for importing `tuple_access` and
`tuple_pattern` in the case where the `var` has a tuple pattern in its
declaration. Also treat `bind_name`s that are reference bindings to
`var`s as having the same constant reference value as their `var` so
that we can properly import and lower them.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
Co-authored-by: Carbon Infra Bot <carbon-external-infra@google.com>
- Track the `VarPattern` instruction on the `VarStorage` instruction so
that it's available for name mangling.
- Mangle global variables based on the first binding name within their
pattern.
- Give global variables external rather than internal linkage, except if
they have no bindings whatsoever in their pattern.
- To support lowering references to bindings nested within a global var,
such as for `var (x: i32, b: i32)`, add some basic initial support for
reference constant expressions. Treat a global `var` as a reference
constant, and treat an aggregate access into a reference constant as a
reference constant.
I think this would probably have prevented the missed include in #5469
-- it would've just failed completely with a "missing prelude"
diagnostic.
Also note this excludes the included IR from output, because it's
probably low-value to print.
Instead of building an eval block as a separate pass at the end of a
generic, build the eval block incrementally.
The larger change here is that asking for the type or constant value of
an instruction now always returns an unattached type or constant value,
in order to preserve the behavior that we previously achieved by doing
the rewrite to attached types and constant values at the end of handling
the generic.
This also incidentally fixes some subtle issues where attached types and
constant values would leak out into check and cause it to get confused
about differences between attached and unattached values. Check should
no longer see attached values except where it explicitly asks for them.
---------
Co-authored-by: Dana Jansens <danakj@orodu.net>
Don't import `ImplWitnessTable` into the `constants` block, because we
generally don't put `Unique` constants there. This matches the handling
of the other kinds of `Unique` constants. In order to keep the
instruction visible in formatted SemIR, add it to the `imports` block
instead.
Also fix a bug in the instruction formatter that resulted in
instructions in the `imports` block being omitted from the output if
they were only referenced by earlier instructions in the `imports` block
and by instructions in the `constants` block. This was already resulting
in some referenced instructions being omitted from the output, but also
occurred frequently for `impl_witness_table` instructions after this
change because it is common for the only reference to those instructions
to be from `impl_witness` instructions in the `constants` block.
This was originally needed to support constant evaluation of name
expressions, but that's now done in a different way.
This is actually a step toward treating all patterns as constants. The
upcoming change will do so in a slightly different way, and so it will
simplify the review to start from a baseline where patterns are never
constant.
We eliminate the `FacetAccessWitness` instruction, which would sometimes
immediately evaluate to a concrete `ImplWitness`, and sometimes remain
symbolic. This instruction is now replaced by `LookupImplWitness` in all
cases. To support the same use cases, when it is evaluated,
`LookupImplWitness` will look in the self value if it's a facet value,
and attempt to return a concrete `ImplWitness` from it before looking
for an `impl` statement.
The `LookupImplWitness` instruction's value is now canonical, even when
it evaluates to a symbolic `LookupImplWitness` instruction, by
canonicalizing the self value of the lookup query. This canonicalization
unwraps `FacetAccessType` and `FacetValue` instructions to get to an
underlying canonical facet value. However we must preserve and use the
non-canonical query while evaluating the instruction in order to look
for a concrete `ImplWitness` if the query self value was a concrete
`FacetValue`. The canonicalization ensures that symbolic witnesses
obtained from a facet value are compatible with those obtained from an
impl statement, as long as the self types originate from the same
canonical facet value though they may have been narrowed.
Member access now unconditionally does a `LookupImplWitness()`
operation, instead of only sometimes doing the lookup for a final impl
declaration.
`EvalImplLookupResult` is marked `[[nodiscard]]` so that we don't
construct it and forget to return it. This was a mistake made at one
point during the creation of this PR. And the `has_concrete_value()`
method no longer has a precondition that `has_value()` is true, since we
want to look for a concrete result only in the new use of
`EvalImplLookupResult` returned from lookup into the query self facet
value.
The TODO from `FacetAccessWitness` evaluation is addressed by ensuring
the index of the witness in the `FacetValue` comes from the required
interfaces of the `FacetValue`'s type, and that the type (a `FacetType`)
is the same facet type used in the query to construct the `FacetValue`'s
witness block. This is made possible by eliminating the
`FacetAccessWitness` indirection. The lookup into a `FacetValue` happens
while evaluating `LookupImplWitness` and it does so directly on the self
value. This gives a consistent view of the witness set and the facet
type, as they both come from the same instruction.
All of this with 400 less lines of code. :)
---------
Co-authored-by: josh11b <15258583+josh11b@users.noreply.github.com>
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
This allows us to import the table for a given impl only once, while we
can import many ImplWitness instructions with different specifics for a
generic impl.
For example in convert_facet_value_to_narrowed_facet_type.carbon we see
that a single witness table is imported for the BitAnd interface, with
multiple witnesses (for different specifics) imported and sharing the
same table.
The ImplWitnessTable now contains a back-link to the Impl the witness is
for, allowing inst namer to name that interface in the textual semir,
and allowing the interface to be found when debugging from a witness.
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
In preparation for shifting from `TypeId`s potentially representing
attached types to always representing unattached types, using
[terminology suggested on
Discord](https://discord.com/channels/655572317891461132/963846118964350976/1359286326779973712).
This change causes us to track slightly more type spelling information
through SemIR.
One change that has significant impact on the SemIR output is that we
now build a `struct_type` instruction in each class representing the
types of the fields, including the spelling used for those types. This
is now no longer always identical to the corresponding canonical
`struct_type` for the object representation, so it's built separately
and owned by the class.
Also remove `TypeBlock` support entirely, as its only use was
representing `TupleType`s, which now use an `InstBlock`.
This follows the pattern used elsewhere, and allows facet types in eval
blocks to directly reference their operands instead of doing so
indirectly via a `ConstantId` attached to the generic. This prepares us
for making `ConstantId`s always be unattached.
In passing, add a stringified version of the `InstId` to diagnostics in
a couple of places where it seems useful.
Instead of using None, use an explicit ImplWitnessTablePlaceholder in
the witness table for entries that have not yet been populated, to aid
debugging. This would ensure they would show up very clearly in the
SemIR. This uncovered some `<invalid>` in the SemIR under erroneous
conditions that have now been turned into `<error>`.
Add the ImplWitnessAssociatedConstant instruction which wraps the
canonical instruction found from the constant value of the rewrite
constraint. This ensures that we have an instruction inside the eval
block for a generic impl declaration for each rewrite constraint's
value, which allows Subst to be performed to rewrite the symbolic
constant of the ImplWitnessAssociatedConstant instruction to associate
it with the generic. This will prevent the otherwise orphaned symbolic
constant of the rewrite's value from being used which can not have a
specific applied to them.
While applying the new insts in InitialFacetTypeImplWitness(), rearrange
the function to use less nesting. And avoid using entity names from
imported instructions (as we found is not effective in deduce.cpp) and
use a local instruction by going through the constant value.
This PR is part of the effort to allow a rewrite to name a generic
parameter, such as `impl forall [T:! type] T as Z where .X = T`, however
tests for this involve a final impl so that we can typecheck that the .X
value is a specific T, so the tests will come with that work. This piece
is split off because introducing new instructions causes a lot of SemIR
churn, and I wanted to get that done separately.
Instead of storing a `TypeId` that always refer to a facet type that
always contains exactly a single interface, store the interface
directly.
Also improve stringification of `LookupImplWitness` and witness access
into it, switching to using newly-added functionality for stringifying
specific interfaces.
Dana Jansens
Richard Smith
Geoff Romer
Elliott Kalt
