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)`.
Previously it performed two kinds of operations, with a boolean
parameter to control whether it would unwrap FacetValue or not. This
made the function hard to explain as "canonicalization".
Now the contract of GetCanonicalFacetOrTypeValue is as follows:
1. For a facet value expression, it returns the canonical value of the
facet value.
2. For a `<facet value> as type` it returns the canonical value of the
`<facet value>`.
3. For other type expressions, it returns the canonical value of the
type.
1 and 2 together collapse together two representations of a facet value
(as a FacetType or as a TypeType) into a single canonical value, which
is important for constant comparison of facet values where the `as type`
is not meant to change the result. This is the case in impl lookups and
`.Self` comparisons.
The step of unwrapping `FacetValue` is only useful in the constant
evaluation of `LookupImplWitness` and is used to collapse *symbolic*
queries on `FacetValue(T)` and on `T` down to a single canonical value,
since they produce the same result later when `T` is replaced with a
facet value or type that can provide a concrete witness. This is now
extensively documented in the constant evaluation of
`LookupImplWitness`.
This change came out of a request/discussion in #6115 (see comment
https://github.com/carbon-language/carbon-lang/pull/6115#discussion_r2383696576).
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.
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 type has the same object representation as `T`, but always uses a
pointer type as its value representation. No other semantics are
provided for it yet.
Generalize the f64 support to support other sizes. Also provide interop
support for `float`, `_Float16`, and `__float128`.
Also lay some groundwork for non-standard floating-point types, though
we don't have any syntax to name them yet.
Don't convert to f64 until we know that's the type that we actually
want. Also reimplement the conversion from RealId to FloatId to perform
an exact conversion with a real check for overflow, rather than
performing an approximate conversion via the host `double` type.
Unfortunately, LLVM doesn't expose its integer mantissa and exponent to
APFloat conversion, so we convert the RealId back to a string for now.
The LLVM conversion also detects overflow only if the literal would
round to having an out-of-range exponent, not if the literal is outside
the range of values of the type as the Carbon design expects. It's not
clear to me which rule we actually want here, so for simplicitly I'm
using the LLVM rule for now.
In preparation for adding other floating-point types beyond f64.
* Rename the type.
* Change lowering to lower FloatLiteralType values as the placeholder
`{}` value we use for literals instead of as an LLVM f64.
* Change eval to convert the type as part of a floating point
conversion, so that lowering can lower converted constants properly.
For now we still represent a value of FloatLiteralType as a
double-precision APFloat. (That will need to change so that we can
losslessly convert literals to f80 / f128 values, and so that we can
convert literals to f32 values without double-rounding.)
Add missing builtins for float compound assignment, for building a
FloatType, and for converting a float literal to FloatType. Switch
`Core.Float` to being a class and add impls for the various
floating-point operators.
---------
Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
Co-authored-by: Carbon Infra Bot <carbon-external-infra@google.com>
This makes all `.Self` references in a facet type canonically the same
(which will remain true iff they refer to the same `Self` type in the
future), removing the need to do more complex comparisons between them
using the EntityName, interface, and index. This allows the comparison
of types containing `.Self` references to be done correctly regardless
of where the `.Self` appears, as such type expressions will all be
canonically equal if they otherwise equal now, regardless of whether
they are written in the context where `.Self` could have seen different
`Self` facet types.
In order to retain access to constraints on a base `.Self` facet type,
in the case of applying `where` to an existing facet type, we:
- Give the base facet type as a `RequirementBaseFacetType` constraint so
that eval of `WhereExpr` can find and copy all the constraints off of
it.
- Introduce eager/early rewrite constraint resolution, which allows a
constraint to eagerly resolve access to earlier rewrite constraints
(`where .A = () and .B = .A` is eagerly transformed into `where .A = ()
and .B = ()`) before the full constraint resolution step. This allows
use of rewrite constraints in larger type expressions, such as `where .A
= () and .B = C(.A)` and `C` will know that the argument is `()`.
toolchain/check/testdata/builtins/char/basics.carbon and
toolchain/lower/testdata/builtins/char.carbon are probably the most
interesting tests here. The parse tests is required because this adds a
new node kind, and we need coverage of it; but the attached info is
minor. There's a fair amount of test churn here because I'm adding the
Core.Char and Core.CharLiteral types as new singletons.
My intent here is that `CharId` is always a unicode code point, even
when the type is a `Char` and thus must be a single UTF-8 code unit
(single byte). This mainly means the stored value of a `CharValue` can
be printed internally without knowing the type.
---------
Co-authored-by: josh11b <15258583+josh11b@users.noreply.github.com>
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
In preparation for `FloatValue` being used more generally, and not only
for literals.
---------
Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
A rewrite constraint like `.X = .Y.Z and .Y = .Self and .Z = ()` has a
nested `ImplWitnessAccess` `.Y.Z` (technically `(.Self.Y).Z`). The inner
access `.Self.Y` needs to be resolved (in this case to `.Self`) before
the outer `???.Z` can be resolved as `.Self.Z` which is `()`.
Make Subst perform "recursion" on the RHS instructions as they are
replaced, effectively doing a depth-first traversal through the rewrite
constraints doing replacements. This allows us to fully compute
individual associated constants in the minimal amount of work, and cache
the results so they can be reused cheaply in cases where the rewrite
constraints generate an exponential number of references to associated
constants.
Fixes https://github.com/carbon-language/carbon-lang/issues/5672
`SpecificInterface` seems oddly placed. It appears to be in ids.h just
because it's used by typed_insts.h, but maybe that should be factored
differently? We typically aren't having typed_insts.h depend on non-ID
types. To that end, I'm splitting it out to its own file so that at
least I'm not adding a `ValueStore` dep inside ids.h
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Currently if the first operand contains an error, we will return error,
even though the second operands contains a runtime, and it has a
stronger priority (the phase always goes up if possible).
Import is only allowed on instructions with compile-time values, so we
crash if we ever try to import a runtime value. Importable instructions
must diagnose unexpected runtime values and produce errors in the semir
from which they would be imported so that runtime values are never
imported by another semir.
If we had an instruction where you had an error value from the first
operand, and runtime from the second, and we imported it:
- Before https://github.com/carbon-language/carbon-lang/pull/5728 we
would crash in import, but only because we treated errors as runtime
- After https://github.com/carbon-language/carbon-lang/pull/5728 we
would import ErrorInst because we propagate errors. This is desirable
for cases with compile-time values and errors present only.
- After this PR, we would crash again, cuz you're importing a runtime
thing.
This change means that instructions containing an
`InstConstantKind::Never` instruction like`ValueParam` will consistently
evaluate to a runtime value, even if there are errors present. This is
visible in the `BindName` instructions changing in the semir, where they
became constant `ErrorInst` values previously but no longer do.
This is reducing ValueStore inference of types from `using`, and removes
`using ValueType = ...` from affected id types.
I'm adding a number of `using FooStore = ValueStore<FooId, Foo>` because
I think it's a little repetitive otherwise; often 4 cases where I'm
doing this: getter, const getter, member, and getter on `Context`. Note
we also have a number of `-> decltype(auto)` that were added I think
mainly to avoid repeating the type, but I'm not sure whether there'll be
agreement on replacing those and so am not changing them here.
I'm placing these aliases with the value type in general, because I
think it's probably easier to view that way. An alternative would be to
put all the types on `File`, but:
- That would be inconsistent with things like `InstStore`, which are
very `ValueStore`-adjacent and put with their value type.
- `File` would have a _lot_ of using's, and the accessors are already
noisy -- I think it would just make the file harder to skim.
Note this is the heart of what I'd brought up [on
Discord](https://discord.com/channels/655572317891461132/655578254970716160/1388199282250613019).
This PR still leaves CanonicalValueStore and BlockValueStore as things
to also add parameters to, but I thought it best to try breaking the set
of changes apart by type. Both of those rely on ValueStore, so
ValueStore needs to change first.
As I'm looking at splitting value type setting out, this is to make it a
bit easier to see what's part of each type. Note, I expect
`ValueStoreTypes` to remain because of the `StringRef` logic it does --
I'm giving that its own file.
In a facet type constraint, you can write `where .Self impls T` for any
facet type `T`, or the constant `type`. It is possible to write `type`
in different ways though, with a `NameRef` instruction appearing on the
RHS instead of `TypeType`. In this case, the canonical constant value's
instruction will still be `TypeType`, so make eval look at the canonical
instruction to see this.
Add a test with an `alias Type = type` which hits this case.
After this change, we only will accept and find one of the following on
the RHS of `impls`:
- `TypeType`
- A facet type
- An error, if the source code had something else there, which will
already be diagnosed. Tested by `fail_right_of_impls_non_type.carbon`
and `fail_right_of_impls_non_facet_type.carbon`.
So we handle these three cases, and drop the implicit handling of other
things which will never appear there.
`AddImportedInstruction` was turning errors in an instruction into a
Runtime constant value instead of an Error, which led to crashes when
importing an instruction that had an error inside it somewhere.
Fixes#5726
The `IsPeriodSelf` function is problematic, as it's possible for a
FacetType to contain multiple `.Self` bindings which refer to different
selves, when one FacetType is nested within another: `I where .Self.J =
(K where .Self impls type)`.
The `WhereExpr` instruction contains the instruction of the `.Self` of
that `where` clause, which is what `IsPeriodSelf` is looking for, so we
can compare with its constant value instead.
When building a FacetType from an existing FacetType, don't diagnose
rewrite constraints that are compatible with the existing FacetType.
To do this, we consider two RHS as identical[1] if they have the same
constant value after substituting from available rewrite constraints in
the being-constructed FacetType, since the syntactic representation of
the RHS is lost during eval.
[1]
https://docs.google.com/document/d/1Yt-i5AmF76LSvD4TrWRIAE_92kii6j5yFiW-S7ahzlg/edit?tab=t.0#heading=h.qti4vn50zwy
Uses of `ConstantValueStore::GetConstantInstId` or
`ConstantValueStore::GetInstId` in eval indicate that the code expects a
constant value. Instead of just ending up with `None` in strange places,
diagnose this and convert to an `ErrorInst` when expectations are not
met.
We add `RequireConstantValue` to pair with `GetConstantValue`, and
rename `GetConstantValueIgnoringPeriodSelf` to
`RequireConstantValueIgnoringPeriodSelf` since the former would just be
unused.
Adds a test where a runtime value ends up in the RHS of a rewrite
constraint, where a constant value is expected. This issue was uncovered
by a fuzzer.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
Given a facet type: `(Z where .X = .Y) where .X =.Y`
The rewrite constraints in the inner facet type are each an
`ImplWitnessAccess` into a witness for the self of type `Z` (which is
the facet type before the `where`). The rewrite constraints in the outer
facet type are each an `ImplWitnessAccess` for the self of type `Z where
.X = .Y`, which is a different self facet type.
This means when deduping in canonicalization, the first `.X` and the
second `.X` are different instructions, and different constant values,
so they both remain in the rewrite constraints, incorrectly. Then if the
outer `.X` is allowed to evaluate to a value from its facet type, it
finds `.Y` resulting in `.Y = .Y` which is also incorrect.
Because of the failure to dedupe the first facet type, that is also
diagnosed as two different assignments to the same `.X`. To resolve
that, we introduce `CompareFacetTypeConstraintValues()` compare values
in facet type constraints, and treat accesses to the same associated
constant in the same facet value as `equivalent` even when through
different witnesses. This allows us to dedupe the two `.X = .Y` rules
into one in the combined facet type.
Given a different facet type: `(Z where .X = ()) where .X = {}`. Here we
want to diagnose that `.X` has been assigned two different values. To do
so, we need to see that the two `.X` values are the same, and we use
`CompareFacetTypeConstraintValues()` to do this comparison. Then we see
two rewrite rules for the same LHS, and we can diagnose that.
We enable evaluating `ImplWitnessAccess` on `.Self` to pull a value from
rewrite constraints in a facet type so that we can see that we are not
incorrect evaluating the LHS of rewrite constraints and producing
cycles. By doing so, also enable generic code to see and use concrete
values in associated constants in facet types.
The `BitAnd` operation combines two `FacetTypeInfo` structures by
concatenating their lists, but did not apply the current specific to the
instructions in the `FacetTypeInfo` as it forgot to go through
`GetContantFacetTypeInfo`.
`WhereExpr` handling duplicates a lot of the logic in
`GetConstantFacetTypeInfo` by calling `GetConstantValue` on things,
instead of calling `GetConstantFacetTypeInfo` on the `FacetTypeInfo` it
constructs. This meant it also needed to call `GetConstantFacetTypeInfo`
on the base facet type, and on any `impls`-requirement facet types
before merging their values together into a single `FacetTypeInfo`.
Instead, make `WhereExpr` more like `BitAnd`, and have it concatenate
things together as-is to construct a `FacetTypeInfo`. Then call
`GetConstantFacetTypeInfo` to canonicalize it and return a constant
value referring to it.
In `GetConstantFacetTypeInfo` we fix a crasher by propagating errors
inserted into the `FacetTypeInfo` out to the `Phase` so that the
resulting instruction depending on the `FacetTypeInfo` is not resolved
to a constant value with errors inside it. A test is added for this,
which was crashing on import of the `FacetType` with an error within
from the imported `impl` decl.
This refactoring gives us three benefits:
* There's now only a single place that does
`ResolveRewriteConstraintsAndCanonicalize`, which is inside
`GetConstantFacetTypeInfo`. This makes the inputs/behaviour of
`ResolveRewriteConstraintsAndCanonicalize` more consistent.
* There's now only a single place that updates the instructions in
`FacetTypeInfo` constraints with new constant values, so that changes
that rely on observing and interacting with that code only need to be
written in a single place. This will avoid duplicating logic in
https://github.com/carbon-language/carbon-lang/pull/5644.
* This will make it easier to move `WhereExpr` handling to a
`EvalConstantInst` function, as it no longer directly depends on
`GetConstantValue()` from `eval.cpp`.
This preserves the constant values of the arguments to the thunk, which
is important if the thunk requires conversion of an `IntLiteral` to some
other type. This should become unnecessary once we have form support,
but avoiding the indirection through a thunk function seems valuable
even once that support is in place.
To support this, track whether a function is a thunk on the Function
object, and if so, what the callee of the thunk is. This information is
also included in formatted SemIR when dumping the thunk.
Add a facet type rewrite constraint resolution step that is run every
time a facet type is constructed, in line with the design here:
https://docs.carbon-lang.dev/docs/design/generics/appendix-rewrite-constraints.html#rewrite-constraint-resolution
The resolution has multiple steps, and this PR implements the first of
them, finding and diagnosing any duplicate rewrites to the same
associated constant.
We already diagnosed this for impl construction, now we do so for all
facet types, which includes the one used for impl construction, so this
diagnostic is a superset of the previous.
Also fix substitution into constants to provide a source location. This
matters if the result of substitution ends up being part of a generic
eval block.
---------
Co-authored-by: Dana Jansens <danakj@orodu.net>
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
I was thinking about these after #5526, was wondering how others will
feel about this kind of approach:
- Adding a helper to `TypeEnum` to get the table construction.
- In what were previously `Make` functions, return the element instead
of returning the table.
- By returning the element, no more need to pass in a nullptr (now have
a concrete instance).
I think this is a mild simplification, but maybe worth it.
Note, would appreciate it if there are thoughts on how to provide a
boilerplate `Invalid` implementation (maybe it'd be fine to just return
`nullptr` and cause a crash that way, but I was hesitant to do that).
Previously we walked the global variables defined by the current file
and emitted an LLVM global variable definition for each of them. Now
instead, when emitting a constant reference to a global variable, we
emit an LLVM global variable declaration, and we then subsequently walk
the global variables defined by the current file and convert each of
them from a declaration to a definition.
In order to make import of names of global variables work, add support
for import of `var`, as well as support for importing `tuple_access` and
`tuple_pattern` in the case where the `var` has a tuple pattern in its
declaration. Also treat `bind_name`s that are reference bindings to
`var`s as having the same constant reference value as their `var` so
that we can properly import and lower them.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
Co-authored-by: Carbon Infra Bot <carbon-external-infra@google.com>
Tinkering with #5517, splitting out this suggestion to try to avoid
delaying merge. I figured out what I was missing on the variadiac
expansion. :)
(and also realized the struct could probably be a function)
Move the operation of resolving the specific decl block from
`GetConstantValue()` to `TryEvalTypedInst()`, with is now happening
after replacing the fields of the instruction with new constant values,
but before running the evaluation of the instruction. Since imported
instructions are not evaluated, this avoids resolving the specific decl
block from imported instructions, resolving a TODO in
`AddImportedConstant()`. Now `AddImportedConstant()` can replace
constant values in its fields without having to worry about that
operation resolving any specific decl blocks.
We get to add a new TODO however, to explain why we still need a special
case in resolving specific decl blocks for handling `Impl` construction.
The witness table contains instructions with specifics referring to the
generic self of the impl declaration. But the table must be constructed
before the impl's generic is finished, in order to make the instructions
dependent for the generic. But then resolving the specific decl block
can't be done when the instructions are created and evaluated, as that
requires a finished generic.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
- Track the `VarPattern` instruction on the `VarStorage` instruction so
that it's available for name mangling.
- Mangle global variables based on the first binding name within their
pattern.
- Give global variables external rather than internal linkage, except if
they have no bindings whatsoever in their pattern.
- To support lowering references to bindings nested within a global var,
such as for `var (x: i32, b: i32)`, add some basic initial support for
reference constant expressions. Treat a global `var` as a reference
constant, and treat an aggregate access into a reference constant as a
reference constant.
I'm trying to make the offsetting a little easier to understand, and
also get a better `requires` structure on calls. The second is for an
attempt to refactor the `Formatter` API, but also changing the `InstId`
`derived_from` requires seems helpful for clarity on what's really
happening.
InstValueKind is really just wrapping HasTypeIdMember. Rather than
exposing this as an enum, expose it as a bool since it better reflects
what's going on.
In eval.cpp, AddImportedConstant should never be called on an untyped
instruction.
In FormatInstLhs, we can also depend on whether InstNamer has assigned a
name in order to decide whether to print an instruction. This should
avoid some divergence with CollectNamesInBlock.
We also discussed restoring InstValueKind::Untyped, but that's mainly
motivated by the formatter, and the InstNamer approach gives a more
localized implementation.
#5445 updates to bazel 8.2.1, this does more updates (including to
buildifier, which does autofixes like the `sh_test` loads in the other
PR).
Note I'm using the latest available clang-format wheel. That's not
really something I expect people to have installed, but should mostly be
consistent. I'm specifically skipping clang-format 18 because it had
some broad regressions, and 19 got really confused by a `requires` on a
trailing return. Using the latest seemed probably okay since most people
won't see the difference. Do note that trailing returns in macros,
https://github.com/llvm/llvm-project/issues/47664, seems to be cropping
up again as an issue.
Instead of building an eval block as a separate pass at the end of a
generic, build the eval block incrementally.
The larger change here is that asking for the type or constant value of
an instruction now always returns an unattached type or constant value,
in order to preserve the behavior that we previously achieved by doing
the rewrite to attached types and constant values at the end of handling
the generic.
This also incidentally fixes some subtle issues where attached types and
constant values would leak out into check and cause it to get confused
about differences between attached and unattached values. Check should
no longer see attached values except where it explicitly asks for them.
---------
Co-authored-by: Dana Jansens <danakj@orodu.net>
Remove calls to `InstStore::GetLocId()` to build a LocId from an InstId
now that they can be constructed directly from the InstId. Most uses of
LocId are just plumbing, so this does not affect them. However places
that want to look inside the LocId do not want to work with the InstId
form. In these places, introduce `InstStore::GetResolvedLocId()` which
converts a LocId (or an InstId as an optimization) into a LocId which is
not backed by an InstId. These locations can be printed (they have a
line and column when they are a NodeId), they can have flags added to
them (`ToImplicit`, `ToTokenOnly`), they can be converted to an
underlying ImportIRInstId, or they may be `None`.
`Dump()` is made to print a resolved location instead of printing the
InstId in the location, since (at least in my experience) the resolved
location is what is interesting in debugging, and this saves manual
`MakeInstId` steps in the debugger every time a location is of interest.
The LocId constructor from InstId is made `explicit` to add clarity to
function calls passing an `inst_id` now directly instead of calling
`context.insts().GetLocId(inst_id)`. To avoid needing to construct
`SemIR::LocId(...)` explicitly in all cases though, the diagnostics code
in Check uses `DiagnosticLocId` as its template parameter which accepts
InstId as well and does the construction of LocId from it.
Because LocId now requires an explicit construction from InstId, any
callers to `AddInst()` functions will have to explicitly convert to
LocId if they had an InstId, but not if they pass a NodeId. To make this
difference clear to callers, we `requires` that the input type can be
converted to LocId. This ensures that passing an InstId results in an
error at the callsite where the InstId is passed, instead of generating
a compiler error when trying to construct `LocIdAndInst` inside
`AddInst()`, which is less clear about what went wrong and doesn't seem
entirely intentional.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
Provide builtins for compound assignments instead of defining them in
the prelude as a use of a binary operator and an assignment. This allows
us to lower compound assignment directly to LLVM operations instead of
producing a function call. In the short term this also allows us to
define a type-generic compound assignment in the prelude.
Instead of evaluating a non-parameterized class or interface to a
constant with `SpecificId::None`, use the self specific for that class
or interface, which will not be `None` if there is an enclosing generic.
These constant instructions are all TypeInstId already in their type,
and this makes their names match.
Change the name of MakeSingletonInstId as well and update its comment.
We frequently want to operate on singletons. Per discussion, drop
`Singleton` to make the code shorter.
This started off as wanting to write `inst_id.is_error()`, but the
dependency relationship between ids.h and singleton_insts.h would
require some kind of delayed evaluation to allow the implementation to
remain in headers (which I suspect is helpful to have for inlining). I
could have added something like `IsErrorInst`, forward declared in ids.h
and defined in singleton_insts.h (which would always be included by
typed_insts.h), but the template approach felt like a decent balance
between (a) removing the boilerplate `::SingletonInstId`, (b)
understandability, (c) still visually mirroring if we immediately return
a singleton, and (d) flexibility for more than just `ErrorInst`. But TBH
I'd probably still have written `is_error()` if it didn't require
addressing the cross-header cycle.
Then I tried `SemIR::InstId::Is<SemIR::ErrorInst>`, which generally
worked with types but generated the complaint that it didn't shorten
*all* singleton uses. So pulling back on `::Is`, and instead just
dropping `Singleton`.
This is part of a broader plan to have noop destructor functions for
trivial destruction.
Note this emits a SemIR call (`%no_op: init %empty_tuple.type = call
%NoOp.ref() [concrete = constants.%empty_tuple]`), but not LLVM IR. My
thought was this was probably okay, since even though it'll be a little
spammy with destructor calls, the flipside is there'll probably already
be a fair amount for the name reference, and this at least shows when
the call is injected (and discarded).
---------
Co-authored-by: Chandler Carruth <chandlerc@gmail.com>
Dana Jansens
Jon Ross-Perkins
Richard SmithandGeoff Romer
Thomas Köppe
