Instead of evaluating a non-parameterized class or interface to a
constant with `SpecificId::None`, use the self specific for that class
or interface, which will not be `None` if there is an enclosing generic.
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`.
Unintentionally, we were adding ImplWitnessAssociatedConstant to the
witness table and then immediately evaluting and replacing it with its
constant value instruction, which is incorrect. The point of the
instruction is to be a symbolic value in the witness table that is a
dependent of the generic impl declaration being built.
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.
Each of these types takes another type as an operand. Instead of storing
that other type as a `TypeId`, store it as an `InstId` so that we can
track how it was written, not only its canonical form.
The canonical constant values of these types continue to store the
canonical constant values of their operands, as normal.
---------
Co-authored-by: Dana Jansens <danakj@orodu.net>
Give them a value representation of copy, and allow conversions between
two facet values of the same type to work.
Convert was assuming that facet values are compile time constants, but
thye can also be runtime values. In that case, we have no support for
converting to a different facet value of a different facet type. But if
the types are equal then it's all fine.
In theory it seems that we should be able to convert if the target facet
type can be found through the source value's FacetType. But currently
that happens through impl lookup and it requires constant values. Adding
a test for this.
Related to #5241
Factor out logic to evaluate `EntityNameId` instead of duplicating it
between `BindSymbolicName` and `SymbolicBindingPattern`. Remove support
in `SymbolicBindingPattern` for evaluating a pattern to the constant
value of the corresponding binding, which doesn't really make any sense
given that patterns don't generally evaluate to the value that they
matched.
This results in the handling for `SymbolicBindingPattern` being simply
the default handling for an always-constant instruction, so remove the
special case for it entirely and change its constant kind to `Always`.
It's not entirely clear that it makes sense for `SymbolicBindingPattern`
to be treated as a constant when other patterns aren't, but we seem to
be relying on this in various places, so leave it as a constant for now.
Changing it to never be constant will be a smaller change now -- it just
requires changing the `constant_kind`.
The IR changes in the tests are fairly widespread, but mechanical, and
there are two kinds of things changing:
- `symbolic_binding_pattern`s in specifics now evaluate to
`symbolic_binding_pattern`s, not to the argument values. This means in a
few cases we end up with additional `symbolic_binding_pattern`
constants.
- We evaluate the type operand of `symbolic_binding_pattern` now, so an
error in the type will now properly be propagated into an error in the
pattern's constant value.
Don't look for a user-defined conversion (implementation of `As` or
`ImplicitAs`) if the builtin conversion to a facet type fails impl
lookup. This is the behavior we want, and reduces noise in diagnostics.
Partial implementation of #5122. Still to do:
* Give an error if the users tries to implement such a conversion, since
it is now unreachable.
* Add notes to the diagnostic explaining why impl lookup failed.
---------
Co-authored-by: Josh L <josh11b@users.noreply.github.com>
The instruction does act somewhat like a witness, saying that an impl
does exist for a lookup, but the instruction more concretely represents
an impl lookup - since that is done when it is evaluated.
As of #5087, these terms are no longer synonyms. This change preserves
the original meaning.
---------
Co-authored-by: Josh L <josh11b@users.noreply.github.com>
Add a new instruction called ImplSymbolicWitness which represents a
search for an impl declaration given a self type and an interface to
find implemented for the self type. The self type is stored as a
constant instruction id, rather than as a ConstantId, as instructions
don't currently support holding ConstantId. The interface is stored as a
SpecificInterface but we can't fit all of it directly into the
instruction. So we add a new id to refer to the SpecificInterface as
follows.
Add a new SpecificInterfaceId which indexes into a canonical value store
on SemIR::File. This tracks all `SpecificInterface`s stored in an
instruction - specifically the ImplSymbolicWitness instruction.
The SpecificInterface on Impl is still stored there as a value, not as
an id, and no id is eagerly constructed for it. We wait until an id is
needed to make one. Since they are canonical, a new id is only create
when a new SpecificInterface value is seen.
When doing impl lookup, and the query is not concrete, and the impl is
not effectively final, the query needs to consider future impls that may
specialize either the self type or the constaint to make a more precise
match and replace the found impl declaration. Instead of returning the
ImplWitness instruction from the found impl, we generate a
ImplSymbolicWitness instruction, storing the query so that it can be
replayed later. This instruction is added to the generic eval block and
thus will be re-evaluated later with a SpecificId that may make the
query more concrete. When evaluating the instruction and replaying the
query, the lookup has the same conditions and if it does not decide to
use the found impl concretely, then the same instruction is returned
from eval, leaving it as symbolic.
--- Impl lookup changes ---
Impl lookup gets a little more interesting now. It continues to look in
the facet value for a witness if the self type is a facet value. Then
falls back to looking for an impl declaration. This step is no longer
done directly. Instead, we construct a ImplSymbolicWitness instruction
and evaluate it immediately for each interface that are in the query
facet type.
The ImplSymbolicWitness instruction, when evaluated, calls back to the
impl lookup code, with a query specific interface. There we resume back
into the same code path as from before, finding a witness in an impl
declaration. But we may return "found a non-final impl" instead of a
concrete witness. If eval receives this back, it evaluates to the
current ImplSymbolicWitness instruction as the resulting constant value.
To pass lookup failures back through eval, a result of InstId::None from
the second step of impl lookup will result in a non-constant value,
which is used as a signal back up the stack to the original impl lookup
function that the lookup failed. Using a non-constant value here would
break evaluation of the generic eval block if impl lookup could fail
there, however we know it will not since we only leave behind an
ImplSymbolicWitness instruction in the eval block if we found at least
one matching impl already, and we just want to look for a better match
with a more specific query.
We must take care to not store a reference into any value store across
computation in impl lookup, since impl lookup can recurse into itself
invalidate those stores. That includes the SpecificInterface obtained
from a SpecificInterfaceId, which impl lookup also inserts into the
store.
--- The long tail ---
Adding a new instruction and a new id type requires a myriad of changes
to support them:
We add Dump() support for SpecificInterfaceId. And fix a crash in Dump
for SpecificId::None. We also add MakeSpecificInterfaceId() for dumping
arbitrary ids.
The type of ImplSymbolicWitness is a new singleton builtin type
instruction called WitnessSymbolicType (like WitnessType is the type for
an ImplWitness).
Both ImplSymbolicWitness and WitnessSymbolicType are given `Value` as
their expression category as they are builtin constant values. And
BuildInfo() in TypeCompleter is taught about them both, returning a
`ValueRepr::Copy`.
WitnessSymbolicType is added to the set of SingletonInstKinds, so that
it can have a singleton instrution id as a static member.
Lower's BuildTypeForInst() is taught to make an empty struct for
WitnessSymbolicType, similar to WitnessType.
Instruction formatter (FormatterImpl) grows support for printing a
SpecificInterfaceId so that it can print both arguments of
ImplSymbolicWitness on the RHS when printing the SemIR instruction. To
print a SpecificInterfaceId, it prints both the interface id and the
specific id (if there is one). For example, for a query on a generic
interface `Z` with one parameter, the RHS includes the query, interface,
and specific:
```
%Z.impl_symbolic_witness: <symbolic witness> = impl_symbolic_witness %U, @Z, @Z(%U.as_type) [symbolic]
```
IdKind is extended to include SpecificInterfaceId.
InstFingerprinter is taught to look through SpecificInterfaceId and use
the interface and specific ids in the fingerprint.
InstNamer is taught about SpecificInterfaceId, counting the interfaces
when building an index. It is also tought about ImplSymbolicWitness,
using the name of the interface within and the `.impl_symbolic_witness`
suffix. For example, here the LHS is named after the interface in the
query:
```
%Z.impl_symbolic_witness: <symbolic witness> = impl_symbolic_witness %U, @Z, @Z(%U.as_type) [symbolic]
```
StringifyTypeExpr is taught about WitnessSymbolicType, which uses its IR
name since it's a singleton. And about ImplSymbolicWitness which uses
its constant value. The handling of ImplWitnessAccess also needed to be
adjusted, since it assumed that ImplWitnessAccess::witness_id would
always be a FacetAccessWitness, but it can now also be an
ImplSymbolicWitness. (It seems that the witness_id is also assigned
ImplWitness instructions, but those ImplWitnessAccess instructions don't
ever seem to get stringified in a diagnostic at this time.) At the
moment the ImplWitnessAccess with a symbolic witness is just stringified
as "<symbolic>", such as in:
```
x.carbon:1:2: error: cannot implicitly convert value of type `()` to `<symbolic>` [ConversionFailure]
let a: C(D).(Z.X) = ();
^~
```
There is a TODO left behind to include more information there.
The TypeStructure builder is made to handle WitnessSymbolicType and
WitnessType. These come up now in deduce where a generic impl will have
a ImplSymbolicWitness in a FacetValue for a generic self type. The query
may have a concrete ImplWitness in the same position. Since deduce tries
to deduce through the FacetValue, it tries to convert ImplWitness to
ImplSymbolicWitness, tries to do an impl lookup for `impl ImplWitness as
ImplicitAs(ImplSymbolicWitness)` and causes us to build type structures
with each of these.
Subst is updated to handle pushing and popping SpecificInterfaceId.
Without this, when finishing a generic's eval block, we would walk into
the ImplSymbolicWitness instruction, and its arguments, and fail to
recurse down into the SpecificInterfaceId. Then any specifics inside
would be left as "orphaned" without any generic id attached to them, and
we would never update the instructions in the SpecificInterface's
instructions (inside its own SpecificId) with new constant values when
evaluating the generic eval block against a specific. To do this we push
the specific_id inside the SpecificInterface, and when popping we pop
the specific_id then construct a new canonical SpecificInterface with it
and return that id.
We add support for importing ImplSymbolicWitness by importing its self
constant instruction and specific interface id. However we also had to
add import support for SpecificImplFunction, which can now appear in the
generic eval block for a generic impl declaration, and thus must be
imported with the declaration. This is done very similarly to
SpecificFunction, except the `type_id` is a singleton value.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
When transforming instructions with symbolic constant values into the
eval block, we previously special-cased `bind_symbolic_name` (and
`symbolic_binding_pattern`) because they are places where symbolicness
is introduced, rather than propagated from operands, and just copied
them into the eval block. However, `bind_symbolic_name` can be dependent
on other symbolic constants, because it can have a type that is
dependent. In this case, the copy in the eval block would not have its
type properly adjusted to refer to the type within the eval block.
Fix this by performing substitution into `bind_symbolic_name` rather
than copying it directly, and instead, detect cases where substitution
determined that the instruction was unchanged despite having a symbolic
constant value, and force it to be rebuilt in that case.
I've not found any way that the previous behavior actually caused
problems, or affected the observable behavior of the toolchain. The type
of these instructions in the eval block doesn't make much difference to
anything because they get immediately replaced by their corresponding
argument values when we run the eval block. But this came up and caused
some test output churn when I was making a different change, and it
seems like a fix to our representation even if it's not changing
behavior, so I'm splitting it out so it can be handled separately.
When a generic function declaration was encountered for the second or
more time, we would FinishGenericRedecl() for the function decl, but
this just popped the generic region stack and moved on.
The issue with that is when the stack entry is gone, we lose the
symbolic constants from that declaration, and are unable to rewrite them
to point to the actual generic. This left us with a function declaration
with abstract symbolic values that were not useful, and in a function
call we use the declaration attached to the definition, which would be a
declaration with broken symbolic values. Then the function would be
uncallable since deduce would be unable to determine argument types
without the generic bindings.
This resolves the issue for functions, as well as ensuring the correct
generic id from a previous declaration is used for other generic entity
types that have redeclarations.
When a function declaration is qualified, such as defining a class
method outside the class body, we need only the function declaration to
contribute to its generic region stack. The code was collecting constant
values from all qualifier segments together incorrectly.
So when we PushNameQualifierScope(), we also drop the current generic
region stack and rewrite its constant values by calling
FinishGenericRedecl(), and open a new stack entry for the next part of
the qualified declaration.
If a generic declaration somehow has more dependent instruction than a
previous declaration, it would add new instructions to its eval block
with indices beyond the elements in the actual declaration eval block,
since we only store the block from the first declaration found. To avoid
this we plumb through that we are in a redeclaration, and terminate with
an ICE instead of adding new instructions to crash on later.
Fixes#5136.
It's helpful for stability to not have the path in the repo reflected in
test files, something I'm separately running into. So to reduce this,
align INCLUDE-FILE with other split behavior:
- Use the filename (with a "include_files/" subdir to disambiguate),
rather than the full path.
- Note if we eventually want to support splits in these, the same
approach could be extended.
- Only provide as an arg if the user requests files as args.
Factoring AddFile back because it's hard to share; I'm also advocating
to remove the prelude manifest, which would mean the remaining call
could be removed.
If a self type is FacetAccessType, we look through it at the facet
value, both for the query and the impl declaration. This ensures that
while FacetAccessTypes match each other for constant value equality
still, they an also match with facet value queries, such as:
```
impl forall [T:! Y] T as Z {}
^ FacetAccessType for BindSymbolicName of type FacetType(provides Y).
fn F(y:! Y) {
y as Z;
^ Facet value
}
```
Here the facet value and FacetAccessType don't have the same constant
value. Deduction will give the `T!: y` binding the facet value of `y`,
but the `T` in `T as Z` is a FacetAccessType to that binding, which is a
different constant value. Looking through the FacetAccessType gives us
the desired constant value for comparison with the query.
Additionally if the query self value is a FacetValue instruction, look
through that at the underlying type value. Impl lookup is used to
convert from one type or facet value to a new facet value of the desired
facet type. We want facet values to always be able to convert to
everything possible, rather than to have that restricted to just their
current FacetType:
https://github.com/carbon-language/carbon-lang/issues/5137. So this
allows queries such as `(C as Y) as Z` for a class `C` and interfaces
`Y` and `Z`.
Resolves TODO by creating new function `GetAssociatedValue` with the
logic from `PerformCompoundMemberAccess` restricted to the
non-instance-binding case.
---------
Co-authored-by: Josh L <josh11b@users.noreply.github.com>
When performing a call through an impl witness, the callee that we
type-check against is the function in the interface, so we form a
specific for that callee. However, once the impl witness access
resolves, the eventual callee is a different function -- the function in
the impl -- so this would cause us to form a `SpecificFunction` where
the callee is one function but the specific refers to a different
function.
Address this by adding another instruction, `SpecificImplFunction`, that
takes a function in an impl and a specific for the corresponding
function in the interface, and computes and returns a `SpecificFunction`
referring to the corresponding specific function in the impl, or returns
a direct reference to the function in the `impl` if it's not a generic
function.
This flows out of #5084 and trying to reduce UnsafeMake use. It turns
out imports and namespaces were using unexpected node kinds (previously
ImportIntroducer instead of ImportDecl, for example). This fixes and
adds validation.
I was uncertain about whether to just remove the is_convertible check,
since I don't see it as motivating creation of a conversion between
NodeIdOneOf types. So I've just left a TODO for now.
According to approved syntax at
https://github.com/carbon-language/carbon-lang/blob/trunk/proposals/p3848.md#syntax-defined,
`fn F[]` without explicit parameters should be valid. This makes it
work, then adds some validation to prevent `class C[]` in check.
Note that for `fn`, positional parameters are a TODO -- but this allows
me to test validation in `fn destroy[]` which is rejected, not just a
TODO.
New planned direction is to not require completeness. Updated comments
to reflect that some care will be needed once named constraints are
supported. Long term plan is [discussed in
#5089](https://github.com/carbon-language/carbon-lang/pull/5089#discussion_r1985908453):
> We have some options. In our last conversation, it sounded like it
would be beneficial for named constraints to have their own witnesses,
with entries in declaration order. This would allow accesses to the
named constraint while it was being defined. So there would be something
of a hierarchy in a facet type witness, with a named constraint taking a
single slot in a facet type witness, independent of its definition.
Co-authored-by: Josh L <josh11b@users.noreply.github.com>
When converting from a facet value (an instruction whose type is
FacetType), we require making a FacetAccessType
to have a type instruction when building the resulting FacetValue.
Otherwise, the conversion is the same for values of type TypeType, and
we relax the convert function to support either.
Corrects the test expectations for converting `Goat as Animal`, a facet
value of type FacetType, into `Eats`, a facet type of type TypeType.
This would be a promotion in the typish hierarchy which is incorrect. We
had an extra case in Convert that was handling this, and it's now
removed. `Animal`, a facet type, does still correctly convert into
`Eats`, a facet type, if `impl Animal as Eats` exists.
When converting to a facet there are three different failure modes:
1. You provided a non-type value. Only types can convert to facets. So
we tell you that we found a non-type value.
2. You provided a facet type (which has type TypeType) which does not
have witnesses for the the target facet's type. So we tell you that the
type `T` implements `X` but needs to implement `Y`.
2. You provided a (non-facet-type) concrete type (of type TypeType)
which does not implement the target facet's type (which is a FacetType).
So we tell you that we need the type to implement the FacetType but it
does not.
3. You provided a FacetAccessType (which is of type TypeType also, but
we special case this), whose underlying FacetType is not compatible with
the target facet's type. So we tell you that we need the type to
implement `X` but found a FacetAccessType `T` which implements `Y`.
Closes#5027
This changes the SemIR of invalid redeclarations, because previously
they lacked a name. We've avoided this in diagnostics so it doesn't
otherwise come up, but I plan to use it for more easily validating
redeclarations.
This allows the numbering of the parameters to match when checking for a
valid redeclaration. It also prepares us to produce the proper numbering
when generating a thunk.
This required allowing incomplete facet types where previously
completeness was required. Once we support named constraints, we will
need a way to consistently go from an interface to a facet type witness
index without requiring the interface to be complete in these cases.
---------
Co-authored-by: Josh L <josh11b@users.noreply.github.com>
This makes inline and out-of-line functions produce the same type.
I was looking at this and wasn't sure if it was deliberate. It seems
desirable to reuse the type when possible, since it's probably cheaper
too.
The INCLUDE-FILE option is only used in the toolchain tests for now. If
specified in a file test, the given file path is added to the test's
arguments. For toolchain tests this makes the file's package available
to the test. The `--custom-core` command line flag is added to the
driver, which avoids adding the production `Core` package to the command
line. Together, these allow a test to provide their own minimal `Core`
package.
For example, this would replace `Core` with the package and prelude in
`facet_types.carbon`.
```
// INCLUDE-FILE: toolchain/testing/min_prelude/facet_types.carbon
// EXTRA-ARGS: --custom-core
```
To support this:
* //testing knows how to parse INCLUDE-FILE out of the header of a test
file.
* //testing adds the file to the virtual file system, and includes it in
the test's arguments.
* //toolchain/driver grows the --custom-core command line flag to avoid
loading the production `Core` package.
Tests that were creating their own minimal prelude to define BitAnd on
types are now pointed to
toolchain/testing/min_prelude/facet_types.carbon as the prelude. They no
longer need to `import Core` in each test as a result.
Such tests are no longer `no_prelude`, but instead have their own
prelude. So they are moved to a `min_prelude` subdirectory.
Closes#5076
A query facet type may contain multiple required interfaces, in which
case impl lookup should return an ImplWitness for an impl that is used
for each interface in the query. We bundle these together into an
instruction block and return that from impl lookup. The witnesses are in
the same order as the interfaces in the
`CompleteFacetType::required_interfaces`. This allows walking the
`required_interfaces` to find an interface to give an index that can
also be used to grab a witness from this set, or from FacetValue.
FacetValue now has an InstBlockId for the set of witnesses of the
FacetType, instead of a single ImplWitness instruction id.
FacetAccessWitness includes the index of the witness (determined from
the position in `required_interfaces`) of the witness it's accessing
from the FacetType.
The
toolchain/check/testdata/facet/no_prelude/fail_todo_call_combined_impl_witness.carbon
test demonstrates the fix in the resulting SemIR. We can see the calls
to methods on a multi-interface FacetType result in a FacetAccessWitness
with an index of the correct interface, and this results in a witness
that leads to the correct impl's function.
There is a TODO in member access, where it does not have a
`CompleteFacetType` yet, so it uses the index in
`FacetTypeInfo::impls_constraints` instead, but this can be incorrect in
the presence of named constraints, which when completed can add more
interfaces to the `CompleteFacetType` and which are sorted into an
arbitrary order with the rest there.
---------
Co-authored-by: josh11b <15258583+josh11b@users.noreply.github.com>
Exclude SemIR since the point of the test is that it diagnoses something
invalid, not that any particular SemIR is produced.
Co-authored-by: Josh L <josh11b@users.noreply.github.com>
During SemIR, when identifying a specific in a specific context, we'd
have to either look through a specific or through a bound method.
Canonicalize which one to look through first, by having the BoundMethod
created around a SpecificFunction instead.
This changes a lot of check tests.
TODO: As the SemIr does not currently allow removal (access to insts()
is intentionally const), the bound instruction created prior to finding
the specific is not removed from the instructions.
Options: (1) leave as is, (2) add a way to remove the previous bound,
(3) rethink how/when the BoundMethod inst is created.
Implements the rule:
> For compound member access `a.(b)` where `b` names a _non-instance_
member of an interface `I`:
> * `a` is implicitly converted to `I`
> * let `T` be the result of symbolically evaluating the converted
expression
> * `impl` lookup is performed for `T as I`.
>
> Instance binding is never performed.
See
https://docs.carbon-lang.dev/docs/design/expressions/member_access.html#impl-lookup-for-compound-member-access.
Before this PR, non-instance members were treated as instance members.
---------
Co-authored-by: Josh L <josh11b@users.noreply.github.com>
- Explicitly document that `*Param` and `*ParamPattern` insts represent
`Call` parameters.
- Stop wrapping compile-time parameter patterns in `ValueParamPattern`
insts (because they aren't `Call` parameters).
- Document how `MatchContext::results_` relates to the `Call`
parameters, and be more consistent about when it's written to.
- Remove `RuntimeParamIndex::Unknown`: we no longer need to distinguish
"this `Param`'s runtime index is unknown" from "this `Param` isn't a
runtime param", because we no longer use `Param`s at all in the latter
case.
- Rename `RuntimeParamIndex` to `CallParamIndex`.
As a side effect of removing the `ValueParamPattern` insts, this fixes a
minor diagnostic bug where `NoteInitializingParam` didn't identify the
specific parameter that led to a deduction failure, because it expects
generic parameters to only be represented by `SymbolicBindingPattern`s,
but before this change they could be wrapped in `ValueParamPattern`s.
Here are some test changes so the diffs that come from my upcoming
functionality changes are easier to see. Upcoming functionality
includes:
* Compound member access with non-instance associated constants will
change to comply with the design
https://docs.carbon-lang.dev/docs/design/expressions/member_access.html#impl-lookup-for-compound-member-access
in #5059 .
* Impl lookup will add support for finding impl witnesses in facets
#5060
I'm also making the toolchain/check/testdata/impl/compound.carbon test
into a no_prelude version, and adding a version that tests with
importing.
---------
Co-authored-by: Josh L <josh11b@users.noreply.github.com>
TODO to resolve whether it should conditionally say "object of"
depending on the category
---------
Co-authored-by: Josh L <josh11b@users.noreply.github.com>
Replace the large and growing `TryEvalInstInContext` function with one
function per kind. While we still have special-case handling for a small
number of instruction kinds, most instructions are now handled either
fully automatically or use a common codepath that evaluates the
instruction operands and then performs an eval-context-independent
evaluation of the instruction.
To support this, `InstConstantKind` is expanded to describe more
fine-grained details about how each kind of instruction interacts with
constant evaluation. Also, the operand kinds of instructions become
slightly more fine-grained: we now distinguish between operands that
describe the destination of an initializing expression (`DestInstId`)
from other `InstId` operands, because `DestInstId` operands need
different treatment during constant evaluation. In particular, an
initializing expression can have a constant value even if its
destination is non-constant or has not yet been set, because evaluation
of an initializing expression doesn't include the store to the
destination.
Some minor test changes:
- We now more consistently propagate errors into the results of constant
evaluation, so more instructions that depend on errors have a constant
value of `<error>`.
- Diagnostic location for invalid array types now point at the whole
array type rather than the array index expression, because
`EvalConstantinst` doesn't have access to the original expression.
- Diagnostic for failed `RequireCompleteType` doesn't print the original
type any more because `EvalConstantInst` doesn't have access to the
original expression.
As a follow-up, some of this -- in particular, the `EvalConstantInst`
overloads -- will be moved to a separate file, in an effort to split the
overall constant evaluation machinery apart from the logic to evaluate
each individual kind of instruction.
Make facet types complete like other types. This means that in the body
of an interface, the type of `Self` is incomplete. This involved fixing
an issue where eval of a specific_id that was already canonical was not
resolving the specific declaration, which could occur as part of
substituting into a facet type.
---------
Co-authored-by: Josh L <josh11b@users.noreply.github.com>
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Co-authored-by: Dana Jansens <danakj@orodu.net>
Parameter lists need substantially different treatment than tuple
patterns in other contexts, so this change splits them into separate
parse node kinds.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
* Add `RequireCompleteFacetType` and `ResolveFacetTypeImplWitness` to
`check::Context`. Goal was to move code from `impl.cpp` (mostly) without
functional changes.
* Complete type information is cached with the facet type, and is stored
in a `complete_facet_types()` table.
* Main functional change is to diagnose attempts to use a rewrite
constraint on an associated function. Some existing diagnostics have
been updated.
* Remove `check::Context::RequireDefinedType`:
* For class types, use `RequireCompleteType`
* For facet types, use `RequireCompleteFacetType`
* Introduce a `SemIR::SpecificInterface` to hold an interface and
specific id pair.
* Keep the specific interface ids in the impl object.
* Avoid some extra copies in `Dump` functions.
* Future work missing from this PR:
* Resolving for member access or actions that require impl lookup.
* Resolving rewrites constraints that refer to non-concrete values.
* Any support for adding implied constraints that result from a `where`
clause (though TODOs have been added).
---------
Co-authored-by: Josh L <josh11b@users.noreply.github.com>
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Co-authored-by: Dana Jansens <danakj@orodu.net>
Richard Smith
Dana JansensandRichard Smith
Alina Sbirlea
Geoff RomerandRichard Smith
Boaz Brickner