Two somewhat related fixes. The first is call-specific for now (because
it's the first action to take a `MetaInstId`), and the second is general
across all actions, but it seems like calls are the easiest place to hit
it.
1) Add support for refining inst blocks as action operands. Refine all
the insts in the block, using the appropriate InstId-derived type.
2) When an action operand is a `MetaInstId` referring to an unattached
constant, form a corresponding attached constant. This comes up when
forming (for example) an implicit `AddWith(%T)` call, where the `%T`
operand is an unattached constant.
This causes us to form correct specifics in more cases, where previously
we formed specifics that contained values that were still
template-dependent.
This unfortunately causes some existing template tests to produce more
errors, but those errors reflect cases where we were previously silently
doing the wrong thing.
Calls with template callee or args can now be deferred via an
InstAction. This allows code like this to check:
```carbon
import Cpp inline '''
template<typename T>
struct C {};
''';
fn F(generic T: type) {
let unused c: Cpp.C(T) = Cpp.C(T).C();
}
```
Treat `InstConstantKind::InstAction` the same as
`InstConstantKind::ConstInstAction`. Drop `ConstInstAction`, since the
two now behave the same.
Fix eval for specifics in a couple places to handle `InstId::None`.
When a template action is created, any (non-meta) instruction operand
will refer to instructions in the corresponding generic, or possibly to
a constant. This means that when the action is eventually executed when
forming a specific, it would see the generic value for that operand
rather than the intended specific value.
Fix this by refining `InstId` operands to refer to a corresponding value
in the specific, much like we would when rebuilding a constant in the
eval block.
Generalize ConvertToValue template action to handle other kinds of
conversion target that don't perform initialization. Initializing
conversions will need more work since they also need to use a splice to
form the storage block.
This allows various templated constructs to get further through
checking, but typically we hit another unsupported action such as a
conversion or call, so it's not enough to make much work.
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>
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.
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
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`.
As suggested in [1], replace `FieldInitializerMap` with a `FieldStore`.
The corresponding `FieldId` is now stored in `FieldDecl`. To make room
for the `FieldId`, the `ElementIndex` is now stored in the `Field`,
along with the initializer.
In import_ref.cpp, resolving `FieldDecl` initializers is now supported,
and in convert.cpp `LoadImportRef` is called to do so. The
`field_initializer_import.carbon` test now passes.
Printing a `FieldDecl` instruction now prints the initializer as well,
if present. See field_initializer.carbon for an example.
[1]:
https://github.com/carbon-language/carbon-lang/pull/7238#discussion_r3283217158
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.
Class vars are still restricted to simple `name: type` bindings, not
full patterns. This is now handled in the check phase instead of during
parsing.
This is in preparation for supporting `static var`.
See
[here](https://docs.google.com/document/d/1rWcueFwIfZox6GKVGxiUG4cBzjrZ6djXiIDGyJDtrE4/edit?tab=t.0)
for the design doc.
This also removes the default value of the `result_type_inst_id`
parameter of `HandleAction`, moves it before the action in the parameter
list, and documents it. This solves two problems:
- The default made it easy to forget, leading to unnecessary
`TypeOfInst` instructions.
- When it was present, putting it after the fairly "bulky" action
argument tended to make the callsite harder to read.
This is only fixing the decision about *whether* to produce a witness.
Implementation of the witness is still a TODO, though where a body is
generated, it should also precisely reflect where one _needs_ to be
generated.
Note the tests:
- toolchain/lower/testdata/function/generic/import_core_witness.carbon
- toolchain/lower/testdata/function/generic/import_unused_def.carbon
These tests can probably be produced _without_ Core.Destroy, but I found
the essence of them while trying to build //examples with Core.Destroy
and a simpler minimization wasn't striking me.
Assisted-by: Google Antigravity with Gemini
---------
Co-authored-by: jonmeow <jperkins@google.com>
EvalOrAddInst has to create a non-canonical instruction for evaluating a
few typed insts, such as LookupImplWitness which uses an InstId to
provide a location for diagnostics.
But the output of the function is a ConstantId. We do not have access to
the non-canonical InstId after the function returns. But if the constant
value was symbolic, it was being attached to the inst, and the inst
would be added to the eval block of the enclosing generic. This
needlessly added semir for a symbolic value.
The ConstantId returned by EvalOrAddInst can be used immediately, such
as to evaluate an ImplWitnessAccess. In that case, the final evaluated
result is all we need to keep in semir.
If the ConstantId needs to be replaced by specifics, it is only as part
of some other instruction, since ConstantIds themselves are not modified
by specifics, instructions are. In that case, the canonical instruction
in the constant value would have been added to some other (now symbolic)
instruction, which would be replaced by a specific.
This has no functional change, but it reduces runtime overhead and semir
output for LookupImplWitness and ImplWitnessAccess.
Performing a lookup against `Self` inside the definition of the named
constraint leads to cycles, as described in the document [Self
contradictions in Named
Constraints](https://docs.google.com/document/d/17rn2XmME8o2MM4OJqatSVuMa1iYZ1PAgcNrf0PXR9Q4/edit?tab=t.0).
To prevent those cycles, this change introduces a large refactoring of
impl lookup.
The impl lookup done inside eval is reduced to only performing
monomorphization. That is it:
- Only looks for an provides final witnesses.
- Is not allowed to identify the facet type of the query self.
- Returns either a final witness or None (or an error)
The paths for finding non-final witnesses are now done outside of eval,
directly in the initial `LookupImplWitness()` function. If no final
witness it found through eval, the resulting non-final
`LookupImplWitness` instruction witness is returned. It does not produce
cycles to identify the facet type of query self outside of eval, since
that does not result in repeating the identification when resolving
specifics of the named constraint or require decl.
Move the ArrayStack for Context::require_impls_stack into a new class
which tracks a NamedConstraintId (or InterfaceId) for each frame of
RequireImplsIds, so that in type completion we always can find the
correct frame for a given named constraint which is still being defined,
in order to find the RequireImplsIds in the in-progress definition.
This is a step toward removing the index from `InitForm`, so that equal
form values always have equal representations.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
This is iterating on how `Destroy.Op` generates, to start adding body
capabilities. This changes the way the signature is created, and adds a
`CoreWitness` function kind so that mangling can prevent name
collisions. The result is that what _was_ `DestroyOp` is now
`Core.Destroy.Op` or, as can be seen in
toolchain/lower/testdata/interop/cpp/nullptr.carbon,
`_COp.<hash>:core.Destroy.Core` where `:core` is indicating that it's a
core witness (taking a note from `:thunk`).
Assisted-by: Google Antigravity with Gemini 3 Flash
This allows us to capture the location at which a type literal was used,
even in the cases where we don't otherwise need to create a new
instruction to represent the type such as for `char` or `str`.
The logic used to build the underlying type is now marked as desugaring.
For cases such as `iN`, this causes the call to `Core.Int` to no longer
be added as a dedicated IR instruction, and instead its constant value
is used directly as the value of the `type_literal`. This results in
this being on balance a reduction in the size of the IR.
This also fixes a crash in C++ interop when using a `char` literal as a
template argument. The crash was caused by the template argument not
having an associated location when mapping to a C++ location. See
changes to check/testdata/interop/cpp/template/type_param.carbon for an
example that used to crash before this change.
Update alias handling to allow an alias to point at any type literal,
reinstating support for aliases for type literals such as `bool` and
`i32` that had previously worked but stopped working when we
transitioned those types to being defined in the prelude. See changes to
toolchain/check/testdata/alias/builtins.carbon.
All the test changes other than the two mentioned above are mechanical
autoupdate changes switching to the new instruction.
Introduces `Context` and `SoftContext` messages, which can be introduced
through a `ContextBuilder`:
- The `Context` messages come before the diagnostic in the output.
- The first `Context` message steals the diagnostic level from the main
diagnostic, and turns the main diagnostic into a Note attached to the
context.
- A `SoftContext` message works similarly, but if it's preceeded by a
`Context` or `SoftContext` message, then it is dropped. This can be used
as a default/backup scope when nothing more interesting is provided up
the stack, such as in `TryEvalBlockForSpecific`.
The `ContextBuilder` is provided to a callback through
`Diagnostics::ContextScope`, an RAII type `AnnotationScope` but for
context messages.
This allows a high level operation to provide a context message like
"failed to identify facet type {0}" which will then be used as the error
if a diagnostic is produced during identification, with the latter
diagnostic attached as a note to explain why the contextual operation
failed.
In particular, this allows monomorphization errors (such as an array
bound being negative) to be attached to a higher lever operation instead
of being top-level diagnostics themselves, with the monomorphization
site being a note. This inverts the source code locations that appear in
the diagnostic, so that the top-level diagnostic points to the "user
code" which causes the monomorphization.
This is presented as an alternative strategy to #6753, which plumbed
diagnoser callbacks around to achieve the same goals.
We replace the diagnoser callbacks in type completion and operators with
ContextScope callbacks instead, which now provide better diagnostics for
monomorphization errors. Other callers to MakeSpecific do not yet have
ContextScopes introduced in order to turn monomorphization errors into
more interesting diagnostics.
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 is related to #6727, but is generally a necessary fix even without
that issue. I'm not adding a specific test of #6727 because it should
also be covered by the tests in #6726.
Assisted-by: Google Antigravity with Gemini 3 Flash
For example, see toolchain/check/testdata/class/fail_incomplete.carbon
for the diagnostic changes. `IncompleteTypeInFunctionReturnType` should
remain, while the redundant `IncompleteTypeInFunctionParam` is removed.
Note I'm deliberately trying to validate the return type after other
parameters, because I think that's the better user experience. This does
also incrementally change IR.
Add a new builtin function `cpp.std.initializer_list.make` that takes an
array and returns a `std::initializer_list`, initialized to refer to
that array. When C++ initialization wants to perform a
`std::initializer_list`-from-array construction, synthesize a
declaration of a matching builtin function and use that to perform the
initialization.
Ideally we would specify this conversion as an impl of `ImplicitAs` in
the prelude instead of hardcoding it in the interop layer, but
unfortunately that's not currently possible, for various reasons -- we
can't make the conversion form-generic, we can't deduce the array length
from the initializer, and we can't deduce against the arguments of
imported C++ class templates yet -- so for now synthesizing a builtin
function on demand is the best we can do.
Assisted-by: Gemini 3 Pro via Antigravity
The key changes are:
- Function output parameters are now prefixed with `out`, and more
consistently formatted as named parameters.
- Function and inst output arguments are now written as part of the inst
form, rather than as one of the inst arguments.
As a drive-by fix, this also changes `Temporary::storage_id` from
`DestInstId` to `InstId`, because it doesn't represent an output
parameter of the `Temporary` inst itself.
See the review of
[#6532](https://github.com/carbon-language/carbon-lang/pull/6532) and
[this Discord
discussion](https://discord.com/channels/655572317891461132/999638000126394370/1458268977020141589)
for additional background.
Also clarify and enforce that `ConversionTarget::init_id` is used only
as storage for in-place initialization, and correspondingly rename it to
`storage_id`.
Not all functions have a return slot, and once we have composite forms,
functions will be able to have any number of return slots. Obtaining a
unique return slot for a function only makes sense in `returned var`
handling.
Richard Smith
Nicholas Bishop
Geoff Romer
Christopher Di BellaandRichard Smith
Dana Jansens
Chandler Carruth
Özgür T. ÖnsoyandDana Jansens
Jon Ross-Perkinsandjonmeow
