When impl lookup results in an `ImplWitness`, either directly or later
through `LookupImplWitness` evaluating to an `ImplWitness` inside some
specific, it represents a dependency on the eval blocks of the
`ImplDecl` that the `ImplWitness` comes from. The `ImplWitness` contains
a `SpecificId` of the `ImplDecl`, and the constants in the witness table
must be viewed through that `SpecificId`. The constants may depend on
the declaration or the definition block of the `ImplDecl`'s specific, so
any use of the witness table from `ImplWitness` requires the specific
from the `ImplWitness` to be fully resolved: both the decl and
definition blocks.
The specific declaration block is always resolved immediately when
forming a specific id, so there's nothing to do there. And things are
simple when the `ImplWitness` returned from impl lookup is concrete. We
would simply resolve the specific definition directly in the evaluation
of `LookupImplWitness`.
But when the `ImplWitness` is symbolic, meaning there's a dependency on
generic parameters in its referenced specific, then we have a dependency
on resolving the specific definition in the `ImplWitness` for every
specific that gets applied to the `ImplWitness`, eventually resulting in
a concrete specific.
Previously we attempted to do this by representing a dependency edge
from the eval block containing the `ImplWitness` instruction to the
definition block of the specific inside the `ImplWitness`. We
represented this dependency edge with the `RequireSpecificDefinition`
instruction.
The challenge with the `RequireSpecificDefinition` instruction was
ensuring it always landed in the _same eval block_ as the one holding
the symbolic `ImplWitness`. Ultimately, we've found a way to do this
without the `RequireSpecificDefinition`. The witness table is accessed
through `ImplWitnessAccess`, so we resolve the specific definition
before each access to the witness table. If the definition is already
resolved, this just costs a lookup of the Specific from the value store.
We document the other things we tried, which don't work or were too
complex, below.
In PR #6236, we added the `RequireSpecificDefinition` instruction in the
eval of `LookupImplWitness`, but this is too late. If impl lookup
resolved to a symbolic `LookupImplWitness`, the `LookupImplWitness` is
added some eval block. Later, we form specifics and re-evaluate that
eval block, and the `LookupImplWitness` can evaluate to an
`ImplWitness`. Adding the `RequireSpecificDefinition` instruction at
this point inserts the instruction into whatever generic's eval block
happens to be on the top of the generic stack, not the eval block of the
generic we are building a specific for. And indeed we can't add
instructions to a generic's eval block later like this, as it would be
missing from specifics that were already resolved for the generic. This
incorrect insertion shows up as a crash in eval at the CHECK introduced
but commented out in [eval.cpp in
#7981](https://github.com/carbon-language/carbon-lang/blob/56d9bbc40add1e9263f2f9b2555c3a27292d1725/toolchain/check/eval.cpp#L3507-L3512).
This is covered by the
`require_specific_impl_definition_through_specific_instantiation.carbon`
test.
We tried moving the insertion of `RequireSpecificDefinition` to be
earlier - at the point where the `LookupImplWitness` is constructed, in
the `LookupImplWitness()` function, instead of inside evaluation. This
seemed like a straightforwardly correct answer, as the eval block that
contains the `LookupImplWitness` will then also contain the
`RequireSpecificDefinition`. However, this can cause us to lose the
`RequireSpecificDefinition` entirely. Because it's a separate
instruction, not an operand of `LookupImplWitness`, it does not get
treated as a dependency of the witness. When we do an impl lookup inside
an `alias`, we build a generic for the alias and then discard it. This
discards the eval block containing the `LookupImplWitness`. Then, in an
enclosing generic, we refer to the constructed `AliasDecl` instruction,
which pulls all its dependent operands into the enclosing eval block.
This pulls in the `LookupImplWitness` into the enclosing eval block, but
it does not preserve the `RequireSpecificDefinition`. This is covered by
the
`require_specific_impl_definition_from_lookup_impl_witness_in_class_definition.carbon`
test.
We tried having `ImplWitness` evaluation resolve the specific definition
of the specific `ImplDecl` it refers to. This seems like the simplest
approach by far as we should always be working with the correct eval
block and nothing can get lost. However it does not work because of the
recursive nature of `ImplWitness`. The `ImplWitness` instruction appears
in the declaration of an `ImplDecl`, and the `ImplWitness` holds a
`SpecificId` for that same `ImplDecl`. When building a specific
`ImplDecl`, we evaluate the `ImplWitness` in the `ImplDecl`'s
declaration block which then immediately resolves the specific
definition before the declaration is finished. This leads to missing
instructions in the specific definition. In short, this puts the
dependency edge to the `ImplDecl`'s specific inside the `ImplDecl`
itself, which is problematic.
We tried wrapping `ImplWitness` in another instruction,
`ImplWitnessUse`. Inside the `ImplDecl` declaration there is still
`ImplWitness`, but then in the definition there is `ImplWitnessUse` and
that is what is returned from the evaluation of `LookupImplWitness`. The
`ImplWitnessUse` is an `ImplWitness` plus it ensures the specific
referred to by the witness has its definition resolved. However this
breaks constant comparisons, as we get different witnesses inside an
impl versus outside.
The first iteration of this PR's solution had us insert the
`RequireSpecificDefinition` into the eval block when we sniffed a
`LookupImplWitness` instruction, however that missed cases where impl
lookup immediately resolved to a symbolic `ImplWitness`, which is why we
now sniff for the `ImplWitness` in the eval block. This is covered by
the
`require_specific_impl_definition_from_symbolic_impl_in_class_definition.carbon`
test.
We also originally made a mistake of not inserting
`RequireSpecificDefinition` when inside the impl definition that the
`ImplWitness` refers to. But we can form specifics ImplWitness values
through function signatures, and these must add the dependency edge on
that specific ImplDecl definition. This is covered by the
`require_specific_impl_definition_from_inside_impl_definition.carbon`
test.
Finally, we did have a working solution. It was to sniff instructions
added to the eval block of a generic, and when an `ImplWitness` is added
to the eval block (it is implied that it is symbolic), also add a
`RequireSpecificDefinition` to that eval block immediately. We also
resolved the definition of a specific referred to by `ImplWitness` when
`LookupImplWitness` evaluates to `ImplWitness` to cover the concrete
witness case. This complicated the construction of the eval block
immensely, however. It caused a situation where adding one inst to the
eval block actually adds two, which we needed to avoid when importing.
And it required inst-kind specific behaviour in the very general
construction of an eval block, completely outside of eval itself.
A `where` expression can have symbolic references to `.Self` inside it,
but those references do not represent symbolic instructions that should
be part of an enclosing generic eval block. They are internal details of
the facet type, and as such they are PeriodSelfSymbolic, not Symbolic.
The WhereExpr evaluation is careful to produce a Symbolic constant value
only if it has symbolic dependencies on the LHS of the `where`, which
avoids leaking the symbolic nature of `.Self` into he facet type's
constant value.
But until now, those PeriodSelfSymbolic insts could still leak into the
enclosing eval block if they ended up in an operand of a Call
expression. A compile-time Call expr evaluates to a TemplateSymbolic
phase constant if it's not concrete, which includes PeriodSelfSymbolic,
because we don't want to evaluate the Call expr until the `.Self` gets
replaced during identify. But that means the Call constant value, and
its operands including PeriodSelfSymbolic constants, all end up in the
eval block. We fix this by making Call evaluate to a PeriodSelfSymbolic
phase if its input phase is PeriodSelfSymbolic. This keeps the RHS of
the `where` expression self-contained. If the facet type is concrete,
then it doesn't add any instructions to the enclosing eval block (which
may not exist at all in a non-generic context). Regardless, when we
identify the facet type and replace `.Self`, the dependent Call
expression will be rebuilt as well and it will evaluate as appropriate.
We also enclose the `where` expression in its own generic scope. This
means that instructions which are not part of the facet type do not end
up in an eval block. They may be symbolic instructions, and they will
not get updated by a specific when it's applied the enclosing generic
anymore. But since they are not part of the facet type their
substitution is not required.
The new test had an incorrectly elided SpecificImplFunction instruction
in the resulting semir, as SubstPeriodSelf was using the location of the
input canonical instructions as the location for the results which
resulted in an instruction without a location. We fix that by using
`loc_id_` in the subst rebuild.
Formatting now saw the initialization of a value from an `eval fn` call
to a function in a witness table, so it needed to be taught to look
through ImplWitnessAssociatedConstant (which is what we find in the
witness table).
There were a number of iterations on the design that weren't reflected
in the initial port to the terminal rendering library. This PR tries to
complete the rendering side of things to match the design we landed on:
code-focused, with messages attached by lines to the code, and condensed
as much as possible to avoid the vertical space.
It also improves the styling to make things more visually
distinguishable such as adding a `->` to the main error line, and making
the underlines and connectors for it have a heavy weight (when the font
supports that).
Another major change is having multiple messages on a single line be
drawn in source lexical order. This requires the connectors to cross
without joining, but makes it easier to read the messages.
This PR also has some refactorings to enable these improvements, and a
_number_ of improvements to comment prose and terminology to try and
avoid jargon and make things read more naturally. Sorry for the fact
that Claude left a bunch of this mannerly language in comments despite
my efforts, but I'm continuing to try and drive the prose towards
something direct and simple to read. Some of the terminology
improvements also needed to be made in the design doc and those have
been made here.
Note that I don't think this rendering is perfect or should be viewed as
final. I'm happy to iterate on it further, and have a few adjustments in
mind already, but would like to focus this on getting the
_implementation_ of the current rendering design in place, and then we
can make targeted changes or tweaks as needed. For example, I'm not yet
fully happy with the rendering when messages are too long and we move
them all the way to the left, but am leaving that for a follow-up.
Assisted-by: Claude Code
Determining that a type has trivial destruction requires checking
whether all of its subobjects have trivial destruction and whether its
type has a custom witness for `Destroy.Op`. This isn't possible when
`Destroy` is the custodian for `Destroy.Op` and `SubobjectDestroy.Op`,
so we need to make `SubobjectDestroy` its own interface.
`Destroy.SelfDestruct` becomes a standalone function because it depends
on `SubobjectDestroy` and `Destroy` being implemented, and is also
responsible for generating any custom witnesses for both interfaces.
Making `SelfDestruct` an interface was experimented with, but this
required a blanket impl that caused issues for facet types.
`Destroy` depends on `SubobjectDestroy` to be implemented. We'd ideally
use `require impls SubobjectDestroy`, but it isn't complete yet, and it
also causes problems when `Destroy` is implemented inside the class
definition. We work around this by temporarily renaming the `Destroy`
interface to `DestroyImpl`, and introducing a constraint called
`Destroy` so that user code doesn't need to write `Core.SubobjectDestroy
& Core.Destroy`.
`FindStorageArgForInitializerInSpecific` is missing a `SpecificConstant`
case, and crashes due to the function not expecting `SpecificConstant`.
`SpecificConstant` should be handled by this function, as it derives its
expression category from the instruction it wraps.
Consider this example:
```carbon
interface I {}
interface J {}
constraint C {
extend require impls I & J;
}
```
Calls, such as `I & J`, are initializing expressions. `extend require
impls` wraps the call in a `SpecificConstant`. This code crashes when
asking for its storage, such as during formatting for `--dump-sem-ir`.
This commit adds a case for `SpecificConstant`, deferring to its wrapped
instruction when transitive lookup is allowed, and reports no storage
otherwise.
Assisted-by: Antigravity with Claude Opus 5.5
Update tests for observe equivalence chains to cover more cases and to
better test the things they meant to. The design is clarified to explain
that in an `interface` the types in an equivalence chain must all be
related to that interface except for one.
This is based on #7484 with some followup changes by me.
Co-authored by: ozguronsoy@gmail.com
Diagnostic rendering was part of the data model: `Loc::FormatLocation`
and
`Loc::FormatSnippet` wrote plain text to a stream, so no capability,
color, or
terminal width could reach them.
A diagnostic is now one message with labels, contexts, and location
information
attached, and `Diagnostics::Renderer` draws it into a
`Terminal::Buffer`. The
design is in `toolchain/docs/diagnostics_rendering.md` (#7668),
authoring in
`toolchain/docs/diagnostics.md`.
Before:
```
example.carbon:6:3: error: 1 argument passed to function expecting 0 arguments
Run0(1);
^~~~~~~
example.carbon:3:1: note: calling function declared here
fn Run0() {}
^~~~~~~~~~~
```
After:
```
error: 1 argument passed to function expecting 0 arguments
╭─┤ example.carbon:6:3
┆
3 │ fn Run0() {}
· ─────┬─────
· ╰──┤ calling function declared here
┆
-> 6 │ Run0(1);
· ───────
╰────────────
```
`--no-diagnostic-snippets` gives a compact form, `example.carbon:6:3-9`.
File
tests use it; the rendering tests under `toolchain/diagnostics/testdata`
ask for
snippets.
C++ interop diagnostics are data now rather than text Clang drew:
`SemIR::ClangSourceLocId` names a `clang::SourceRange`, and a Clang
location is
marked as the point it names.
`StreamConsumer` moves to a `:renderer` target that `:emitter` does not
depend
on, keeping `common/terminal` out of `consumer.h`. Capabilities come
from the
file behind the error stream, so tests get plain ASCII.
`renderer_fuzzer` draws diagnostics built from fuzzer bytes with every
capability combination, since a shape the renderer doesn't expect must
never be
why the compiler dies while reporting a problem.
TODOs mark the follow-ups: line weights and crossing lines, wrapping
that keeps
a quoted type whole, and ordering diagnostics that share a byte offset.
Also: `IncorrectExtension`'s hint moves into its message, since its note
had no
location of its own, and `strip_noise` in `check_diagnostics.py` blanks
multi-line `#define`s instead of deleting them.
Assisted-by: Claude Code
When unqualified lookup finds a name from an enclosing generic, create a
`specific_constant` if it's a constant, or a `specific_inst` otherwise,
to refer to that name from the inner generic. This allows consumers of
generic instructions to always use a specific of the current scope's
generic when accessing the constant value or type of a dependent
instruction.
Fixes crashes in dominance and in `GetExprCategory` caused by using
specifics for the wrong generic.
Assisted-by: Gemini via Antigravity
When identifying a facet type, defer evaluation of LookupImplWitnesses
by redirecting them to evaluate into IdentifiedWitness. Then the caller
of identify will need to substitute the IdentifiedWitnesses, either with
witnesses of their own, or with LookupImplWitness to cause a lookup
operation.
This allows:
- An `impl as` on a named constraint can subst the witnesses in
designators from the named constraint with a witness to the impl itself.
This prevents them from looking for and finding some other overlapping
impl. (This PR)
- Impl lookup to collect rewrite/equality requirements from all named
constraints and then use the impls it has already found to evaluate any
designators in those requirements instead of performing impl lookup
again (creating a cycle). (Done in
https://github.com/carbon-language/carbon-lang/pull/7944)
- Similarly, an `impl as` can collect impls and equality requirements
from all named constraints and verify them correctly by replacing
witnesses that should refer to the impl with the correct witness instead
of performing a lookup and finding an overlapping impl. (Done in
https://github.com/carbon-language/carbon-lang/pull/7937)
- ImplWitnessAccess to be able to find rewrites from a named constraint,
not just from the initial DeclaredFacetType. (TBD)
To do this we need to introduce the concept of "identified" specifics.
In order to identify a facet type, we must construct a specific for each
require decl in a named constraint. These specifics will have
IdentifiedWitnesses inside them, but their args would otherwise match a
specific for the require decl constructed in another context. To avoid
polluting the canonical specific namespace, we introduce a second
namespace of specifics that are "identified" since they are created by
identifying.
As we have introduced a new global state that modifies eval behaviour,
we must disable that state when resolving a specific (as per
https://github.com/carbon-language/carbon-lang/pull/7923). But inside an
"identified" specific, we preserve the behaviour so that the witnesses
modified by identify (through SubstPeriodSelf or through evaluating a
require specific) have their evaluation deferred.
We perform the same canonicalization as in LookupImplWitness, which
makes the output of eval more consistent across witness types, and
enables more straightforward constant comparisons between them when
needed.
This change partially implements [PR #7362], which revises how objects
are destroyed. It is a partial implementation for two reasons:
1. This change moves `Destroy.Op`'s current behaviour into
`Destroy.SubobjectDestroy`, but it doesn't add support for objects with
non-trivial destruction.
2. `Destroy.SubobjectDestroy` is a workaround for `require impls
SubobjectDestroy`. We aren't able to use the latter until the dependents
add their requirements' implementations to their own witness tables.
[PR #7362]: https://github.com/carbon-language/carbon-lang/pulls/7362
When a generic impl uses a default or final fn, it picks the specific
function value out of the interface to put in the witness table.
However, because this is done by modifying an existing instruction
block, the generics machinery has no hook to convert the function
constant into an attached constant, and because it was found in a
specific for a different generic, the constant inst will be unattached.
Fix this by manually mapping to an attached constant inst in the current
generic when building the witness table.
The type `type` is now a `FacetType` inst with no constraints. This
brings the model implemented in the toolchain into better alignment with
the language design. The `SemIR::TypeType` struct remains as a scope for
holding the `TypeInstId`, `ConstantId`, and `TypeId` constants, but is
not an `InstKind` anymore.
The `TypeType` inst looks a lot like singletons, but there are many
`FacetType` insts so it doesn't quite fit that model. So we put it
alongside singletons with a fixed inst id but refer to it as a more
general "builtin" inst that is not a singleton.
`Namespace::PackageInstId` is similar, and we group it with `TypeType`
conceptually as another builtin instruction with a fixed id.
No conversion is needed anymore to use a `type` as a facet, since types
also have a `FacetType` type. This simplifies and removes a number of
helpers and branches throughout the code.
The `TypeType` inst is now part of the constant store, so we end up
printing it in the constants block in every test. But it's also named
`type` rather than `%type` to preserve the majority of existing
formatting behaviour, though this does look different from other
constants.
Assisted-by: Opus 5 was used to generate a first draft and validate the
refactoring. Though nearly everything non-trivial the tool wrote has
been modified or rewritten.
In the `EntityName` for a binding, preserve the `TypeInstId` describing
how the type was written. When a diagnostic refers to that type via
`TypeOfInstId`, use the type-as-written in the diagnostic rather than
the canonical type.
Assisted-by: Claude Opus via Antigravity
Use a single `SemIR::Function` per `Core` interface method, whether it's
generated locally or imported. This prevents generating duplicate
functions, which lead to different types when the witness appears in a
`FacetValue` as part of a specific for a class.
We use a `CanonicalValueStore` of `GeneratedFunction` objects that allow
finding an existing FunctionId for a `Generated` special function before
(re-)generating it. Mangling for `Generated` functions is also moved to
use the values from the `GeneratedFunction`'s canonicalization key, so
that we have a consistent source of truth for the unique ID of a
`Generated` function across all files.
New tests are in
`toolchain/check/testdata/impl/custom_witness/destroy.carbon`.
This adds most support for default and final methods. Missing components
include rejecting definitions for non-default/final methods, and
permitting out-of-line definitions.
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Implementing interface modifiers causes an infinite loop when generating
fingerprints because the witness value generates a fingerprint that's
dependent on something dependent on the witness value. We've debugged
this to the witness table's `elements_id` field.
This hack is a workaround for creating a new block type whose value is
not codependent with its identity.
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
This solves the problem where `interface` imports are incorrectly
diagnosed as duplicate names in impl files.
Follows the implementation logic used in `handle_class.cpp`.
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
In Convert, we require the source value's type to be compete so that we
can look for `base` classes and `adapt` relationships. However these can
only be present in a `ClassType`, so we only need `ClassType`s to be
complete.
Reduce the requirement in Convert to not complete types that are not a
`ClassType`, and which can not contain a `ClassType` as part of their
class.
Ideally we would only _only_ require the `ClassType` itself to be
complete, and only if we're looking for a base or adapt. However lower
depends on us completing all Convert source types that contain a class.
This seems to suggest we're lacking checks for complete types somewhere
else and Convert is making up for it. A TODO has been added. The
`toolchain/driver/testdata/compile/optimize/optimize_debug.carbon` test
is an example that CHECKs due to failing to verify the LLVM module if we
do not compute the complete type of all class-containing types in
Convert.
The critical step this PR is doing is to stop trying to complete a
`FacetType` when converting from a facet. This avoids trying to complete
a named constraint when converting `Self` inside that named constraint.
Doing so causes a cycle when the conversion of `Self` is performed in
eval of an `extend require` decl, since requiring the named constraint
to be complete re-evaluates the `extend require` decl again. A test is
added that crashed in an infinite loop before this change.
It also depends on #7584, which was intended to be an optimization but
is now load bearing. Because converting `Self` leaves an impl lookup
inst behind, and if that inst is re-evaluated inside impl lookup (by
forming a specific of a `require` decl through identify) then we have a
similar cycle.
During impl lookup, for each (generic) impl candidate, we form a
specific for that impl by deducing its generic arguments. Then we
compare the query interface against the impl's specific interface. That
comparison needs the deduced arguments applied to the impl's specific
interface. Previously we were doing this by getting the impl's
constraint facet type with the impl's specific applied (via
`GetConstantValueInSpecific()`) and then identifying that facet type
with the impl's deduced self.
Identify is a fairly expensive operation. It runs subst, trying to
replace `.Self` references. It walks named constraints. It collects
require declarations. We're looking at making it do _more_ in the future
too, including rewrite constraint resolution and collecting rewrite and
same-type constraints. For this reason we have a cache to make it cheap
on the second run, but it's still a very heavyweight operation to
involve in impl lookup, when all we want is to apply the impl's specific
to its target interface.
We almost have all the information we need to avoid the identification
step. We have the impl's specific after deduction. And we have the
SpecificInterface that the impl is targeting in the `Impl` struct. When
we form the specific for the impl itself, we resolve the declaration
block and form new constant values for all instructions in there, but
that does not cover the SpecificInterface that we're storing in the
`Impl` struct. So we add a new instruction to the impl's eval block,
which will be symbolic when the impl is generic and the target interface
depends on a generic parameter. And we store the `InstId` in the `Impl`
struct. This allows us to gets its constant value later with the impl's
specific applied. From that constant value we can then pull out the
SpecificInterface that the impl is targeting.
Point symbolic witnesses into `.Self` written inside an impl decl at the
impl that is being declared. This is tricky because the impl does not
yet exist. So we use a new instruction `ImplSelfWitness` which _will_ be
replaced by the `ImplWitness` once it becomes available. The
`ImplSelfWitness` acts like a symbolic witness, except it does not
perform lookup, since we know which impl we will get a witness from.
This prevents us from finding other impls when performing lookups into
`.Self` in an impl decl, which produces incorrect/incoherent results.
Implement the toolchain side of proposal #7254, removing the `:!`
binding
syntax for generic and template parameters in favor of the keywords
`generic`,
`template`, and `runtime` plus contextual defaults for phase.
For valid programs this is semantics-preserving: each binding resolves
to the
same phase, and produces the same SemIR, as it did under `:!`/`:`. The
parser
derives a binding's phase from its syntactic context plus any explicit
phase
keyword; new diagnostics and error recovery for misused keywords are
described
below.
Implementation details for each component:
- Lexer: remove the `:!` (`ColonExclaim`) token, move its virtual
parse-node
budget onto `:`, and add the `generic` and `runtime` keywords.
- Parser: thread a `BindingContext` (`ExplicitParam`, `DeducedParam`, or
`CompileTimeEntityParam`) from declaration introducers down through
parameter
lists to each binding pattern, using a one-token lookahead to
distinguish a
name-qualifier parameter list from a declaration's own final list.
Parameters
of a compile-time entity (`class`, `interface`, `constraint`, `choice`,
`alias`, `export`, `namespace`) and deduced `[]` parameters default to
checked
generic; explicit function parameters and local bindings default to
runtime.
`HandleBindingPattern` resolves the phase from that context plus the
keyword: a
`generic` keyword needs no node of its own (the phase is carried by the
binding's node kind), while a `runtime` keyword is preserved as a
`RuntimeBindingName` node so `check` can name it in a diagnostic. A
phase
keyword that is merely redundant with the contextual default is
diagnosed
here, without invalidating the parse tree.
- Check: a phase keyword that is invalid for its context (for example
`runtime`
on a checked-generic parameter) is diagnosed here, and recovers by
building an
error binding that still introduces the name so that later uses of it do
not
produce cascading errors.
The removed `:!` syntax is now rejected as an ordinary parse error.
The `form`/`:?`/`->?` ("extended types") portion of proposal #7254 is
left for a
separate change.
Assisted-by: Claude Code
When formatting an impl definition, include in the textual output if the
impl is final. Since we (mostly) write them as `final impl` in the code,
use the same notation in the textual semir.
In #7436 we stopped substituting `.Self` when collecting witnesses out
of a facet type. While this was correct, it did not capture all the
cases that need to avoid substituting `.Self`. And it poisoned the
`IdentifiedFacetType` cache by not replacing `.Self` but storing the
result in the cache. This led to incoherent behaviour, where the result
of an impl lookup would change depending on which ones had been done
previously.
Now we use a flag to track for each `.Self` if we're currently
type-checking inside the scope where it was introduced in a facet type.
While inside that scope, identify should not replace the `.Self`. Any
use of it should remain as-is since we don't yet know what value will
replace it. We call this state "frozen" since it should not be modified
by identify. This requires a substitution step when we leave the scope
that introduced the `.Self`, to remove the flag. The flag is set in the
`EntityName` of the `SymbolicBinding`, and is part of the canonical
value, since `.Self` can become part of types, which are constants, and
the flag needs to follow it for correct behaviour.
We also have to ensure the flag is the same when doing comparison with
constants from inside a facet type and constants from outside. For
instance in `(Z where .Z1 = ()) where .Z2 = .Z1`, when we arrive at the
second `.Z1` its `.Self` will be frozen, while the `.Z1 = ()` contains a
non-frozen `.Self`. So we add the frozen flag to the first when storing
it in `where_stack` in order to compare the constant values of the two
`.Z1`.
The `WhereExpr` requirement inst kinds now have an `InstConstantKind` of
`AlwaysUnique` instead of `Never`. This allows us to add them to the
usual InstBlocks, and in an `eval fn` body they have a constant value,
so eval does not fail when trying to call that function. We have to be
careful to not consider `AlwaysUnique` as being actually concrete
though, since their constant value erases `.Self`-dependence. This
allows us to stop special casing them when thawing the requirements
block in a `WhereExpr`, and we can just thaw each `InstId` in the block
in a straightforward manner.
We add the new flag to the instruction's fingerprint and name in
formatted semir.
This adds SemIR structs and implements building `observe` lists, as well
as naming, formatting, and importing `observe` declarations.
---------
Co-authored-by: Dana Jansens <danakj@orodu.net>
In some cases the pattern block can depend on the initializer, so it
must be sequenced after it. See #7469 for a more detailed explanation of
why this is necessary.
The bulk of this change is changing most pattern insts to be `Always`
rather than `AlwaysUnique` constants, so that they can be wrapped in
`SpecificConstant`s to perform substitution. That then lets thunking
rely much more on `SpecificConstant` wrappers instead of deep-copying
the inst tree with modified types.
This approach to thunking should scale better, particularly as things
like form generics make function signatures more complex, because we can
leverage the existing support for constant evaluation and substitution.
Unfortunately, applying this approach to binding patterns will require
more work; see the TODO near the top of `thunk.cpp` for details.
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Implements proposal #7016: `self` moves from the deduced implicit list
(`fn F[self: Self]()`) to the front of the explicit list. Its type may
be written explicitly (`fn F(self: Self)`) or omitted, in which case it
defaults to `Self` (`fn F(self)`, `fn F(ref self)`); `self` in the
implicit list is rejected.
Throughout checking, `self` is modeled as the first explicit parameter.
Because a method is just a function whose first parameter is `self`, it
can also be called as an ordinary function with the receiver passed
explicitly (`Type.M(obj, ...)`), not only as `obj.M(...)`. A new
`SemIR::CallArgParamPatterns` helper chooses the parameters matched
against the explicit arguments, excluding a leading `self` only when it
is supplied as a method-call receiver; arity checking, conversion, and
generic deduction use it. The resulting SemIR and lowering are
unchanged: `self` is still `call_param0`, and witnesses, thunks, and
vtables are unaffected.
An omitted `self` type is parsed as a `SelfBindingPattern` node with no
type expression; checking synthesizes the `Self` type so it behaves
exactly like `self: Self`. However, the exact spelling used must match
between a forward declaration and a definition, following #3763's rules
around declaration matching.
Generated functions, thunks, and C++ interop import/export build `self`
as the first explicit parameter, and the `self`-type override (e.g.
Derived->Base for a virtual override) applies to the explicit `self`.
Placement is validated by new diagnostics: `SelfInImplicitParamList`,
`SelfNotFirstParam`, and `SelfOutsideParamList`. The benchmark source
generator and the documentation adopt the `(self)` shorthand; the
prelude, the examples, and the test data are migrated in the following
commits.
Assisted-by: Claude Code with Claude Opus 4.7
---------
Co-authored-by: josh11b <15258583+josh11b@users.noreply.github.com>
Fix import logic to make all imported packages be children of the
`NameScopeId::Package` scope. Previously, indirectly-imported packages
would end up as children of their importing package's scope, which
resulted in them not being treated as packages at all, and in particular
not being fingerprinted as packages.
Fixing that caused a failure in the fingerprinting logic as we started
to encounter packages with no correspoding import scopes. Instead of
looking for import scopes, use a simpler mechanism to map packages to
their package names, and clean up.
Unfortunately the latter change churns all the fingerprints again :(
Hopefully this is the last time for a while.
When we import from another library in the same package, its entities
end up with our library as their parent scope, resulting in cross-file
fingerprint mismatches. Instead, only include the library ID when
fingerprinting either a package-private entity or an `ImportIRId` that
refers to a particular `SemIR::File`.
Include the library name in the fingerprint of an entity declared
`private` at namespace scope. Include the entity's fingerprint in the
mangling of a library-private entity.
This fixes miscompiles if two libraries in the same package declare
`private` entites with the same name. We can't fix this with internal
linkage because library-private entities can be reachable through
generics defined in the API file of the library.
Assisted-by: Gemini via Antigravity
Fixes link failures when referencing a symbol involving a fingerprint
from a different package.
Previously we included the `Namespace`'s `import_id` as part of its
fingerprint, which caused local and imported namespaces to get different
fingerprints. We now store the `import_id` on the `NameScope` instead of
on the `Namespace` inst to avoid this problem.
Also, when we reach a package-level `NameScopeId`, consistently
fingerprint it as a (package name, library name) pair. Previously the
fingerprinting depended on whether it was imported or not, as an
imported `NameScopeId` had a parent scope (the current package). We need
to include the library name here so that private entities with the same
name in different libraries have different fingerprints.
When a class extends an interface, referring to a member name of the
interface as an unqualified name should refer to the class's
corresponding associated entity value, not to the associated entity
itself. Similarly, in an `impl`, unqualified names of associated
entities should refer to the `impl`'s corresponding value for that
entity.
To support this, we treat `impl`s as `extend`ing their implemented facet
type, and we make lookups into an extended facet type use the `Self`
type of the extending `impl` or `class` if lookup finds an associated
entity. We already did the latter if the extending entity was an
interface; this extends the existing support for these other cases.
A nested designator like `.(X.X1).(Y.Y1)` results in nested
ImplWitnessAccess instructions, which can produce cycles in the
toolchain easily when replacing `.Self`.
First, when constructing a facet type like `V:! Z where .Z1 impls (Y
where .Y1 = U)` we substitute replace `.Self` in the nested facet type,
and in this case we replace `.Self` with `.Z1` which contains a `.Self`
of its own. This was coming from us being lazy about replacing `.Self`
in an `impl as` declaration, such as `impl C as Z where .Z1 = .Self`.
The self type is known there, so we can more eagerly replace `.Self` as
we do in a `require impls` declaration. Then the replacement for `.Self`
never comes with a `.Self` that needs to also be replaced. Any resulting
`.Self` would always be the top-level one.
Second, when evaluating ImplWitnessAccess, we were replacing .Self in
the LHS of rewrite constraints, but the `.Self` may itself have a type
that contains rewrite constraints. If one of those rewrite constraints
has nested ImplWitnessAccess instructions, we evaluate the new
ImplWitnessAccess, which again finds rewrite constraints to replace
`.Self` in, and we repeat forever. For this one we just stop replacing
.Self in the LHS of rewrite constraints. Since they are always against
.Self, we can always look in the access facet's type for a value.
While fixing ImplWitness access, also correct the lookup to search
through the types of nested ImplWitnessAccess instructions to find a
rewrite value, since it may find it at any level up to the eventual
`.Self`.
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Implement the alias rules from proposal #5389, wherein an alias is
permitted so long as the target has a constant value. While that
proposal is not yet accepted, this seems like a reasonable basis for
further iteration, and will be useful for the examples we're currently
pursuing.
This lets us stop eliding it in textual semir tests with dump ranges.
Previously it would always get elided, even though it was part of the
range being dumped, and was referred to by other instructions in the
dump range.
Since each `.Self` is unique (can change its type if not its value) in a
facet type, having each one distinct by location also aids
understanding.
A `where` expression nested inside a `T impls X` constraint makes
`.Self` ambiguous on the right-hand side of the `where` if `T` is
anything other than `.Self`. After the `where`, the value of a `.Self`
could be `T` or could be the value of `.Self` before the `impls`
constraint: the so-called top-level value of `.Self`.
Implicit use of `.Self` in designators is always allowed, and they are
bound (and replaced by a reference) to the inner-most possible value of
`.Self`. On the right-hand side of the nested `where` above, they have
the value `T as X`.
`.Self impls ...` is also always allowed, since it acts more as a
keyword here, and it always refers to the inner-most possible value of
`.Self`.
Any other explicit use of `.Self` is diagnosed when ambiguous, in any
kind of constraint. This is done in the handling of `WhereExpr` since it
has enough context to allow `.Self impls` (which is an explicit use)
while disallowing other explicit uses. And because it has non-canonical
instructions to work with, so it is able to diagnose errors with precise
locations.
Since `.Self` is no longer going to be marked with depth modifiers, the
eval of `WhereExpr` does not need an input facet value instruction
representing `.Self` to compare with, as they are now going to all be
equivalent. So revert it back to just looking for the `PeriodSelf` name
id, through a shared helper being introduced as `IsPeriodSelf`. And drop
the period self InstId from the `WhereExpr` instruction. This causes
most of the formatted SemIR changes.
Move helpers for working with and replacing `.Self` to their own file,
out of the `facet_type.h` header/cpp files. These are working with
`.Self` facet values more than facet types, though `.Self` is a name
that only exists inside the scope of a facet type.
Dana Jansens
Chandler Carruth
Christopher Di Bella
Richard Smith
Geoff Romer
Özgür T. Önsoy
