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
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.
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`.
Also add a default for `EqWith.NotEqual`.
Switch advent examples to use these named constraints, and also go
through all the other TODOs in the advent examples and fix the ones that
are trivially fixable now.
Fix a bunch of cases where we use the same external name to mean
multiple different things in the same test. We've historically gotten
away with this, but under `--share-cpp-ast`, it becomes an error, at
least if the entity is either defined in, or used from, C++ code.
Assisted-by: Gemini via Antigravity (original change) and Claude Code
(suggested edits in review)
---------
Co-authored-by: Chandler Carruth <chandlerc@gmail.com>
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>
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.
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.
Instead of tracking the cleanup scope depth on entry to each scope,
track an "ambient" cleanup scope depth that's *after* the destructors of
local variables in that scope. This gets increased to include the
destructors of local variables when we create a name-binding
declaration. Then, when we reach a point where temporaries should be
destroyed, run cleanups that are after the ambient cleanup scope depth
on the stack. This happens:
* At the `;` of a statement expression.
* At the `)` of an `if` or `while` statement.
* After performing the implied `HasValue()` call in a `for` statement.
Per informal agreement with leads, this means we lifetime-extend all
temporaries created in the initializer of a name-binding declaration to
the full scope of that declaration, but that temporaries created in an
expression statement are destroyed at the `;`.
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.
If C++ overload resolution selects a builtin operator candidate for an
enum comparison or bitwise operator, provide support for that operator
by generating a corresponding Carbon builtin function. This is
structured to be easily extensible to other C++ builtin overload
candidates if we so choose, but for now the operators defined in the
prelude are doing what we want in most cases.
Bitwise operators on enums produce the same enum type as a result. This
intentionally deviates from C++, where they produce a promoted integral
type.
Assisted-by: Gemini via Antigravity
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>
Because we now support calling a function with a `self` parameter
directly, we can unconditionally call `operator$(lhs, rhs)` rather than
calling `lhs.operator$(rhs)` if the selected operator function happens
to be a member function.
This makes the logic a bit simpler and the SemIR a bit smaller.
We can't do the same for Carbon operators, unfortunately, as we use the
member access to trigger impl lookup.
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.
Implement support for floating-point <-> integer type conversions as
described in #820 and #845, extended to support `unsafe as` conversions
for the conversions that can't be expressed as either implicit
conversions or `as` conversions.
One tricky part here is conversions from floating-point literals to
integer types. Such literals may have both a very large mantissa and a
corresponding somewhat large negative exponent, and still produce a
result that is in the range of values that a small integer type can
represent. In order to support that while avoiding building very large
2^N or 10^N constants in general, we first compute a conservative
approximation of the number of bits necessary to represent the integer
result, with an early exit if the number is either definitely too large
or definitely zero. The remaining cases have a reasonable bound on the
size of integer necessary to compute the base^exponent multiplicand.
Assisted-by: Gemini via Antigravity
Since we can't currently iterate over ranges as references, the
`CppUnsafeDeref` interface can't require types that implement it to take
or return references.
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`.
Carbon-side thunks (for example the `Copy`/`Destroy` witness thunks
generated for imported C++ types) are mangled by Carbon, and their names
incorporate a fingerprint of the involved types. The instruction
fingerprinter identifies a class only by its name and parent scope,
which is sufficient for Carbon classes but not for imported C++ classes:
different specializations of one class template (and other cases such as
types in anonymous namespaces) share a Carbon name and parent scope. As
a result, the thunks for two distinct specializations could mangle to
the same name, producing a single LLVM function with two definitions and
failing `verifyModule` during lowering.
When fingerprinting a class imported from C++, also include the Clang
mangled name of its type.
Test: toolchain/lower/testdata/interop/cpp/thunks.carbon gains a split
with two specializations of one class template, each requiring a thunk;
their thunks now get distinct mangled names instead of colliding.
Assisted-by: Claude Code
---------
Co-authored-by: Christopher Di Bella <cjdb.ns@gmail.com>
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.
We were incorrectly computing the index of the Clang implicit conversion
corresponding to method arguments. This led to wrong code and a crash in
lowering due to a calling convention mismatch.
Fixes#7224.
Assisted-by: Gemini via Antigravity
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>
Previously, we picked a single Carbon parameter pattern for each C++
parameter pattern. This doesn't work well in cases where the Carbon
semantics and the C++ semantics are not perfectly aligned. In
particular, when a parameter is passed by value in C++, that might mean
either pass-by-move (which in Carbon would best be modeled by a `var`
pattern, as no other form of parameter would perform a move) or
pass-by-copy (which in Carbon would best be modeled by a value
parameter, as a `var` parameter would force an extra copy).
After this change, we compute a passing mode for each parameter based on
the implicit conversion sequence from the argument to the parameter as
determined by C++ overload resolution, and use that to determine the
Carbon pattern corresponding to each C++ parameter. This results in
potentially generating multiple different thunks for the same C++
function if it's called in different ways, but we already did that to
handle default arguments and list-initialization. The passing modes are
included in the thunk mangling.
Add a new value store for clang decl signatures, which capture the
information about parameter passing mode as well as the other existing
information about different ways that a C++ function might be imported
to Carbon.
Most of the rules for computing passing modes are the same as before:
const references use pass by value, non-const lvalue references use
pass-by-ref, non-const rvalue references use pass-by-var. But for C++
non-reference parameters, pick between pass-by-value and pass-by-var
based on whether the implicit conversion sequence was effectively
performing a copy. Prefer pass-by-value if either would work and they'd
do the same thing. We still use pass-by-value for const references, even
when the argument is an lvalue and we could pass a reference; we may
want to change this in future.
For virtual functions, we try to pick a worst-case passing mode, as we
can only pick a single signature for what goes in the vtable. Calls to
virtual functions will still use a thunk to C++, allowing variance in
the calling convention at call sites. We don't allow variance in the
overriders as we don't implement support for thunks for virtual
functions yet. We currently use pass-by-value for const reference
parameters here, but that should probably change at some point.
Assisted-by: Gemini via Antigravity
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.
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.
When importing a C++ function with an rvalue reference parameter, we
previously produced a Carbon value parameter. This would lead to the
toolchain believing it could pass the address of a non-expiring object
to the function, which would lead to a use-after-move.
Instead, we now map non-const rvalue reference parameters to Carbon
`var` parameters. This forces the object passed into C++ to be unique
and owned by the call. While that's not an exact match for C++ rvalue
reference parameters, given that it provides "always move" not
"conditionally move", it's the closest match we have at the moment.
Add import/ and export/ under function/. Move most top-level tests to a
new basics/ with subdirectories for `import` directives and `inline
Cpp`. Add subdirectory for primitive type handling. Move all `reverse/`
tests to somewhere else, typically under an `export/` directory.
I split two test files up: constexpr.carbon got split into var/ and
function/ pieces, and reverse/simple.carbon was inlined into
namespace/export.carbon. The rest are just simple renames.
Christopher Di Bella
Dana Jansens
Richard Smith
Geoff Romer
Özgür T. ÖnsoyandDana Jansens
