This action was created to wrap any `MetaInstId` operand of an action
instruction. This served two purposes:
1) It had a special hook in `OperandIsDependent` to allow it to be
performed while it had a dependent operand (the reference to the
instruction in the generic).
2) It created a `specific_inst` so that the downstream action saw an
instruction in the specific instead of one in the generic.
These are both replaced: the special case in `OperandIsDependent` for
`RefineInstAction` is replaced by a special case for `MetaInstId`s in
general, and the `SpecificInst` is now created as part of performing the
downstream action, rather than as a separate step carried out
beforehand.
This simplifies the produced SemIR and reduces the number of splices
significantly. It also prepares us to handle actions like
initialization, where we don't actually want to create `SpecificInst`s
immediately in the location where the action is performed, because they
actually belong somewhere else in the IR.
Assisted-by: Claude Opus 5 and Gemini 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`.
Add basic support for lowering templates: we can now lower `SpliceInst`
in the case where the generic and specific are from the same file (and
we don't support importing templates from other files yet in general).
In order for this to work, lowering needs to be able to query the
expression category, and to handle instructions that appear to be
(template) constants in the generic but turn out to be non-constant in
the specific, so support for that is added.
Switch `type_of_inst` from being added as an action inst to being added
as a normal inst, since it's not an action and the old approach led to a
crash in lowering.
Replace `refine_type_action` with `refine_inst_action`, and generate a
`specific_inst` instead of an `as_compatible` to represent the specific
version of an instruction that's used as an input to a template action.
This gives us a place to handle other properties of the instruction that
might vary from generic to specific beyond its type, such as its
constant value and its expression category.
For now, we provide a non-template-dependent constant value to the
`specific_inst` in addition to the non-template-dependent type we have
traditionally provided. This doesn't seem to matter for any current
actions, but sets us up to better handle future actions. The
`specific_inst` representation also allows downstream consumers of the
instruction to track which specific they should be requesting
information from. Providing a correct expression category for
`specific_inst` will be handled in a future PR.
`GetCallee` is sometimes called on a spliced instruction, and is
checking its exact inst operand to see if it's a `BoundMethod`. This
fails if the `BoundMethod` is wrapped in another instruction, such as a
splice. Normally our approach for such a situation would be to
constant-evaluate the operand, but that doesn't work here because the
`BoundMethod` will be non-constant if its bound `self` is. So instead we
now step through splice instructions manually when looking for the
`BoundMethod`.
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`.
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.
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
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`.
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.
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 shifts logic a little so that empty top-level scopes are printed
less often. This affects imports mainly for now, but should be expected
to affect the soon-to-be-added generated scope more significantly.
Assisted-by: Google Antigravity with Gemini 3 Flash
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
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.
Pursuant to recent decisions on #6124, switch `Destroy` to use a
`CustomWitness` for its implementation. Right now this is manufacturing
no-op implementation functions on each lookup, which obviously isn't
ideal but is intended as a first pass. I'm mostly trying to find the
right balance between updating the approach to reflect new decisions,
while still breaking apart work in a way.
The `CoreInterface` logic is intended to build on `CoreIdentifier`
support. We have a number of additional interfaces that require
specialized logic, and that'll extend pretty far with C++ interop, so it
seemed easiest to have a generic function for it. That's what's
replacing the logic inside C++ interop that was doing string comparisons
(which could have already been moved to `CoreIdentifier`, I just missed
it in my first pass).
This adds `CustomWitness` support because the `Destroy` witnesses can be
imported cross-file. `CustomWitness` was previously only used for C++
types, which don't yet support import, which is why that wasn't
previously an issue. The addition of `query_specific_interface_id` is
similarly needed in order to get correct sorting of witness blocks when
imported.
This PR also removes builtin constraint logic (note this is in a
separate commit to help review; it's not a separate PR because it's
difficult to split apart without tests breaking). This had been made
generic with the expectation that destroy, copy, move, and conversions
would all need related support. Under the new decision, we are not going
to do blanket impls and will instead just manufacture a `CustomWitness`
for everything.
A lot of SemIR fingerprints change, but that's probably because the
addition of `Destroy` on core classes is yielding structural changes.
Instead of naming the root namespace `package` (because it's accessed by
the `package` keyword), change it to use the current package name. Note,
buried in the checksum changes,
`toolchain/check/testdata/package_expr/fail_not_found.carbon`:
```
- // CHECK:STDERR: fail_not_found.carbon:[[@LINE+4]]:16: error: member name `x` not found in `package` [MemberNameNotFoundInInstScope]
+ // CHECK:STDERR: fail_not_found.carbon:[[@LINE+4]]:16: error: member name `x` not found in `Main` [MemberNameNotFoundInInstScope]
```
for:
```
// CHECK:STDERR: var y: i32 = package.x;
// CHECK:STDERR: ^~~~~~~~~
```
I'll leave it to you if you prefer this; the alternative I see is to
just rename `IsCorePackage` to `IsImportedCorePackage`, and/or change it
to a helper that takes a `Context` and does the right thing with
`parse_tree` (which, I need for `Destroy`-related reasons and was my
default approach).
This is a prerequisite for support for interop with C++ template names.
No behavior change here, except that it sadly changes the fingerprinting
for a lot of tests.
Right now, the impl lookup can both fail to resolve the specific
definition because it's symbolic, and return a "final" constant because
it's a `final impl`. This is adding an instruction to help ensure the
specific is resolved.
The constant evaluation is fully recursive, but I'm not adding a TODO
since that's a known issue with impl lookup in general.
I was trying to figure out the right way to get specifics to be added to
the work.
Technically, we could keep the pending_specific list; this is taking a
different approach of inserting inside the work stack, which will do
extra work moving entries, although typically that should be expected to
be small. One challenge of `pending_specifics` is that if we would need
to shift them to work after both `Done` (for immediate processing) and
`Retry` (for processing after the current instruction is later revisited
and done). That feels kind of awkward as additional tracking to do.
Also, the common case is probably that there's either 0 or 1 specifics
being added, so an additional vector may be significant overhead. That's
why I leaned more in this direction of just inserting them in the vector
of work.
Give TupleLiteral and StructLiteral a constant value, if their contents
have constant values. Their constant values are TupleValue and
StructValue respectively. This supports their ability to convert to a
constant type (or facet type).
This way when deduce finds a TupleLiteral as the argument to a
_symbolic_ facet type, it can also find a constant value to use for that
argument. This allows deduction to move onto step two, where it can
substitute into the symbolic parameter from previous deduced arguments,
and then perform the conversion from the TupleValue to the desired facet
type.
Allow `PerformBuiltinConversion()` to convert from a canonical
TupleValue or StructValue to `type` instead of only from literals. Then,
also support conversion from a symbolic binding of type TupleType or
StructType to `type`.
This resolves a TODO in `expr_info.cpp` by using the inst kind rather
than the bound value to track the binding's category.
Since we're churning all the `bind_name` insts in testdata anyway, I'm
also taking this opportunity to align the inst naming with the design's
terminology, by calling these insts "bindings" (this aspect of the PR is
dependent on #6231 resolving an ambiguity in that terminology). For
consistency we'll need to rename several other insts as well (see the
TODO on `RefBinding`); I'm deferring that to a separate PR to minimize
the review load, but I think those name changes are in-scope for this
review.
When deducing arguments for generic parameters of an `impl`, the
deduction calls `Convert` on the input arguments. Often, the input
argument is a facet, and needs to be converted to a type via
FacetAccessType in order to produce a different facet. These
instructions end up being added to the semir, but only their constant
values are needed for the resulting specific returned from Deduce.
In the best case, these extra instructions are just noise in the semir,
or they just cause instruction names to get differentiated with larger
suffixes.
In the worst case, these extra instructions contain references to
instructions from a generic context, and leak them out of that generic
context and into another. In particular, when importing a
LookupImplWitness instruction, the re-evaluation of it can do deduce
(when the lookup is against a generic `impl`). The instructions created
in Deduce are not part of the import, and end up referring to imported
instructions from the local context, which leads to confusion in the
toolchain, and can crash.
The `import_self_specific.carbon` test demonstrates this. It causes the
`I.F` function to be imported from the `I` interface when building the
witness table for the `impl`. Doing so imports the specific of `C` which
includes a LookupImplWitness for `Self.Accoc` in `I`. The `Self` is a
BindSymbolicName with generic binding index 0, in `I`. When Convert
creates instructions in the generic `impl forall D`, however, they end
up referencing and including this BindSymbolicName into its eval block.
But the generic binding 0 in the `impl` is a very different thing (a
value of type `E`). This confusion leads to crashes.
The SymbolicBindingType refers to the type value that will be
substituted in for the BindSymbolicName, but holds onto the EntityNameId
from the BindSymbolicName instead of (or in addition to, for now) the
instruction.
The EntityNameId will be used to look in the ScopeStack to find the
witnesses either from the BindSymbolicName instruction, or other
instructions that specify `impls` constraints against the EntityName.
This will allow us to have the `T` in `I(T)` resolve to a `.Self`
reference in the type so that we get type equality with the binding's
type: `T:! I(.Self)`.
This also does a little restructuring in the same direction, following
#6124.
Leads want `Destroy` to work similarly now for all types. As a
consequence, there doesn't seem to be as much benefit to splitting off
aggregate destruction. In this PR, the `type.destroy` function can now
be expected to destroy anything that's destructible; that means it'll be
usable for the `final fn` once that support is available.
Similarly, this gets rid of the impls other than the single blanket
impl, now using `type.can_destroy`. Since they all need to use the same
function, there's no benefit to splitting approaches. Also, now it can
just be a `final impl` since there should be no need for people to
create specializations -- if this blanket impl applies, it means the
`final fn` is the same.
This also slips in `partial` support since there's no reason to have it
diverge anymore. Also `abstract`, which I'm not sure is broadly testable
since most cases it'd come up, the `abstract` keyword is explicitly
detected/rejected.
Note though that this doesn't make any really big changes. It's just
realigning on the leads decision. I'm going this way to try to reduce
name-related churn for other changes.
Locations are similarly fragile, because adding a comment changes them.
This has made me pause when making prelude changes in #6144, so dropping
them for those cases.
Instruction ids aren't actually that interesting outside debugging, and
can be churny when doing other structural changes. I've seen this in
particular when doing singleton changes, which bump every instruction
id.
Note there are still other ways fragility from locations can crop up.
This shouldn't be considered a complete fix, but hopefully a small
improvement.
The main direction of this change is the edits to `destroy.carbon`
(matching in both prelude and min_prelude).
Previously there was a no-op blanket impl for `Destroy`, which hid all
missing implementations of `Destroy`. This does a few things:
- Sets up builtin aggregate destruction for struct and tuple types as
before, but also adds C++ class types and array types to the same
handling. (all as a TODO for actual implementation)
- Also maybe-unformed destruction, for now at least. (there's a chance I
may try a different approach on this, but the impl lookup wasn't working
as I'd hope in order to write it in code)
- Adds handlers for simple things that are easy to do in code: `type`,
`bool`, pointers. (because these are no-op destruction)
- Redirect `const T` destruction to `T` destruction.
This leaves as future issues:
- `partial T` destruction. (this can't be done similar to `const`
because it only works for non-`final` class types; I think `class`
definitions should just generate what's needed)
- Destruction of other prelude-provided types. (will probably come up as
we implement class destruction, that the adapted builtin type doesn't
implement `Destroy` -- but may end up special-casing that in a way that
moots it)
This moves the `&` operator from `facet_types.carbon` to
`convert.carbon` because more things need to handle type and now that
we're getting separate copy and destroy interfaces. It should be
low-cost (an interface and builtin) so hopefully this is the right
balance for complexity and re-use.
A few tests are also edited in order to focus them more on what they
intend to test, and avoid a `Destroy` dependency.
This is in support of a goal of changing the blanket `destroy` impl to
use (roughly):
```
private fn CanAggregateDestroy() -> type = "type.can_aggregate_destroy";
// Handles aggregate type destruction.
impl forall [AggregateDestroyT:! CanAggregateDestroy()] AggregateDestroyT as Destroy {
fn Op[addr self: Self*]() = "type.aggregate_destroy";
}
```
That isn't done here because there's still other issues that migrating
raises. What this *does* do is add the builtin functions, and in
particular, support to `FacetTypeInfo` to make `CanAggregateDestroy`
work.
The "special requirement" approach in `FacetTypeInfo` allows us to
support restricting a blanket impl under the current approach of impls.
Maybe we'll find a cleaner approach that can work in the future, but
this fits into the current model by propagating similar to other
requirements. I'm using an enum mask because we have a number of similar
things to add (e.g. copy, move) but I'm not sure we need a full vector.
A few alternatives considered were:
- Supporting syntax more like `where .Self impls
TypeCanAggregateDestroy(.Self, SupportedInterface,
UnsupportedInterface)`. I think it'd be a little cleaner, but requires
better compile-time evaluation in order to assess the type of the call.
Right now it's expected to be a `FacetType` too early to make this work,
and I was concerned about pouring too much more time down this route.
- Providing an actual interface, in particular doing name lookup back
into `Core.` for an interface. This would've added name lookup overhead,
and the question of whether an `impl` exists.
- Generating an interface. This avoids the name lookup, but would still
raise the question of whether an `impl` should also be generated. Work
I've previously done generating interfaces for class destruction also
feels complex to both write and understand (an unfortunate issue).
- Still modeling as an `ImplsConstraint`, for example by defining a
special `InterfaceId::CanAggregateDestroy = -2` similar to what we do on
other ids. I was hesitant because of how this expands the number of
modes of `InterfaceId`, and things for consuming code to watch out for,
for what feels like a relatively niche set of use-cases that are only
interface-like.
---------
Co-authored-by: Dana Jansens <danakj@orodu.net>
Instead of hardcoding which types are copyable, add a `Core.Copy`
interface to perform copying. Move almost all the current copy support
to that interface. Some remaining pieces are still using builtin logic
after this PR:
* For tuples and structs, builtin logic is used to perform elementwise
copies. This also supports copying *adapters of* tuples and structs,
which seems like it may not be desirable, especially for non-extending
adapters. A `Copy` impl is provided for tuples of at most 2 elements, so
that `Core.Copy` constraints are satisfied, but we can't implement this
generally until we have variadics support, and don't yet have a
mechanism to generalize this to structs.
* For `enum` types imported from C++, builtin logic is used to perform a
copy. This is temporary until we have a mechanism to identify these
types from an impl in the prelude.
One lowering test in `toolchain/lower/testdata/class/generic.carbon` is
disabled for now, as it causes a crash in the lowering code due to an
ABI mismatch between the call signature in the lowered declaration of a
specific function and the call that is generated in the specific callee.
Fixing this is a little involved, and will be done in a separate PR.
---------
Co-authored-by: Geoff Romer <gromer@google.com>
This avoids impl lookups involving, say, `Core.Int` pulling in all ~65
impls in "prelude/types/int", which resulted in a lot of unnecessary
importing work, followed by a lot of unnecessary inst namer and inst
formatter work.
Before:
```
Ran 1335 tests in 6186 ms wall time, 146818 ms across threads
Slowest tests:
- toolchain/check/testdata/interop/cpp/function/arithmetic_types_bridged.carbon: 5611 ms, 5532 ms in Run
- toolchain/check/testdata/interop/cpp/function/operators.carbon: 2034 ms, 1981 ms in Run
- toolchain/check/testdata/primitives/import_symbolic.carbon: 1796 ms, 1786 ms in Run
- toolchain/lower/testdata/operators/arithmetic.carbon: 1729 ms, 1728 ms in Run
- toolchain/lower/testdata/function/generic/call_recursive_sccs_deep.carbon: 1700 ms, 1697 ms in Run
[==========] 1335 tests from 1 test suite ran. (682 ms total)
```
After:
```
Ran 1335 tests in 2419 ms wall time, 109587 ms across threads
Slowest tests:
- toolchain/check/testdata/interop/cpp/function/arithmetic_types_bridged.carbon: 1748 ms, 1665 ms in Run
- toolchain/check/testdata/interop/cpp/function/operators.carbon: 1106 ms, 1057 ms in Run
- toolchain/lower/testdata/function/generic/call_recursive_diamond.carbon: 1044 ms, 1041 ms in Run
- toolchain/lower/testdata/function/generic/call_recursive_sccs_deep.carbon: 1015 ms, 1012 ms in Run
- toolchain/lower/testdata/operators/arithmetic.carbon: 998 ms, 997 ms in Run
[==========] 1335 tests from 1 test suite ran. (652 ms total)
```
That's still slower than it should be, but a large improvement
nonetheless.
Fixes#6029
This uses each vector's size as a barrier between lists, to eliminate
the possibility of incidental collisions between entries of different
lists. This is the same as is done inside `AddBlock`.
Change impls from `<interface>.impl` to `<self>.as.<interface>.impl`,
and *member* functions to `<parent scope>.<fn>` (non-member functions
exclude their parent scope). Stop special-casing builtin functions,
given the new naming scheme.
The purpose of this is to make it clearer when a member function is
being accessed and, if so, which member function. In particular, we
often access interface `Op` functions. The builtin function
special-casing was intended to help with that, but we still have lots of
`Op` functions. This particular approach should make the interactions
clearer.
This changes up queueing of block IDs a little because, in particular,
we need to process bodies of entities only after constants finish
processing. But, it should also result in less memory usage during
processing because it means we have less on the insts stack at any given
time, since we track a block rather than all instructions contained by
the block.
Richard Smith
Dana Jansens
Nicholas Bishop
Christopher Di BellaandRichard Smith
Chandler Carruth
Özgür T. ÖnsoyandDana Jansens
Geoff Romer
Jon Ross-Perkins
