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 allows us to import the table for a given impl only once, while we
can import many ImplWitness instructions with different specifics for a
generic impl.
For example in convert_facet_value_to_narrowed_facet_type.carbon we see
that a single witness table is imported for the BitAnd interface, with
multiple witnesses (for different specifics) imported and sharing the
same table.
The ImplWitnessTable now contains a back-link to the Impl the witness is
for, allowing inst namer to name that interface in the textual semir,
and allowing the interface to be found when debugging from a witness.
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
While facets may come with a rewrite for an associated constant, they
are symbolic. A final impl has the ability to provide a concrete value
instead, which allows generic code to use the concrete value in place of
the associated constant's (fully qualified) name.
For instance, instead of `I.Type`, the concrete type `()` can be used if
there is an `impl final [T:! type] T as I where .Type = ()` impl.
This does not yet cache the result of the lookups.
Depends on https://github.com/carbon-language/carbon-lang/pull/5255
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Instead of using None, use an explicit ImplWitnessTablePlaceholder in
the witness table for entries that have not yet been populated, to aid
debugging. This would ensure they would show up very clearly in the
SemIR. This uncovered some `<invalid>` in the SemIR under erroneous
conditions that have now been turned into `<error>`.
Add the ImplWitnessAssociatedConstant instruction which wraps the
canonical instruction found from the constant value of the rewrite
constraint. This ensures that we have an instruction inside the eval
block for a generic impl declaration for each rewrite constraint's
value, which allows Subst to be performed to rewrite the symbolic
constant of the ImplWitnessAssociatedConstant instruction to associate
it with the generic. This will prevent the otherwise orphaned symbolic
constant of the rewrite's value from being used which can not have a
specific applied to them.
While applying the new insts in InitialFacetTypeImplWitness(), rearrange
the function to use less nesting. And avoid using entity names from
imported instructions (as we found is not effective in deduce.cpp) and
use a local instruction by going through the constant value.
This PR is part of the effort to allow a rewrite to name a generic
parameter, such as `impl forall [T:! type] T as Z where .X = T`, however
tests for this involve a final impl so that we can typecheck that the .X
value is a specific T, so the tests will come with that work. This piece
is split off because introducing new instructions causes a lot of SemIR
churn, and I wanted to get that done separately.
Implements some of the changes from proposal #5168.
* The data structure for complete facet types has been repurposed for
identified facet types. Identified facet types are now a concept in the
toolchain, but without named constraint support they are not
substantially different from incomplete facet types.
* Identified facet types keep the list of required specific interfaces
in sorted order, for efficiency improvements in impl lookup. Found
another way to identify the interface to impl (or number of impls if not
1).
* Forward `impl` declarations of identified but incomplete facet types
are allowed unless the facet type has rewrites. An incomplete facet type
with rewrites is already either an error or has more than one interface
and so can't be implemented, so this case can't be exercised very well
yet.
* Forward `impl` declarations of interface without rewrites use a
placeholder inst block for the witness.
* Changed some machinery to use RequireIdentifiedFacetType to access the
interfaces of the facet type so we only need to add support for
expanding named constraints into interfaces in one place.
---------
Co-authored-by: Josh L <josh11b@users.noreply.github.com>
Co-authored-by: Dana Jansens <danakj@orodu.net>
I was thinking about this after `seq` changes in #5182, and looked for
other uses that might be replaceable. Here's the resulting cleanup
around `seq`:
- Switch to `enumerate` or `zip` when possible.
- `int _` -> `auto _` (it's typically a `size_t`, but there's no reason
to cast when unused)
- Fix a case of cast style `(size_t)...` -> `static_cast<size_t>(...)`
- Switch `(void)close_children_count` to `[[maybe_unused]]`
This allows the numbering of the parameters to match when checking for a
valid redeclaration. It also prepares us to produce the proper numbering
when generating a thunk.
This tripped over a lowering crash when a member function with self was
declared-but-not-defined, so that's why some test cases were updated to
have (empty) function definitions.
I'll follow-up with/look into a fix for the
self-declared-but-not-defined cases separately.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
Co-authored-by: Carbon Infra Bot <carbon-external-infra@google.com>
There aren't remaining uses on `Context` other than `DiagnosticEmitter`
itself. I'm adding `SemIRLoc` because I feel odd about having both
`Carbon::Check::DiagnosticBuilder` and
`Carbon::DiagnosticEmitter<T>::DiagnosticBuilder`, but it seems
relatively little additional typing outside the handful of
`DiagnosticEmitter` uses on `Context` itself:
```
Context::DiagnosticEmitter
DiagnosticEmitter<SemIRLoc>
Context::DiagnosticBuilder
SemIRLocDiagnosticBuilder
Context::BuildDiagnosticFn
BuildSemIRLocDiagnosticFn
```
Also clean up #include's while I'm finishing here.
* Add `RequireCompleteFacetType` and `ResolveFacetTypeImplWitness` to
`check::Context`. Goal was to move code from `impl.cpp` (mostly) without
functional changes.
* Complete type information is cached with the facet type, and is stored
in a `complete_facet_types()` table.
* Main functional change is to diagnose attempts to use a rewrite
constraint on an associated function. Some existing diagnostics have
been updated.
* Remove `check::Context::RequireDefinedType`:
* For class types, use `RequireCompleteType`
* For facet types, use `RequireCompleteFacetType`
* Introduce a `SemIR::SpecificInterface` to hold an interface and
specific id pair.
* Keep the specific interface ids in the impl object.
* Avoid some extra copies in `Dump` functions.
* Future work missing from this PR:
* Resolving for member access or actions that require impl lookup.
* Resolving rewrites constraints that refer to non-concrete values.
* Any support for adding implied constraints that result from a `where`
clause (though TODOs have been added).
---------
Co-authored-by: Josh L <josh11b@users.noreply.github.com>
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Co-authored-by: Dana Jansens <danakj@orodu.net>
Currently it returns false which just ends typechecking. Instead handle
the error state later and avoid firing overlapping diagnostics in
'extend impl as'.
---------
Co-authored-by: josh11b <15258583+josh11b@users.noreply.github.com>
This in particular uses free functions because it's likely to end up
more consistent with types (versus a wrapper object for InstStore).
Note, this is unlikely to have a performance impact, but if it does, we
can look into related approaches (and we've already discussed using
LTO).
Renames `PendingBlock::AddInst` to `PendingBlock::Add` because
`MakeElementAccessInst` expects the matching name to exist.
This creates a new check/type.h for most logic, and also moves some
functions to TypeStore in sem_ir/type.h. My approach for TypeStore is to
focus on moving the read-only functions there.
context.cpp is getting large, so I'm looking at a few ways to cut out
clusters of functions. This felt like a logical cluster of functions to
move to their own file.
Note I have two commits in this PR: one moving the functionality to a
new file, and one specifically changing TypeCompleter to use out-of-line
function implementations. This is to assist reviewability.
Based on [the lastest thinking on
#4672](https://github.com/carbon-language/carbon-lang/issues/4672#issuecomment-2606209281)
, require a full syntactic match for impl redeclaration, instead of
excluding the `where` restriction. This means no updates to the impl
witness on redeclaration, and no diagnostics that those updates are
consistent.
Not included in this PR, but will need to be done in the future:
* Support for assigning values to associated constants in the body of
the impl definition. This will require moving the checking that
non-function associated constants are set from the definition start to
definition end.
* Identify semantic redeclarations that are not syntactic matches to
give a failed redeclaration diagnostic. This should be done once we are
already identifying impl declarations with the same type structure in
order to require they be identified in an impl_priority/match_first
block.
* Merging of the functions in `check/impl.cpp` that are now always
called together.
Also add some test coverage of `where` parsing I developed in PR I've
now abandoned because of this new simplification of the impl
redeclaration semantics.
---------
Co-authored-by: Josh L <josh11b@users.noreply.github.com>
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
Benefits:
* Provide a proper API for accessing lookup information.
* Make assumptions on whether the result is poisoned or not and how we
can use `InstId` explicit.
* Allow safely reusing the `InstId` value for pointing to the poisoning
entity for poisoned results (in a future PR).
* Consolidate `LookupNameInExactScopeResult`, `std::pair<SemIR::InstId,
bool>` and part of `LookupResult`.
Part of #4622.
Add a full entity representation for associated constants, and build a
`Generic` object for them. This `Generic` is parameterized by the
enclosing `Self` type, allowing the use of `Self` within the type of the
associated constant to be supported.
When performing impl lookup for an associated constant, produce the type
with the provided self type substituted for its `Self` along with any
generic parameters of the interface.
Split the handling of associated constant declarations into two parts,
corresponding to the code before the `=`, and the code between the `=`
and `;` (if any). The former goes into the generic declaration region;
the latter into the generic definition region. This prepares us to
handle the default value for an associated constant, but for now we're
just storing the information and not actually using it.
Remove the entity type field from `assoc_entity_type`, because it's
almost unused and is an attractive nuisance -- it must necessarily be a
type in the generic scope of the associated constant rather than in the
scope of the instruction (because there is no `Self` anywhere else),
which means that it's hard to substitute into or derive meaning from.
See `toolchain/check/testdata/impl/assoc_const_self.carbon` for tests of
the new functionality; these used to cause the toolchain to crash.
High level, replacing `Id::Invalid` with `Id::None` and `Id::is_valid`
with `Id::has_value` for clarity, as discussed
[here](https://discord.com/channels/655572317891461132/655578254970716160/1331664574545395794).
The `IntId` refactoring is needed together with `AnyIdBase` because it's
also used with `ValueStore`.
Note, trying to be careful not to rewrite `EnumBase::InvalidIndex`, or
`is_valid` in general (e.g., `IdKind::is_valid`).
I've tried to sequence commits here:
1. Automatic replacements:
- `((?:Id|Index)(?: |::|\(|Base(?:\(|::)))Invalid((?:Index)?\W)` ->
`$1None$2`
- `<invalid>` -> `<none>`
- `InvalidNodeId` -> `NoneNodeId`
- `/\*invalid\*/` -> `/*none*/`
- `id((?:_|\(\))(?:\.|->))is_valid` -> `id$1has_value`
2. Manual edits:
- In `int.h` and `int_test.cpp`
- `IntT` has `is_value`, which I'm renaming to `is_embedded_value`.
- Manual edits to comments in this file.
- `AnyIdBase` and `IdBase`
- Declaration of `is_valid` -> `has_value`, `InvalidIndex` ->
`NoneIndex`.
- In `ids.h` and `ids.cpp`
- `is_valid` -> `has_value`
- `// An explicitly invalid ID.` -> `// An ID with no value.`; similar
for index
- Various math on `InvalidIndex` -> `NoneIndex`
- Various mentions of "valid" in comments
- In `value_store.h`, for `IdT::Invalid`, plus one comment
- In `impl.h` and `tokenized_buffer.h`, we had different initialization
of `::None` values (versus `ids.h` syntax) that I fixed manually.
- Spot checks to compile
- Particularly where `is_valid` replacements didn't catch spots due to
different naming.
3. Autoupdate tests
4. verbose.carbon (NOAUTOUPDATE)
5. Comment spot checks
Note there are probably other mentions of "Invalid" that should be swept
up, but I'd like to argue for merging and separating out remaining
cleanup since this is so sweeping (and likely to hit merge conflicts
from churn). We'll probably have lingering mentions of "invalid" for a
bit regardless, just because there are uses of "invalid" in non-Id APIs.
I believe `check_syntax` is already controlling the semantic vs
syntactic merge, added in #4149. Other parts of the TODO are clarified
per discussion. But this is tested, e.g. errors with the bool flipped:
```
impl i32 as I {
+ // CHECK:STDERR: method.carbon:[[@LINE+6]]:14: error: redeclaration syntax di
ffers here [RedeclParamSyntaxDiffers]
+ // CHECK:STDERR: fn F[self: i32](other: i32) -> i32 = "int.sadd";
+ // CHECK:STDERR: ^~~
+ // CHECK:STDERR: method.carbon:[[@LINE-7]]:14: note: comparing with previous
declaration here [RedeclParamSyntaxPrevious]
+ // CHECK:STDERR: fn F[self: Self](other: Self) -> Self;
+ // CHECK:STDERR: ^~~~
fn F[self: i32](other: i32) -> i32 = "int.sadd";
}
```
With this change, we now support impl of interfaces with non-function
associated constants.
Also:
* Make impl diagnostics use more consistent names
* Make some impl tests "no_prelude"
Still to do:
* Facet type resolution as a separate, reusable step
* Using the assigned values of associated constants (see
`fail_todo_use_assoc_const.carbon`)
---------
Co-authored-by: Josh L <josh11b@users.noreply.github.com>
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Change the implementation to use an explicit `is_poisoned` bit instead
of `InstId::PoisonedName` value.
Zero behavior change.
This would allow to more easily change the API to support accessing the
poisoning declaration so we can have better name poisoning diagnosis.
#4622
* Change `InterfaceWitness` -> `ImplWitness`
* Include a `SpecificId` in the `ImplWitness`. This allows the
`InstBlock` it contains to have its own identity, allowing it to be
changed as the impl is processed. Evaluation only updates the specific.
* Create the `ImplWitness` at the start of the impl definition. In the
future, this will be populated with the values of non-function
associated constants. For now, it starts full of invalid instruction
ids.
* Implements the model suggested in #4672 .
Note that the non-SemIR testdata changes are to these file:
* `toolchain/check/testdata/impl/lookup/fail_todo_undefined_impl.carbon`
* `toolchain/check/testdata/struct/import.carbon`
* `toolchain/check/testdata/tuple/import.carbon`
The last two are due to an import of generics bug exposed by this PR,
which will be fixed in a follow-on.
---------
Co-authored-by: Josh L <josh11b@users.noreply.github.com>
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Also fix a bug in `Context::GetClassType` that previously tried to
complete the class type before returning it. That's not correct --
`GetCompleteTypeImpl` is only appropriate for cases where the type can
trivially be completed and completing it can't fail -- and led to
infinite recursion with this change because we would call `GetClassType`
when producing a diagnostic if completing that class type failed.
When a generic requires a symbolic type to be complete, add a new
`require_complete_type` instruction to the generic eval block. During
monomorphization of such an instruction, require that type to be
complete.
`ids.h` and `ids.cpp` are the manual edits, everything else is
search-and-replace.
The full list of things moved is:
- `TypeId::TypeType`
- `TypeId::AutoType`
- `TypeId::Error`
- `ConstantId::Error`
This is to unblock removing `InstId::Builtin*`.
* Rewrite constraints are stored in a facet type, substituted, imported,
and formatted.
* We now distinguish `.Self` from other symbolic bindings in two ways:
* `.Self` itself now has an invalid compile time binding index (since it
doesn't bind to any of the generic parameters). As a result, we no
longer need to create a generic region in `handle_where.cpp`.
* There is a new phase tracking values that are only symbolic because
they transitively depend on `.Self`. This allows us to give the result
of a `where` expression template phase as long as it doesn't use any
symbolic constants other than `.Self` or other designators.
* `AddConstant` has been removed from `check/context` since it was only
used from `eval`. This meant less plumbing of the phase change.
* Evaluation of `BindSymbolicName` now also performs substitution into
its type.
* Include a bit more information in some diagnostics.
* `StringifyTypeExpr` outputs rewrites, which required adding support
for associated entities as well.
* Associated entities now have an entity name set when importing.
* Adds tests for some interesting cases with rewrites and uses of
`.Self` mixed with other symbolic constants.
Still to do:
* There is no validation that any particular type satisfies rewrite
constraints.
* Access to members of a facet type do not see the rewritten values.
* Impls don't recognize whether associated constants have rewrites
setting their values.
* No support for resolving facet types.
---------
Co-authored-by: Josh L <josh11b@users.noreply.github.com>
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
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>
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).
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 a primarily automated change:
- Search & replace for capitalization
-
`(CARBON_DIAGNOSTIC\((?:\n\s+)?\w+,(?:\n\s+)?\s\w+,(?:\n\s+)?\s")([A-Z])`
- `$1\L$2`
- Search & replace for period
-
`(CARBON_DIAGNOSTIC\((?:\n\s+)?\w+,(?:\n\s+)?\s\w+,(?:\n\s+)?\s"(?:[^)]|\n)+)\.("[,)])`
- `$1$2`
- Limited search & replace for `ERROR: ` -> `error: ` in streamed things
- Leaving a TODO for command_line because there's more cleanup that can
be done there
- Modify diagnostic_consumer.cpp
- ERROR -> error
- WARNING -> warning
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
This switches `DCHECK` and `FATAL` as well.
The goal is to reduce the code size impact of these assertions so that
we can keep more of them enabled. Currently, the largest cost I see from
`CHECK` is not the actual check or the cold code itself, but actually
the failure to inline trivial functions due to the presence of the cold
code. This means that our goal isn't to reduce apparent code size in the
final binary but the LLVM IR cost assessed for these routines in the
inliner, which closely correlates with code size but is a bit different.
As discussed in #4283, experimentation shows that a single function call
with a minimal number of arguments is the lowest cost model for these.
This is easily achieved with a format-string API that internally uses
`llvm::formatv`. This PR is essentially the `CHECK` version of #4283.
However, the check macros are substantially harder to make work with
both format strings and streaming because they also take a condition.
Also, unexpectedly, I was very successful at devising a regular
expression based automated rewrite from the streaming to the format
string form with only low 10s of manual fixes. This includes compacting
strings broken up across lines, etc. Given how well that went, I've
prepared this PR which just directly switches to the format string API
and migrate everything to use it.
One nice side-effect is that the format string approach ends up greatly
simplifying the implementation here as well.
This is ... *shockingly* effective. Parsing speeds up by more than 3%
with just this change. And checking speeds up by **8%** with this change
alone:
```
BM_CompileAPIFileDenseDecls<Phase::Parse>/256 86.3µs ± 1% 82.9µs ± 1% -3.94% (p=0.000 n=17+19)
BM_CompileAPIFileDenseDecls<Phase::Parse>/1024 431µs ± 1% 415µs ± 1% -3.76% (p=0.000 n=18+19)
BM_CompileAPIFileDenseDecls<Phase::Parse>/4096 1.77ms ± 1% 1.71ms ± 1% -3.18% (p=0.000 n=18+19)
BM_CompileAPIFileDenseDecls<Phase::Parse>/16384 7.44ms ± 1% 7.17ms ± 2% -3.56% (p=0.000 n=18+20)
BM_CompileAPIFileDenseDecls<Phase::Parse>/65536 30.7ms ± 1% 29.7ms ± 1% -3.15% (p=0.000 n=18+20)
BM_CompileAPIFileDenseDecls<Phase::Parse>/262144 131ms ± 1% 127ms ± 1% -2.81% (p=0.000 n=18+18)
BM_CompileAPIFileDenseDecls<Phase::Check>/256 878µs ± 2% 800µs ± 1% -8.91% (p=0.000 n=19+20)
BM_CompileAPIFileDenseDecls<Phase::Check>/1024 1.88ms ± 2% 1.72ms ± 1% -8.56% (p=0.000 n=19+20)
BM_CompileAPIFileDenseDecls<Phase::Check>/4096 5.78ms ± 2% 5.28ms ± 1% -8.70% (p=0.000 n=20+18)
BM_CompileAPIFileDenseDecls<Phase::Check>/16384 21.9ms ± 1% 20.1ms ± 1% -8.02% (p=0.000 n=18+20)
BM_CompileAPIFileDenseDecls<Phase::Check>/65536 90.4ms ± 2% 83.1ms ± 1% -8.04% (p=0.000 n=19+20)
BM_CompileAPIFileDenseDecls<Phase::Check>/262144 381ms ± 2% 352ms ± 1% -7.79% (p=0.000 n=19+19)
```
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Co-authored-by: josh11b <15258583+josh11b@users.noreply.github.com>
Remove `ReusingLoc` and add enforcement that even for imported
locations, the kind of the parse node for an instruction matches the
kind specified in the instruction definition.
Change the node kind for a few instructions to `NodeId`:
- A couple of instructions had a typed node but could be created
implicitly with any node as part of a builtin implicit conversion. This
happened for `AddrOf`, `ArrayIndex`, and `Deref`.
- A bunch of instructions had `InvalidNodeId` as their associated parse
node kind but were actually always created with a location.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
This adds fields to `EntityWithParamsBase` to reflect the intention with
`extern library` design. I'm renaming `decl_id` because it shouldn't be
expected to be assigned anymore. import_ref.cpp I'm deliberately keeping
on `first_owning_decl_id` (which will break when importing `extern
library` declarations). Most other cases are for diagnostics, and I'm
using `latest_decl_id` to try and get the closest declaration to the
error. Note I'm partly splitting out this PR to show the test effect,
which apparently we don't test related cases.
This prepares us for modeling associated entities of parameterized
interfaces.
We don't use the interface parameters when type-checking `impl`s or uses
of interface members yet, but we do now check interface arguments during
`impl` lookup.
Dana Jansens
Jon Ross-Perkins
Boaz Brickner
Brymer Meneses