This introduces `calling_convention_param_ids`, a single block that
consolidates all the information that was being used by consumers of
`param_refs` and `implicit_param_refs`, in a form that's easier to
produce and typically easier to consume.
See also [this Discord
discussion](https://discord.com/channels/655572317891461132/655578254970716160/1300545448909738125)
regarding the decision to keep the return slot last in the SemIR calling
convention, even though it goes first in the LLVM calling convention.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
The new `FacetValue` instruction represents `C as I` for some type `C`
and facet type `I`. It is named `FacetValue` instead of just `Facet` to
parallel the `FacetType` instruction.
This PR uses this instruction represent the facet value `Self` in an
`impl` declaration. This instruction will be used in the future to also
support things like:
* `C as I` where `C` is a class; and
* forming a specific for a generic with a `T:! I` parameter where `T` is
being given a concrete value.
(Here `I` is an interface or other non-`type` facet type.)
Also do some renaming and add some comments to make things a bit more
clear.
* `FacetTypeAccess` -> `FacetAccessType` to clarify this is not access
of a facet type, but access of the type of a facet
* `.facet_id` -> `.facet_value_inst_id` to parallel the `FacetValue`
instruction
`FacetAccessWitness` will be in a future PR.
---------
Co-authored-by: Josh L <josh11b@users.noreply.github.com>
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Replace the special case adding the base class to the list of extended
scopes with a fully general approach. We use an ImportRef to lazily
import the extended scopes on first lookup.
Adds per-builtin instructions, removing `BuiltinInst`. This collapses
`builtin_inst_kind.def` into `inst_kind.def` so that we have a single
place for all macro uses. I still want to remove `BuiltinInstKind`, but
it's something I think is better separated from the `BuiltinInst`
removal.
I'm collapsing the build targets `ids` and `inst_kind` into one because
they both have links to builtin kind information now. It's hard to
separate without a cycle. I'm using the `typed_insts` name because that
seems like the actual most significant thing there, and more interesting
relative to the `inst` target.
This is for more clearly distinct names, and to make it a clearer
transition from `BuiltinInst` for name conflicts. `FloatType` is also an
instruction, and we have `Carbon::Error` (common/error.h). This avoids
affecting tests, although the name is embedded in the builtin test.
In `LegacyFloatType`, `Legacy` because I was having trouble coming up
with a more appropriate name. I'm not clear this is a `FloatLiteralType`
at present, it needs some work to mirror `IntLiteralType`.
In `ErrorInst`, the suffix `Inst` was discussed as good and similar to
`BuiltinInst` (although I'm trying to get rid of that).
For the few remaining uses of the builtin `i32` type, manually build an
`IntType(Signed, 32)` value instead. These are:
- The return type of `Run`.
- The type that int literals in an `if` expression are converted into.
- The type of an array index expression.
We should consider converting those three cases away from `i32` over
time.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
Stop rejecting `extend adapt` of non-class types such as struct and
tuple. These don't actually work just yet because name lookup into
struct and tuple types is a special case that doesn't handle adapters,
but this gets us a bit closer.
This also slightly improves error recovery for name lookup into an
invalid scope.
When an `IntLiteral` appears as an operand of an `if` expression,
convert it to `i32` for now, so that we don't reject things like `if
cond then 1 else 2` due to having a non-constant value of type
`IntLiteral`.
For tuple indexing expressions such as `(a, b).0`, convert the index to
type `IntLiteral`, not to type `i32`. This isn't strictly necessary to
do in this PR, but avoids the need to provide an `IntLiteral` -> `i32`
implicit conversion for `no_prelude` tests using this syntax.
Instead of leaving array bounds as whatever integer type they arrive as,
convert them to the `IntLiteral` type as part of forming an `ArrayType`.
This ensures that array types canonicalize properly even when the bounds
are specified with different types.
Create an empty generic definition for a generic builtin function to
avoid this causing "use of undefined generic function" errors.
In preparation for changing integer literals to be of `IntLiteral` type.
Conversions from the integer literal type are only permitted when the
value fits within the destination type.
For now, if conversion cannot be checked because the source value is a
symbolic constant, produce a symbolic constant representing the
conversion rather than rejecting it.
- Do not include entities imported from files that we are not dumping.
- Do not include constants and import_refs that are not referenced by
something that we are including in the formatted output.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
Co-authored-by: Carbon Infra Bot <carbon-external-infra@google.com>
As a prerequisite for switching the type of int literals to be the
`IntLiteral` type, add support for performing conversions of in-bounds
integer constant values to other integer types in which they fit.
This incidentally is our first compile-time-only builtin function, so
add very minimal support for compile-time-only functions while we're
here.
* The `extended_scopes` in a `NameScope` were represented by a
`NameScopeId`. Replace that with an `InstId` of an instruction returning
the type that is extending this name scope.
* `Context::LookupQualifiedName` now can take multiple scopes to look
in.
* `GetAsLookupScope` was moved out of `member_access.cpp` and is now
`Context::AppendLookupScopesForConstant`
This PR also fixes some existing issues that were revealed as part of
writing and testing this PR:
* Additional validation and handling of invalid ids.
* `extend impl` in a class is not properly imported yet, but at least
now it doesn't crash.
The change to use an `InstId` also allowed some diagnostics and
formatting to be improved.
---------
Co-authored-by: Josh L <josh11b@users.noreply.github.com>
I'm working to make sure remaining diagnostics have coverage, at least
the ones I'd previously added a TODO for. Note in particular that I
couldn't figure out a repro for UnaryOperatorRequiresWhitespace; if you
have one, I can add a test, but otherwise maybe it's actually
unreachable due to being diagnosed through infix logic (or, maybe
this'll let fuzzing tell me an example).
The test update shows a class derived from an imported base class with a
vptr, and without this change the derived class got its own vptr, with
this change the derived class can see the base is dynamic, so the
derived doesn't need to add a vptr and can rely on the base class's vptr
instead.
This can lead to us trying and failing to print certain kinds of
constant value, but we can fix that in future changes.
Note that `StringifyType` should probably be substantially refactored.
For this change I'm trying to leave the overall structure relatively
intact, but hopefully this additional formatting support will help guide
future refactorings.
While here, also change the diagnostic emission to pass the interface
type rather than the interface name. This prepares us to include the
arguments in the diagnostic.
Instead, wait until we know whether it is used as a value or reference
expression. This allows us to avoid materializing a temporary if it is
used as a value and the initializing representation holds a copy of a
value representation.
This does a few things:
* Replaces the single `TypeId` in the `FacetTypeInfo` struct with a
vector of `InterfaceId`, `SpecificId` pairs (sorted in id order)
representing the set of interface requirements of the facet type. This
will later be used to support facet types with multiple interface
requirements (as in `I & J` or `I where .Self impls J`).
* Replace `InterfaceType` instructions (used as the type of an
`InterfaceDecl` instruction) with `FacetType` instructions (introduced
in #4460) with a (newly introduced) `FacetTypeFromInterface()` function.
* Replace code that consumed `InterfaceType` values with code that
consumed `FaceType` values. I've generally left the assumption in the
code that it is dealing with a single interface, using the (newly
introduced) `FacetTypeInfo::TryAsSingleInterface`, and producing an
error otherwise. There isn't yet support for the `&` operator or `where
.Self impls`, so this is generally a good assumption for now, except you
can get a facet type with no associated interfaces from a `type
where`... expression. In some cases, the facet type value is pulled from
the evaluation of an `InterfaceDecl` instruction, where the single
interface assumption will hold permanently.
* Some related cleans up: nicer stringification and formatting of facet
types, suppression of some errors when there already was an error.
There is still a lot left to do, including:
* Type `type` should be a facet type with a reserved id, replacing the
built-in instruction.
* Code using `TryAsSingleInterface` should generally be upgraded to
handle more than (or less than) one interface. Name lookup should be
particularly exciting.
* Operator `&` should be defined on facet types, unioning their
interface and other requirements.
* Requirements from a `where` clause don't do anything yet.
* Impls and impl lookup need to resolve facet types, and do things like
determine if all the associated constants are given values.
---------
Co-authored-by: Josh L <josh11b@users.noreply.github.com>
Co-authored-by: Geoff Romer <gromer@google.com>
This is primarily to free up space in the BindingPattern insts, but as a
side effect it moves the link between BindingPattern and its BindName
out of the SemIR, and into a transient data structure in Context.
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Adds coverage for an existing diagnostic. Code appears to work without
modification.
---------
Co-authored-by: Josh L <josh11b@users.noreply.github.com>
Adding empty IDs to all block types. This is primarily for
TypeBlockId::Empty, which allows me to disambiguate empty tuple types,
which I think might yield a SemIR readability improvement (PR
forthcoming). I'm splitting PRs so that the limited test impact here is
clear.
This converts `StructTypeField` from an instruction to a dedicated type,
with its own store. This had originated from discussing how
`.GetAs<SemIR::StructTypeField>` was more prevalent than for other
instructions, but is probably more interesting for the storage savings
(16 bytes StructTypeField + 4 byte LocId + 4 byte InstId -> 8 byte
StructTypeField).
Due to the different structure, these now have their own stack during
construction, reducing (but not eliminating) `args_type_info_stack_`
use-cases.
The test changes of different InstIds is expected because structs and
classes generate fewer instructions now. Other than that, results should
remain the same.
I'm generally trying to avoid unrelated cleanup here due to the PR size,
though I did scrutinize the `VerifyOnFinish` calls, adding one and
commenting others (putting them in member order because that's how I was
checking what was verified and what wasn't).
The cehck for vptr could be done differently (is this class dynamic and
its base class non-dynamic), and if we change the layout (to put the
vptr after any base) then this code will break (could add an assertion
that the vptr isn't present apart from at the start of the field list if
that'd seem worthwhile).
There's still later issues with lowering (if this patch causes crashes
in lowering, do they result in fuzz failures/need to be avoided before
this change is submitted?)
When an instruction is created as part of an implicit call to an
interface member, we generated a bunch of constants for naming the
interface, finding the corresponding specific, accessing its member
function, and so on. This led to significant bloat in SemIR.
Instead, we now track whether an instruction is created implicitly in
its location, and where relevant, we use the constant value of the
instruction directly instead of storing a new `Inst`.
This doesn't reduce the amount of work we need to do, but does make the
representation in SemIR smaller and more readable.
Still to do:
* Represent facet type values in a canonical form
* Produce & consume facet type values instead of interface values
* `type` should be associated with a canonical facet type value
* Support `&` on facet type values
* Type check and enforce requirements in facet types
---------
Co-authored-by: Josh L <josh11b@users.noreply.github.com>
An invalid parse due to an error token isn't likely a great diagnostic
as it will already have been diagnosed by the lexer. A common case to
start handling that is when the parser encounters an invalid token when
expecting an expression.
This removes a number of unhelpful diagnostics after the lexer has done
a good job diagnosing.
This also means that there may be parse tree errors that aren't
diagnosed when there are lexer-diagnosed errors, so track that.
Follow-up to #4430 that almost finishes addressing its diagnostic TODO.
This instruction represents integer values, whether they come from
literals or calculations, so it the old name is inaccurate. I also plan
to rename `BigInt` to `IntLiteral` based on recent discussion and this
change aims to avoid confusion stemming from the same name being used
for two different things.
I'm not renaming `FloatLiteral` because recent discussion suggests we
may want distinct `FloatLiteral` versus `FloatValue` representations in
SemIR.
Initialization can import a function and thus invalidate the reference
we're holding to the enclosing function. Don't use the reference after
initialization completes.
We previously generated a `value_bind` instruction of the wrong type,
resulting in lowering building bad LLVM IR.
Also fix another issue exposed by this change, where we would import
constants without marking their types as complete, and then crash in
lowering while trying to lower them. This happens in particular for the
`FunctionType`s of functions in `ImplDecl`s. Address this by skipping
lowering for constants with incomplete types.
This is because `var x:! () = ();` modifies the binding index, which
causes `A` to be generic, which causes the params to be used, which
crashes. There may be another issue to fix here so that the invalid
binding doesn't modify the binding index, but at least
`param_patterns_id` should probably be set consistently with
`params_id`.
This is in particular to avoid churn from changes such as #4370. I think
the import list can be helpful (particularly to understand what the
library is aware of), but it's a different trade-off for the prelude
package due to the implicit imports.
This PR makes it so that types can implement the `IndexWith` interface
so that they can provide their custom indexing behavior.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>