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>
As described in symlink_helpers.bzl, copied here for visibility:
Symlinking busybox things needs special logic.
This is because Bazel doesn't cache the actual symlink, resulting in
essentially resolved symlinks being produced in place of the expected
tool. As a consequence, we can't rely on the symlink name when dealing
with busybox entries.
An example repro of this using a local build cache is:
bazel build //toolchain
bazel clean
bazel build //toolchain
We could in theory get reasonable behavior with
`ctx.actions.declare_symlink`, but that's disallowed in our `.bazelrc`
for cross-environment compatibility.
The particular approach here uses the Python script as a launching pad
so that the busybox still receives an appropriate location in argv[0],
allowing it to find other files in the lib directory. Arguments are
inserted to get equivalent behavior as if symlink resolution had
occurred.
The underlying bug is noted at:
https://github.com/bazelbuild/bazel/issues/23620
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.
Undoes a chunk of #4125 because nobody's really in favor of keeping the
formatting, and it's occasionally caused a crash in Formatter to
dominate output (and even when working, it can be verbose; the source
location in (5) is often more helpful).
Basically goes back to:
```
4. Check::Context
NodeStack:
0. LetIntroducer: no value
1. BindingPattern: inst+15
2. LetInitializer: no value
3. StructLiteralStart: no value
inst_block_stack_:
0. block<invalid> {inst+0, inst+1, inst+6, inst+7, inst+8, inst+9, inst+10, inst+11, inst+12}
1. global_init {}
pattern_block_stack_:
0. block<invalid> {}
param_and_arg_refs_stack:
0. block<invalid> {}
args_type_info_stack_:
0. block<invalid> {}
5. alias_of_alias.carbon:15:12: checking StructLiteral
let d: c = {};
^~
```
Fixes#4145
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.
While we don't need a lookup table for an imported impl from a different
library, we do still need to import the name scope so we can compute the
parent scope for mangling purposes.
Also use the `Impl` base class to type-erase the small size of
`SmallVector`. (I'd like to do the same for `Map` and `Set` by using
`MapView` and `SetView`, but that runs into ambiguities due to
`BumpPtrAllocator`'s unconstrained converting constructor.)
Previously Collect() was used for types that implemented
CollectMemUsage() but otherwise Add() was used. This required the caller
to think about the type of the field and know/decide which method to
use.
Now, the caller always uses Collect() unless they are adding specific
byte values, in which case Add is used. Typically then, Add will only be
used to implement the CollectMemUsage() function.
To do this we require all Collect() methods to be templates so that they
all be a single overload set. The Collect on BumpPtrAllocator is
converted to a template that checks
`std::same_as<llvm::BumpPtrAllocator, T>`.
This avoids producing an LSan leak report for the objects that got
leaked by the crash, which would otherwise scroll all the useful
information about the crash off the terminal.
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>
I'm seeing three issues with CalleeParamsInfo:
1. Although ResolveCalleeInCall had been extracted out and the
EntityWithParamsBase could be used directly, that had not been cleaned
up.
2. implicit_param_refs_id is unused; param_refs_id was only used by
ResolveCalleeInCall. The factoring as a struct seems to be obscuring
what's used and what isn't (creating unnecessary copies).
3. On #4446, CalleeParamsInfo seemed to be obfuscating what
ConvertCallArgs actually worked with (a Function, not a generic entity).
I'm thinking that just removing CalleeParamsInfo is the best resolution
here, it looks like it's tripping people up more than it's helping.
Note, I think Function used to be named Callable, which was where the
"callable" name originally came from (I might be wrong about this). But
"function" seems clearer about the type now.
Split StructComma into StructLiteralComma and StructTypeLiteralComma in
order to easily differentiate handling (remains the same in this PR).
Add "Literal" to StructField and StructTypeField because it feels
inconsistent versus the other non-shared things. StructFieldDesignator
remains shared between value literals and type literals.
Note I probably would've made StructFieldDesignator non-shared too, but
that'd require either a lookahead of 2 (to see the separator`) or a
writeback after parsing the separator, neither of which felt especially
crucial for this, when what I'm really trying to do is split type
literal handling a little further.
Came up due to [libc++ install
issues](https://discord.com/channels/655572317891461132/655577725347561492/1302023663155023905)
We've discussed clang version verification, and adding that as long as
I'm in here. The more significant bit is the libc++ check, which if it's
not installed should fail like:
```
(tons of output)
ignoring nonexistent directory "/include"
#include "..." search starts here:
#include <...> search starts here:
/usr/lib/llvm-16/lib/clang/16/include
/usr/local/include
/usr/include/x86_64-linux-gnu
/usr/include
End of search list.
/usr/local/google/home/jperkins/.cache/bazel/_bazel_jperkins/85deb7d9d96f7e0e80b42618a55969d7/external/_main~clang_toolchain_extension~bazel_cc_toolchain/_temp:6:2: error: "No libc++ install found!"
#error "No libc++ install found!"
^
1 error generated.
ERROR: Analysis of target '//toolchain:toolchain' failed; build aborted: Analysis failed
INFO: Elapsed time: 0.265s, Critical Path: 0.08s
INFO: 1 process: 1 internal.
ERROR: Build did NOT complete successfully
```
pre-commit runs bazel, and GitHub runners have an old clang by default
(caught by the new check), so I'm installing here for a consistent
version.
---------
Co-authored-by: Chandler Carruth <chandlerc@gmail.com>
This is in anticipation of making the integer value store be customized
heavily. I'd like to extract it from the common code when doing that, so
first disentangling them here without any intended change in
functionality or behavior to enable that.
I've tried to update `#include`s to be as minimal as I can and added a
few missing includes spotted in the process.
I've split the test for value store to include what was easy focused on
just the value store templates rather than the unified shared value
stores.
This might surface some opportunities for adding more tests, but for
this PR, just doing the minimal restructuring.
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.
Removes the separate language server binary; I'm not sure we need to
provide it. Instead, `carbon language-server` is added as a subcommand.
Moves //language_server to //toolchain/language_server. Splits into a
trivial language_server.h, and a substantive server.h. I wasn't sure
about a better name, but wanted the split similar to check/check.h,
lex/lex.h, etc. At the same time, the class is probably going to be a
little big so not a good fit to through into just a cpp file.
This fixes some style issues with the language server class, but
generally I'm trying to not address things here in order to keep it
simpler.
This teaches the driver library to track when its being used with
fuzzing and disables the `clang` subcommand from actually running Clang.
The Clang libraries have a large backlog of fuzzer-found issues that
isn't being actively reduced, so we can't productively fuzz into it.
This lets us more productively fuzz at the top level.
This is also available on the command line itself, which should be
useful if anyone wants to fuzz Carbon from the command line using tools
like AFL -- they can inject this flag to avoid getting noise from the
fuzzer hitting known issues in Clang.
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.
In this case, the callee may be non-constant because it includes a
reference to `self`, so we need to be able to lower a non-constant
`specific_function`.
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`.
Richard Smith
josh11bandJosh L
Jon Ross-Perkins
David Blaikie
Dana Jansens
Sam Estep
Dana Jansens
