I admit I'm tempted to make a MODULE.bazel.md for these comments, so
that modifying them doesn't trigger a lockfile update, but I don't
really want it at the top level.
The caching isn't buying us much, and is adding complexity and
divergence between the codepaths for generic and non-generic functions.
This means we no longer suppress diagnostics for the second or
subsequent time we call a function with an incomplete return type. If we
want to add that back, it might be worth considering moving the
suppression to `TryToCompleteType` and only diagnosing that a type is
incomplete once, regardless of why we're requiring it to be complete.
As discussed in toolchain meeting, we want to avoid overloading the
meaning of "instance", and "specific" was the best name we found. It's a
little unorthodox and inventive, but hopefully over time will become as
unsurprising as the term "generic" is.
I'd considered moving DeclParams uses over, but when handling qualified
names, there's a parse node instead of an instruction. I did try to
unify a couple other uses though, including adding MergeDefinition. I
expect `interface` will use a little more once it's more completely
implemented, but maybe I'm wrong about that.
This implements a few closely related features:
- Starts merging namespaces discovered inside imports.
- Stores results of cross-package name lookup as an entry inside the
scope.
- Note this is particularly visible with `i32`.
- Moves more of the imported instructions to the import scope.
Note this is primarily for executing the namespace TODO in check.cpp,
which is removed here.
`testdata/namespace/merging_with_indirections.carbon` tests key
behavior.
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
This provides an example of downloading and using the nightly buildings
of the toolchain. I've tried to provide appropriate caveats about the
fact that this is very early and not something that's really reliable.
But I wanted folks to know how they can play with the nightly releases
if they're interested.
I've also focused the source build instructions on the toolchain where
we'd be interested in contributions, and the first paragraph on trying
out Carbon in the browser with CE.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
When evaluating within the context of a specific, we can encounter uses
of bindings that are nested within that specific, for example parts of
the declaration of a nested generic. Those bindings should evaluate to
the canonical form of themselves, as they would when evaluating outside
the context of the specific.
Fixes#4157.
Instead of reusing instructions from the generic entity in the eval
block, rebuild constants in the same way we rebuild types. The previous
attempt to not rebuild these constants assumed that every constant used
in a generic would be built in that generic, and not referenced directly
or referenced from some enclosing scope, which isn't true in practice
and is a fragile assumption in any case.
We could add back some reuse of instructions from the generic -- if we
happen to see the right instruction to build a constant, we could
opportunistically reuse it -- but given the complexity added by doing
so, I'm not pursuing that here.
Now that the eval block for a generic consists of instructions uniquely
owned by that generic, rather than often being shared with another
entity, include the generic in the formatted SemIR output. I'm using the
same scope name for the generic object itself as for the parameterized
class / function / interface, because there are very frequently
references between them and this keeps the IR simpler and more readable,
and avoids needing to invent a second name for the scope.
Note this isn't implementing checking through imports. The parse node
there is harder to access through the context, so would require
examining the entity in order to get the import declaration, to get at
the ImportIR. We also don't have a parse tree attached in that case, and
would need to add one to SemIR::File. But I believe we do want to add
that, so it's explicitly a TODO.
Note GetTokenText re-lexes literal values, so there's a bit of potential
overhead there. Not sure if we want a more efficient manner for
comparing in cases like this.
Also adds import_ir_scope to namespace formatting. I'd done this as an
aid for #4153, and am splitting it out.
---------
Co-authored-by: Geoff Romer <gromer@google.com>
At present, I think if we crash from multiple threads in parallel, it
can lead to the stack trace not being printed out. Using
CrashRecoveryContext here seems to more successfully print a stack trace
on errors, which I'm hoping will ease debugging.
i.e., before:
```
-----------------------------------------------------------------------------
.Please report issues to https://github.com/carbon-language/carbon-lang/issues and include the crash backtrace.
Stack dump:
0. performing autoupdate for toolchain/check/testdata/alias/no_prelude/import_order.carbon
1. Program arguments: compile --phase=check --dump-sem-ir --no-prelude-import --exclude-dump-file-prefix=/usr/local/google/home/jperkins/.cache/bazel/_bazel_jper.kins/85deb7d9d96f7e0e80b42618a55969d7/execroot/_main/bazel-out/k8-fastbuild/b.in/toolchain/install/prefix_root/lib/carbon/../../lib/carbon/core a.carbon b.carbon
.CHECK failure at ./toolchain/sem_ir/ids.h:140: is_valid()
CHECK failure at ./toolchain/sem_ir/ids.h:140: is_valid()
external/bazel_tools/tools/test/test-setup.sh: line 328: 1151859 Aborted "${TEST_PATH}" "$@" 2>&1
```
(EOF)
after:
```
-----------------------------------------------------------------------------
Please report issues to https://github.com/carbon-language/carbon-lang/issues and include the crash backtrace.
Stack dump:
0. performing autoupdate for toolchain/check/testdata/alias/no_prelude/import_order.carbon
1. Program arguments: compile --phase=check --dump-sem-ir --no-prelude-import --exclude-dump-file-prefix=/usr/local/google/home/jperkins/.cache/bazel/_bazel_jperkins/85deb7d9d96f7e0e80b42618a55969d7/execroot/_main/bazel-out/k8-fastbuild/bin/toolchain/install/prefix_root/lib/carbon/../../lib/carbon/core a.carbon b.carbon
..CHECK failure at ./toolchain/sem_ir/ids.h:140: is_valid()
CHECK failure at ./toolchain/sem_ir/ids.h:140: is_valid()
......CHECK failure at ./toolchain/sem_ir/ids.h:140: is_valid()
CHECK failure at ./toolchain/sem_ir/ids.h:140: is_valid()
.....................................................................................CHECK failure at ./toolchain/sem_ir/ids.h:140: is_valid()
.CHECK failure at ./toolchain/sem_ir/ids.h:140: is_valid()
............................ #0 0x000056045ee22d7d llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/usr/local/google/home/jperkins/.cache/bazel/_bazel_jperkins/85deb7d9d96f7e0e80b42618a55969d7/execroot/_main/bazel-out/k8-fastbuild/bin/toolchain/testing/file_test.runfiles/_main/toolchain/testing/file_test+0x731dd7d)
```
(elided the full stack trace)
This uses a heuristic reserve to greatly reduce hashtable growth of the
identifiers hashtable. The design of the hashtable itself is optimized
around compact memory use and is especially slow to grow and so this has
an outsized impact.
The heuristic was computed using `scripts/source_stats.py` and looking
at C++ codebases. We may want to periodically re-evaluate it as Carbon
code emerges and we have better data on its distributions of tokens.
This also required fixing the `Reserve` method on `CanonicalValueStore`
that wasn't actually used anywhere and so didn't even compile correctly.
I added it to the relevant unit test so it is at least compiled locally
to its definition.
This better follows the principle that types are simply constants of
type `type`, and allows more uniform treatment of types as just another
kind of constant from generics handling.
Use a hash table to map from `TypeId` to information about the complete
type. This makes basic operations on types a bit simpler, and operations
that actually need to access the complete class information a bit more
complex.
Changes crash messages to start printing verbose forms of instructions,
rather than just the ID. Fixes some indentation issues with stacks. Also
switches unexpected inst formatting, because now there are lots, and
it'd be helpful to know where they are.
This uses a pimpl pattern for Formatter due to the number of member
functions on Formatter. Maybe we should refactor that, but this didn't
feel like a good place to do so.
Note, I have two concerns about this change... to note them here, to
make sure others are considering them when evaluating the
implementation:
1. Some instructions are very verbose to print, as evidenced by the
fn_decl printing (which includes function params) or scope printing
(which includes scope members).
- I'm not sure whether there's a way to simply reduce this, as it seems
essential to the requested printing of instructions.
- Long-term, we may at least want to limit the number of lines printed
here. However, I've already spent a fair amount of time here and I think
it's in a good state to evaluate.
2. Increased complexity in the crash handler may result in crash
messages failing to generate.
- For example, a crash in Formatter (and its deps, such as InstNamer or
location handling) prevents a stack from being printed. I'm pretty sure
I've written crashes in Formatter before.
Here's an example crash snippet (generated by adding a crash inside
`return` handling) before:
```
2. NodeStack:
0. FunctionDefinitionStart -> function2
1. ReturnStatementStart -> no value
2. IntLiteral -> inst+26
inst_block_stack_:
0. block<invalid> {inst+0, inst+1, inst+2, inst+23}
1. block9 {inst+26}
param_and_arg_refs_stack:
args_type_info_stack_:
```
And after:
```
2. Check::Context
NodeStack:
0. FunctionDefinitionStart: function2
1. ReturnStatementStart: no value
2. IntLiteral:
unexpected.inst+26.loc12_10: i32 = int_literal 0 [template = constants.%.2]
inst_block_stack_:
0. block<invalid> {
package: <namespace> = namespace [template] {
.Core = unexpected.inst+2
.F = unexpected.inst+23.loc11_22
}
unexpected.inst+1 = import Core
unexpected.inst+2: <namespace> = namespace unexpected.inst+1, [template] {}
unexpected.inst+23.loc11_22: %F.type = fn_decl @F [template = constants.%F] {
unexpected.inst+9.loc11_9: init type = call constants.%Bool() [template = bool]
unexpected.inst+10.loc11_9: type = value_of_initializer unexpected.inst+9.loc11_9 [template = bool]
unexpected.inst+11.loc11_9: type = converted unexpected.inst+9.loc11_9, unexpected.inst+10.loc11_9 [template = bool]
unexpected.inst+12.loc11_6: bool = param b
@F.%b: bool = bind_name b, unexpected.inst+12.loc11_6
unexpected.inst+19.loc11_18: init type = call constants.%Int32() [template = i32]
unexpected.inst+20.loc11_18: type = value_of_initializer unexpected.inst+19.loc11_18 [template = i32]
unexpected.inst+21.loc11_18: type = converted unexpected.inst+19.loc11_18, unexpected.inst+20.loc11_18 [template = i32]
@F.%return: ref i32 = var <return slot>
}
}
1. block9 {
unexpected.inst+26.loc12_10: i32 = int_literal 0 [template = constants.%.2]
}
param_and_arg_refs_stack:
args_type_info_stack_:
```
This improve the toolchain's performance by about 10%.
It will also allow us to leverage TCMalloc's extensions to do heap
profiling and get other information about how efficiently we're using
the heap.
Note that currently this causes all of our builds to produce a warning
due to an issue with `rules_python` and multiple modules registering
python toolchains:
https://github.com/bazelbuild/rules_python/issues/1818
This is also only enabled on Linux as there is no support for other OSes
at the moment.
Most of this is enabled by default, but there is some that needs an
explicit flag.
Avoiding `-Wnon-virtual-dtor` itself for now because that warning can
require changes that carry overhead such as having an extra, unused
destructor entry in the vtable. Hopefully we don't have too many folks
who need our code to be `-Wnon-virtual-dtor` clean.
Querying a local constant with an invalid instruction triggers a crash,
this will prevent self_param_id to be used unless it is defined.
Closes#4071 and #4080
This will only fix the crash. The scenario where a declaration-only
interface is imported then defined still gives an error message, but it
won't crash this time.
When evaluating in a generic context, a constant with a symbolic type
might evaluate to a constant with a concrete type (or a more specific
symbolic type). This can't actually happen yet given the current state
of the toolchain, as far as I can determine, so this is more just a
refactoring for now, but will be relied upon by future generics work.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
Co-authored-by: Carbon Infra Bot <carbon-external-infra@google.com>
When forming a specific (previously called a generic instance), evaluate
the eval block of the generic to determine the values of any constants
used in that specific. The majority of the work here is updating
eval.cpp so that it can use the results of prior evaluations in the same
block when computing later values.
Include the computed results in the formatted SemIR output.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
I was considering making a "/bazel" alias for this, but I'm really on
the fence about whether that's a good choice. However, I do think this
is good to have to simplify installs.
Fixes#3071 and #3896
Proposal for how Carbon version numbers work:
- A single version across language, standard library, compiler, linker,
etc.
- Semantic Versioning (SemVer) based
- Details of how SemVer criteria for major, minor, and patch should
apply to
Carbon
- Details of how we will operate before 1.0 and how this connects to
Carbon's
milestones
- Directional guidance for future work including post-1.0 versions, LTS
versions, and standardization
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
This is for consistency with #4120. Similar to that, we can use
overloads on the typed NodeId rather than individually named handlers.
There isn't the same caller benefit here though, since the calls from
check.cpp are already boilerplate.
Adds scripts/calculate_release_shas.py to print the versions, and
updates tool versions. Consolidates target-determinator logic with the
other tool logic.
This is also intended to make it easier to add more tools.
Also makes an alias so that this is easier to find. Verified that
running the alias still finds prelude files.
Note the actual target is printed when building (although a symlink is
also created, using that symlink confuses file-finding).
```
╚╡bazel build :carbon
...
Target //toolchain/install:prefix_root/bin/carbon up-to-date:
bazel-bin/toolchain/install/prefix_root/bin/carbon
```
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>
Renames Lower::Handle* to Lower::LowerFunctionInst. This allows writing
a templated handler for instructions, moving code out of the macro
expansion and removing some of the redundancy in things like
`HandleAddrOf(..., SemIR::AddrOf inst)`
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.
This is to remove all the FatalIfEncountered handlers in handle.cpp.
They just feel like noise when reading the file. Plus it's one less bit
of boilerplate to add for instructions that don't lower.
Note that I left HandleParam/HandleAddrPattern. I'd be happy to change
those to just set lowered=false too, but was hesitant to given the
separate logic.
Also, I'm separately considering migrating the macro logic into similar
constexpr things. If I do, I might switch Define to take in a struct.
But for how this particular parameter works, the overload felt
reasonable, particularly since is_lowered is not used in combination
with TerminatorKind.
I'm trying to increase the distinction between BuiltinKind and
BuiltinFunctionKind. BuiltinKind is for instructions,
BuiltinFunctionKind is for function definitions. To get to this point,
I'm doing a few changes:
- BuiltinKind -> BuiltinInstKind
- builtin_kind.* -> builtin_inst_kind.*: filename consistency
- Builtin -> BuiltinInst: mainly for consistency with the above
- Builtin::builtin_kind -> BuiltinInst::builtin_inst_kind: somewhat
repetitive but seems like a consistent edit
- Function::builtin_kind -> Function::builtin_function_kind: seems a
useful distinction
I'm leaving alone things like (and mentioning in case there's a desire
for more renames):
- InstId::BuiltinError, InstId::ForBuiltin: these I think are more
apparent because they're directly associated with Inst.
- GetBuiltinICmpPredicate in lowering: maybe builtin function handling
should be in its own file, but these local names don't feel problematic
to me.
- GetBuiltinType, BuildBuiltinValueRepr, PerformBuiltinIntComparison:
similar to the above, names don't feel too problematic
Require types into which qualified lookup is performed to be completely
defined. Eventually this will trigger substitution into the definition
for generic types.
Jon Ross-Perkins
Richard Smith
Brymer Meneses
Chandler CarruthandJon Ross-Perkins
Gıyaseddin Tanrıkulu
