Just a small packing optimization. We currently have 222 `NodeKinds`, so
this reduces us to just 30ish more we can add without needing to pack
more. However, if we did, there would be a couple options for bringing
the count down by reusing `NodeKinds` and disambiguating based on the
token kind (the 29 infix operators as an example). Or we could just undo
this.
I'm expecting this to yield a small improvement. I'll see if I can get
better numbers since my machine's not really reliable, but here are some
basic values.
Also suggesting to draw the use of `::RawEnumType` for `TokenKind`,
since bit packing appears to work without it. Hoping the `static_assert`
is easier for people to understand the size of the field.
With the change:
```
----------------------------------------------------------------------------------------------------------------------------
Benchmark Time CPU Iterations Bytes Lines Tokens
----------------------------------------------------------------------------------------------------------------------------
BM_CompileAPIFileDenseDecls<Phase::Parse>/256 50399 ns 50359 ns 14336 104.588M/s 3.87217M/s 21.8629M/s
BM_CompileAPIFileDenseDecls<Phase::Parse>/1024 237823 ns 237629 ns 3072 136.721M/s 4.11986M/s 24.2058M/s
BM_CompileAPIFileDenseDecls<Phase::Parse>/4096 997645 ns 996771 ns 768 142.343M/s 4.04105M/s 23.9363M/s
BM_CompileAPIFileDenseDecls<Phase::Parse>/16384 4020308 ns 4018319 ns 192 152.041M/s 4.05966M/s 24.0874M/s
BM_CompileAPIFileDenseDecls<Phase::Parse>/65536 16691390 ns 16683058 ns 48 151.317M/s 3.92374M/s 23.2936M/s
BM_CompileAPIFileDenseDecls<Phase::Parse>/262144 75265735 ns 75233476 ns 8 135.842M/s 3.48421M/s 20.6862M/s
```
Without the change:
```
----------------------------------------------------------------------------------------------------------------------------
Benchmark Time CPU Iterations Bytes Lines Tokens
----------------------------------------------------------------------------------------------------------------------------
BM_CompileAPIFileDenseDecls<Phase::Parse>/256 51515 ns 51480 ns 13312 102.312M/s 3.78789M/s 21.387M/s
BM_CompileAPIFileDenseDecls<Phase::Parse>/1024 241040 ns 240900 ns 3072 134.865M/s 4.06392M/s 23.8771M/s
BM_CompileAPIFileDenseDecls<Phase::Parse>/4096 985593 ns 984657 ns 768 144.094M/s 4.09077M/s 24.2308M/s
BM_CompileAPIFileDenseDecls<Phase::Parse>/16384 4109327 ns 4105496 ns 192 148.813M/s 3.97345M/s 23.576M/s
BM_CompileAPIFileDenseDecls<Phase::Parse>/65536 17459655 ns 17446006 ns 48 144.7M/s 3.75215M/s 22.275M/s
BM_CompileAPIFileDenseDecls<Phase::Parse>/262144 80802815 ns 80737489 ns 8 126.581M/s 3.24668M/s 19.276M/s
```
---------
Co-authored-by: Chandler Carruth <chandlerc@gmail.com>
Non-entry-block allocas will allocate new stack memory each time they're
reached, resulting in leaking stack memory over time for allocas in a
loop. Move all such allocas to the entry block instead, and use an LLVM
intrinsic to mark when the lifetime of the variable actually begins.
In order to support these examples, this adds two new builtins to the
toolchain: `print.char` and `read.char`, which map to the libc functions
`putchar` and `getchar`.
When substituting into a generic in order to form a generic eval block,
we form `SpecificId`s to track the list of arguments that should
eventually be used to form a specific referenced by the eval block.
Values within that specific are not needed and won't ever be used, so
it's safe to skip forming them in the first place.
Co-authored-by: Josh L <josh11b@users.noreply.github.com>
I was looking at this again, considering how best to add new checks, and
realized we could just change the format and probably get better deltas
in the future.
This change should just be formatting, with no functional impact.
Instead of treating `Core.Int` as the toolchain's builtin `IntType`,
model it as a class that adapts the builtin type. This aligns us better
with the intended language model, gives an associated library for
`impl`s involving `Core.Int` to live within, and opens the door adding
member functions to `Core.Int` if we decide that is desirable.
Remarkably it also seems to make the formatted SemIR a little smaller,
because a call to a generic class generates less IR than a call to a
function.
Such indexing operations are created by array initialization. Since we
switched integer literals to be of type IntLiteral we've been attempting
to index arrays with the (empty) representation of an IntLiteral rather
than with an actual integer value.
Given code like the following:
```
auto kind = ConversionTarget::Kind{0};
CARBON_CHECK(!loc_id.is_valid(), "hello {0} world", kind);
```
Currently we would print 'hello <the next line>', as the check string
would be treated as terminating at the '{0}', so it does not print the
rest of the string or a newline. This is because ConversionTarget::Kind
is an enum with underlying type `int8_t` which is a char, and
llvm::formatv does not look if the type is an enum and treat is
specially. So it prints it as a char rather than a number, which in this
case is a nul terminator.
With this change, the '{0}' value will be converted to a larger integer
before being passed through to llvm::formatv so that char-sized enums
will print as a number, and the result is that we will print 'hello 0
world\n' as the developer intended.
- Provide `Check::Dump(context, arg)` and similar.
- gdb and lldb should do contextual lookup, and `call Dump(*this,
Lex::TokenIndex::Invalid)` has been tested with gdb.
- Since this is only for debug, keeps the functions fully separated from
code.
- Uses alwayslink to ensure objects are correctly linked, even though
there are no calls.
- `-Wno-missing-prototypes` is needed when we don't have forward
declarations.
- Code is not linked in opt builds, using `#ifndef NDEBUG`.
- This probably could be doing something in BUILD files with a
`select()`, but the `#ifndef` seemed easier.
This is based on #4620, but uses free functions instead of member
functions.
Co-authored-by: Dana Jansens <danakj@orodu.net>
---------
Co-authored-by: danakj <danakj@orodu.net>
Almost all callers actually could never fail and nearly all of those
already `CHECK`-failed on failure. Add a new overload for that case, and
add a location parameter for the one remaining call.
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.
This is essentially the result of looking at `.begin()` uses. We also
frequently do `std::shuffle`, but unfortunately STLExtras doesn't
provide a wrapper for that.
For example, format the `ImplicitAs` interface as `Core.ImplicitAs`
rather than simply `ImplicitAs`.
When importing an entity in a namespace, also import a declaration of
the enclosing namespace if necessary so that we can determine its name.
Proposes a set of core features for declaring and implementing generic
variadic
functions.
A "pack expansion" is a syntactic unit beginning with `...`, which is a
kind of
compile-time loop over sequences called "packs". Packs are initialized
and
referred to using "pack bindings", which are marked with the `each`
keyword at
the point of declaration and the point of use.
The syntax and behavior of a pack expansion depends on its context, and
in some
cases by a keyword following the `...`:
- In a tuple literal expression (such as a function call argument list),
`...`
iteratively evaluates its operand expression, and treats the values as
successive elements of the tuple.
- `...and` and `...or` iteratively evaluate a boolean expression,
combining
the values using `and` and `or`, and ending the loop early if the
underlying
operator short-circuits.
- In a statement context, `...` iteratively executes a statement.
- In a tuple literal pattern (such as a function parameter list), `...`
iteratively matches the elements of the scrutinee tuple. In conjunction
with
pack bindings, this enables functions to take an arbitrary number of
arguments.
---------
Co-authored-by: josh11b <josh11b@users.noreply.github.com>
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Co-authored-by: josh11b <15258583+josh11b@users.noreply.github.com>
Co-authored-by: Chandler Carruth <chandlerc@gmail.com>
Co-authored-by: Carbon Infra Bot <carbon-external-infra@google.com>
This is primarily moving code around, to try to create a logical split
of the code in check.cpp, makingthe API boundaries clearer.
There's one small, deliberate logic change around false returns from
`HandleParseNode`, where before there was a `CARBON_CHECK` instantiated
by the `#define` (per `NodeKind`), and now it's outside the `#define`
(done mainly because the message didn't keep up with the `Handle##Name`
-> `HandleParseNode` rename).
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.
This is a preparation change for adding name poisoning support
(https://github.com/carbon-language/carbon-lang/issues/4622), which is
expected to require more elaborate logic around NameScope since a name
can be not defined yet, defined, or poisoned.
The API separates looking up a name from getting the full entry since we
have cases where the entries are invalidated between the time we're
looking for the name and when we access (and sometimes modify) the
entry.
This change has the following benefits:
* `names` and `name_map` are internal to `NameScope` and are guaranteed
to match.
* `extended_scopes` and `import_ir_scopes` can not be manipulated (only
new scopes can be added).
* `inst_id`, `name_id` and `parent_scope_id` are constants.
* `has_error` can only be mutated from false to true.
---------
Co-authored-by: jonmeow <jperkins@google.com>
The offsets were originally added to deal with churn from builtins in
the raw semir. In textual semir, we mostly see instruction IDs for
imports, and builtins have also settled down more.
On imports, where possible, use the `EntityNameId` for an import instead
of printing an instruction. Next, show the source location if we have a
node. Only show the instruction if there's no location.
This also exposes `Parse::Tree` and `TokenizedBuffer`, so that we can
pass a `SemIR::File` without the component parts. In particular this
allows us to get the `TokenizedBuffer` for import IRs without
substantial structural modifications. We may want to make these optional
for serialized `SemIR` later, but the nodes/tokens contain source
location, which we'd need for debug information -- so it's not clear how
much we can really make them optional without substantial information
loss.
Reduce arguments to just `File` in a few spots, as a result of the
accompanying `TokenizedBuffer` and `Parse::Tree`. Also updates style to
pass around `const File*` where the reference is maintained, instead of
`const File&`.
I was considering keeping a direct reference to the tree and tokens on
`Context`, but initially my thought was it wouldn't make much
difference. I can re-add those if desired, just as direct caching of the
`File` fields.
This fixes a crash in lowering, at least (though only initializes the
vptr to
null for now) - certainly open to naming feedback on the instruction, or
the
exact semantics (we could have a global vptr instruction that's
referenced from
the existing instructions for reading globals, for instance).
I guess we'll want one type parameter for the vptr_init instruction,
which is
the type that this is a vptr for? (can do that here or in a follow-on
patch)
Actually presenting two options in this one review - if you look at the
specific
commits in this PR, the first commit represents my first attempt - and
if you
look at the overall PR change for the second attempt.
But I'm totally open to completely different approaches/ideas - these
were just
my rough guesses.
Extends the set of function signatures that support being given a
builtin definition to include cases where a parameter or return type is
an adapter for a supported type. For example, if we can give a builtin
definition to `Add(a: i32, b: i32) -> i32`, then we can also give a
builtin definition to `Add(a: MyI32, b: MyI32) - >MyI32` where `MyI32`
adapts `i32`.
This is a prerequisite for changing `Core.Int` to be a class type that
adapts the builtin int type.
Also make `-gsimple-template-names` lldb-only due to it tripping up gdb
in some cases (I came across it breaking SmallVector pretty printing
where gdb wouldn't associate a simplified type named declaration with a
simplified type named definition in another translation unit - seems gdb
can associate a type decl/def when it sees both (if you step into both
translation units or otherwise trigger gdb loading/parsing them) but it
doesn't seem able to /search/ for the type definition). Filed
https://sourceware.org/bugzilla/show_bug.cgi?id=32421 for this.
A couple of other things I'd like to do, but don't know how:
* It might be nice to allow opting into or out of gdb_index (with the
default being 'on' for gdb_flags, but you could opt out). But doesn't
seem super important.
* We should turn off fission by default, it seems - bazel has trouble
making the .dwo files available at the same path as is in the binary
especially on partial rebuilds. (not sure if we can do that, I guess
we can make fission a no-op/doesn't add any flags, even if we can't
change the fission default in bazel itself)
* can we have gdb_flags imply/disable lldb_flags? (so you can use
--features=gdb_flags without always having to add
--features=-lldb_flags)
This slightly improves the handling of adapters, by making them copyable
in some cases when their adapted type is copyable.
Refactor some of the repeated checks for properties of value
representations.
In passing, move all the adapter tests in check/testdata/class to a
subdirectory since we now have quite a few of them.
I believe our flags enable the warning by default, it's just that it
doesn't catch this in clang-16 (maybe more; I reproduced with clang-18
and didn't keep digging). For example:
```
toolchain/parse/tree_test.cpp:86:28: error: ISO C++20 considers use of overloaded operator '==' (with operand types 'value_type' (aka 'Carbon::Parse::NodeIdInCategory<Carbon::Parse::NodeCategory::Decl>') and 'AnyDeclId' (aka 'NodeIdInCategory<NodeCategory::Decl>')) to be ambiguous despite there being a unique best viable function [-Werror,-Wambiguous-reversed-operator]
86 | EXPECT_TRUE(*any_decl_id == any_decl_id2);
| ~~~~~~~~~~~~ ^ ~~~~~~~~~~~~
```
The different `operator==` approach works except for with
`Parse::NodeId::Invalid`, which seems easy to replace with a
`.is_valid()` check.
Jon Ross-PerkinsandChandler Carruth
Richard Smith
josh11bandJosh L
Dana Jansens
Boaz Brickner
David Blaikie