`TemplateInst` wraps another inst. If that inst is symbolic, it is
treated as a template by `OperandDependence`.
Use this to replace `CallCppTemplateAction` with the more general
`CallAction`.
Instead of allowing lower to pick whatever type layout it desires,
compute the layouts of types as part of completing the type, and make
lower build types that match that representation.
For now we assume that all pointers are 64-bit, since we don't have
access to target information. We allow tail padding reuse for structs
and tuple types (and by extension, for classes, since they use structs
as their object representation), but not for arrays.
In order to build matching LLVM types, we create LLVM packed structs
where necessary, and we insert inter-field padding on the end of the
previous field so that GEP indexes still always match Carbon's
ElementIndexes.
We don't yet use the computed alignment much in LLVM IR generation -- in
particular, `alloca`s, `load`s, and `store`s should probably use the
computed type alignment, but don't.
Assisted-by: Gemini via Antigravity
Ignore storage arguments when evaluating a call, like we do for other
kinds of instruction. Create a placeholder constant to represent each
out parameter so that it can be used in the function body to form more
storage arguments.
Assisted-by: Gemini 3.1 Pro via Antigravity
* When a C++ static data member is imported, evaluate its address to a
constant like we would for a namespace-scope variable.
* When an imported variable is used in a way that doesn't require its
type to be complete, emit the variable with an opaque type instead
of skipping it (and potentially crashing later).
Instead of comparing `InstId` indexes, which aren't *necessarily* in the
same order as raw indexes, compare the raw indexes themselves. Convert
the test for out-of-order lowering into a `CHECK` failure if a constant
is found to refer to another constant with a later-created instruction.
In principle this is fixing a bug: if there were so many files and
instructions that the bits of the tag overlapped the bits of the
`InstId`, we could return `nullptr` for a constant that actually had a
value. But in practice this would be very hard to test, and even harder
to test reliably, so I'm not including a test here. The purpose of this
change is to add the `CHECK`, not to fix an obscure bug.
Add a `Core.CppCompat.NullptrT` type that C++'s `nullptr_t` maps into.
Map `nullptr` to an uninitialized constant of that type -- `nullptr`
doesn't actually have any defined bits within it, despite having the
same representation as `void*`.
Add a `Core.String` class to the prelude representing a string view, and
rename the `String` keyword to `str` and make it evaluate to
`Core.String`.
`Core.String` is represented as a pair of a pointer to a character
(actually, to the first character of a string, but we don't have a way
of modeling that yet) and a size (which should be pointer-width, but is
currently always a `u64` as we don't have a `usize` equivalent yet).
`Core.String` values are generated directly by the toolchain for string
literal expressions.
This follows the direction established at the recent summit, but the
design implemented here has not been through the proposal process 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.)
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>
This is from a quick discussion with zygoloid. There's a mix of uses,
we're both comfortable with a "minimum syntax" decision here to use ADL.
Note this doesn't touch llvm::cast uses in yaml_test_helpers, but ADL
doesn't work there (because the objects are in a `llvm::yaml`
namespace). This gets to another choice made here: if we specify a
namespace, prefer `llvm::` because it's shorter. `clang` has a using of
them, but it's probably better to point at the canonical version if
we're being explicit about it.
Some specific features:
* Use `SpecificFunction` for vtable entries for generic classes.
* Create specific constants for vtable entries in classes derived from
generic classes to reference the appropriate specific of the function
in the context of such a derived class.
* Create specific constants for vtable_ptrs for uses of specific generic
classes.
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
The goal was/is to reduce the overhead for vtables in generics - the
previous representation/prior to this patch caused a new vtable to be
created in every specific which isn't generally what we want for Carbon
generics (the whole specific/generic thing is meant to avoid creating
specific versions for things that can be a generic form parameterized by
a specific instead of manifest as a unique entity per specific)
So this moves vtables to a top level object (like functions, classes,
etc). Each dynamic class will have a vtable in this list.
Classes have a `vtable_ptr` instruction in them that points to the
vtable.
The actual generic support hasn't been implemented in this patch, as
I've been struggling with just getting this part of the migration going
& wanted to get it flushed out before adding the additional
complications.
It's possible more laziness when doing cross-file importing would be
suitable - for instance if we only need to reference the vtable from
another file, but don't need to know its individual contents, it may be
beneficial for the functions in the vtable to be import_refs (or to add
another layer of indirection - so it can be a single import_ref
all-or-nothing for the functions in the vtable).
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Co-authored-by: Carbon Infra Bot <carbon-external-infra@google.com>
Changes the vectors on `Lower::FileContext` to be `FixedSizeValueStore`
where possible, which we have several at this point.
This changes `FixedSizeValueStore` to prefer inferring the size from a
`ValueStore<IdT>`, which should make adding incorrect sizes harder. Note
I wasn't sure that adding a `size()` to `TypeStore` that returned
`insts().size()` would be good because it doesn't directly work that
way; `ConstantValueStore` would've also required more work since it
doesn't have access to that right now.
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>
- 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.
Replace the large and growing `TryEvalInstInContext` function with one
function per kind. While we still have special-case handling for a small
number of instruction kinds, most instructions are now handled either
fully automatically or use a common codepath that evaluates the
instruction operands and then performs an eval-context-independent
evaluation of the instruction.
To support this, `InstConstantKind` is expanded to describe more
fine-grained details about how each kind of instruction interacts with
constant evaluation. Also, the operand kinds of instructions become
slightly more fine-grained: we now distinguish between operands that
describe the destination of an initializing expression (`DestInstId`)
from other `InstId` operands, because `DestInstId` operands need
different treatment during constant evaluation. In particular, an
initializing expression can have a constant value even if its
destination is non-constant or has not yet been set, because evaluation
of an initializing expression doesn't include the store to the
destination.
Some minor test changes:
- We now more consistently propagate errors into the results of constant
evaluation, so more instructions that depend on errors have a constant
value of `<error>`.
- Diagnostic location for invalid array types now point at the whole
array type rather than the array index expression, because
`EvalConstantinst` doesn't have access to the original expression.
- Diagnostic for failed `RequireCompleteType` doesn't print the original
type any more because `EvalConstantInst` doesn't have access to the
original expression.
As a follow-up, some of this -- in particular, the `EvalConstantInst`
overloads -- will be moved to a separate file, in an effort to split the
overall constant evaluation machinery apart from the logic to evaluate
each individual kind of instruction.
Noted CopyNameFromImportIR while glancing around (this one's interesting
because it's NameId, not void nor auto), did a scan just for a few other
cases. Not an exhaustive fix, and TBH assuming we'd prefer `auto ... ->
auto` since equivalent Carbon syntax would probably be `fn ... -> auto`
This allows iterating on all values in a store along with the Id for
each value, instead of `llvm::enumerate(store.array_ref())` which would
give you the indices.
While the indices are really the same as the Ids, this provides a
typesafe way to enumerate() over a store.
There's no use for this right now, but I thought I needed this, and it
helped me debug, and it was a pain to write correctly without dangling
references.
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
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.
The InstId in this field is to an instruction that declares the
function, rather than the function itself, so that diagnostics can print
where the function is coming from. The type of the function (the
FunctionType instruction) is the type (the type_id) of the
function_decl_id. So we rename the field to help make this distinction
more clear.
Followup to #4739
* 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>
Fixes integer builtins to produce the correct values (and not
CHECK-fail) when used on integer literals. Also adds impls to the
prelude to use the new builtins to perform operations on integer
literals.
Perhaps most importantly, this allows directly initializing `i32` values
with negative numbers, as the negation operation on integer literals now
works.
For testing I've added tests for use of literals with one operator in
each class (addition, multiplication, ordering, bitwise, etc) for which
there are distinct rules or overflow behavior, rather than exhaustively
testing all the combinations. This is aimed at finding a good tradeoff
between maintainability of the tests and thorough test coverage.
Also fixes lowering of heterogeneous shifts and comparisons. These are
currently disabled when one of the operands is an integer literal, but
we may want to allow that when the integer literal operand has a known
constant value.
Represent the type as an `InstId` rather than as a `TypeId` to preserve
how it was written and better support tracking its value in a generic.
Add accessors to `Class` to get the base and adapted type to reduce code
duplication, and add `TypeStore::GetObjectRepr` to make it easier to map
from a type to its possibly-adapted object representation type. In
passing, also move `GetIntTypeInfo` and `GetUnqualifiedType` into
`TypeStore`.
This fixes specifics of generic adapters to properly look at the
specific adapted type, and also fixes importing of adapters.
---------
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>
The first change here is to canonicalize away bit width when tracking
integers in our shared value store. This lets us have a more definitive
model of "what is the mathematical value". It also frees us to use more
efficient bit widths when available, such as bits inside the ID itself.
For canonicalizing, we try to minimize the width adjustments and
maximize the use of the SSO in APInt, and so we never shrink belowe
64-bits and grow in multiples of the word bit width in the
implementation. We also canonicalize to the signed 2s compliment
representation so we can represent negative numbers in an intuitive way.
The canonicalizing requires getting the bit width out of the type and
adjusting to it within the toolchain when doing any kind of math, and
this PR updates various places to do that, as well as adding some
convenience APIs to assist.
Then we take advantage of the canonical form and embed small integers
into the ID itself rather than allocating storage for them and
referencing them with an index. This is especially helpful for the
pervasive small integers such as the sizes of types, arrays, etc. Those
no longer require indirection at all. Various short-cut APIs to take
advantage of this have also been added.
This PR improves lexing by about 5% when there are lots of `i32` types.
---------
Co-authored-by: Dana Jansens <danakj@orodu.net>
Co-authored-by: Carbon Infra Bot <carbon-external-infra@google.com>
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
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).
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.
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.
Generate declarations of specific functions on demand. Definitions are
not emitted yet, and I'm using a temporary, known-broken scheme for name
mangling.
Add a new `specific_function` instruction that represents a generic
function plus its deduced argument list as a callee in a function call.
The new instruction can only appear as the immediate operand of a call
instruction, so we give it a builtin placeholder type.
At the end of each file, require definitions for all specific functions
used in that file. Resolve the generic with the argument list to produce
those specific function definitions as needed, and diagnose if the
generic doesn't have a definition available.
A few tests are updated in cases where they declared and used generic
functions but didn't previously provide a function definition.
Add support for initializing types like `GenericClass(i32)` from a
struct literal. A new kind of instruction, `complete_type_witness`, is
added to the class definition to track the object representation type so
that it's visible to the generics machinery. Accesses to the object
representation of a class have all been updated to pass in the class's
`SpecificId` so that the types of the fields of the specific class are
used instead of the types of the fields of the generic class in places
that look at the object representation -- primarily class
initialization.
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>
We can't use the instruction from the generic directly, because it
doesn't have the right constant value. Instead add an instruction that
models the transition from the constant value in the generic to the
constant value in the generic instance.
Also start associating the self generic instance with unqualified
lookups that find results in an enclosing generic, so that we track the
information necessary to create the new instruction.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
This adds `DefinitionInfo` for `Define`-based configuration so that
parameters are optional. It also makes it easier to provide the
equivalent functions on both `Definition` and `Define`.
A common pattern used here is to change from a `switch` with in-line
`case`s to instead have `case`s that call an overloaded function. What's
happening here is that the instruction type is used to select an
overload, and if an overload is not defined, a compiler error would
result. Meanwhile, clusters of overloads are being defined using
`requires`-based templating, so that equivalent implementations are not
copied. This addresses a limitation of a vanilla `switch` approach where
it's hard to have redundant cases using conditional logic, while also
getting compiler errors when adding new `InstKind` entries, which had
been a significant part of why we used macros previously.
This starts hitting some odd clang-format edge cases causing
`CARBON_KIND_SWITCH(inst){` (missing space), which I haven't seen
before. Adding `CARBON_KIND_SWITCH` to .clang-format works around it.
Require mapping from a `ConstantId` to an `InstId` to go through the
`ConstantValueStore`.
This is a preparatory step for an upcoming generics change where
symbolic `ConstantId`s are no longer just a thin wrapper around an
`InstId` but instead are indexes into a table with additional
information about the symbolic constant beyond its `InstId`.
Most of these are places where we failed to include a header file and
simply never got an error about this. The fix is to include the header
file.
Most other cases are functions that should have been marked `static` but
were not. Finding all of these was a main motivation for me enabling the
warning despite how much work it is.
One complicating factor was that we weren't including the `handle.h` for
all the state-based handler functions. While this isn't a tiny amount of
code, it is just declarations and doesn't add any extra dependencies. It
also lets us have the checking for which functions need to be `static`
and which don't. For the `parse` library I had to add the `handle.h`
header as well, I tried to match the design of it in `check`.
I have also had to work around a bug in the warning, but given the value
it seems to be providing, that seems reasonable. I've filed the bug
upstream: https://github.com/llvm/llvm-project/issues/94138
I also had to use some hacks to work around limitations of Bazel rules
that wrap `cc_library` rules and don't expose `copts`. I filed a bug for
`cc_proto_library` specifically:
~https://github.com/bazelbuild/bazel/issues/22610~https://github.com/bazelbuild/bazel/issues/4446
This removes the builtin FunctionType, replacing it with a FunctionType
instruction. The constant for a FunctionDecl is now a StructValue with
type of FunctionType.
Note this means a function declaration produces _both_ a type, and a
value of the type. This has some consequences in terms of circularity,
and makes the importing of function declarations a little more complex.
It'll get particularly peculiar for imports because of the behavior of
the reference, but that's a known issue due to other things such as
`alias`. The impact will hopefully be contained to
ResolvePrevInstForMerge (and ImportRefs).
To note a small formatting change in diagnostics:
```
- // CHECK:STDERR: fail_member_lookup.carbon:[[@LINE+4]]:3: ERROR: Value of type `<associated <function> in Interface>` is not callable.
+ // CHECK:STDERR: fail_member_lookup.carbon:[[@LINE+4]]:3: ERROR: Value of type `<associated F in Interface>` is not callable.
- // CHECK:STDERR: fail_todo_facet_lookup.carbon:[[@LINE+4]]:3: ERROR: Value of type `<associated <function> in Interface>` is not callable.
+ // CHECK:STDERR: fail_todo_facet_lookup.carbon:[[@LINE+4]]:3: ERROR: Value of type `<associated F in Interface>` is not callable.
```
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Nicholas Bishop
David Blaikie
Richard Smith
josh11bandJosh L
Geoff Romer
Ivana Ivanovska
Dana JansensandRichard Smith
