This undoes parts of #3515 in order to allow PushGlobalInit to be called
when the initializer is called, instead of at the end of the binding
pattern. The current approach is fragile because supported patterns will
become more complex. We also will likely want similar support in `let`,
which puts the initializer first, so this offers a consistent approach
for both.
[Looking
back](https://discord.com/channels/655572317891461132/655578254970716160/1184237511766179840),
this is more or less the second option in that message, but using the
PeekNextIs to avoid vagueness about what's being popped first.
Note I'm putting in PeekNextIs for what I'm hoping will be a pretty
narrow use-case. I could've added depth arguments to the Peek functions,
but that would've rippled through a number of APIs and it's not clear to
me that this has generic utility. I mean, right now it could just be
PeekNextIsVariableInitializer, since it's only optional in that case.
This works by creating a faux FunctionDecl in the context of the current
IR, which seems to be working for function calls. Deduced params are
there, but won't really be tested until classes are up and running. Also
I may need to look further at return_slot_id to ensure it's working. But
the basics, I think, are here.
Reorganizes some other ImportRef work from `has_unresolved` that'd
relied on manual calls to a more detection-based `HasUnresolved`
approach that doesn't require as much checking.
Unqualified names don't handle scopes the way that typical names do, so
a name conflict with a namespace needs to be handled specially. I'm
still favoring keeping code close as much as possible, particularly
since long-term this syntax will probably shift to be more consistent.
For now I'm just flagging when we shouldn't push scopes, so that
MakeUnqualifiedName doesn't need to clean up.
Note, a different approach would basically be:
```
PushScopeAndStartName
ApplyNameQualifierTo
result = decl_name_stack_.back();
decl_name_stack_.back().state = NameContext::State::Finished;
PopScope
return result;
```
But that approach feels worse to me, due to the additional stack
manipulations and the need to duplicate some of the FinishName logic
just to be able to pop the scope that didn't really need to be added.
Adds `BindAlias` with a hybrid of `BindName` and `NameRef` semantics. I
think it's slightly closer to `BindName` because it introduces a name,
so I'm going more in that direction. This also matches the need for
`bind_name_id` with imports on enclosing scopes.
Note, only things that look like a name reference are being allowed on
the RHS of `alias`. This includes builtins that look like name
references, such as `bool`, but not ones that turn into values
underneath, such as `false`.
Similar to #3705, we actually have a mix of `Make` and `Create` in
factory functions too, so this PR is normalizing on `Make`. It's
intended to be consistent with the naming choice for Carbon factory
functions.
Note, MakeSyntheticBlock is the only one I feel a little weird about
because llvm's own APIs use Create, and this is essentially wrapping
LLVM calls. But the flipside is it also feels like a vague line to draw,
when we also differ from LLVM coding style in other ways.
- File::StringifyTypeExpr now has a case that hits the ImportRefUsed
TODO, so implementing that. I think the `static` approach will be
helpful in ensuring there aren't access bugs, particularly when future
support is added.
- `let` wasn't adding to exports because it doesn't use
`decl_name_stack` the way `var` does. This now adds to exports, but we
might want to unify logic for issues such as this.
- BuildImportRefUsedValueRepr is now called for another inst kind, and
it seemed like calling back to BuildValueRepr was the best way to
resolve this. I don't *think* that's going to cause recursion.
When we always had a single file in the test file, we could run the
driver over it to see output. This adds the ability to do something
similar for multi-file tests. So for example in test output, there's
now:
```
[ RUN ] ToolchainFileTest.toolchain/check/testdata/class/fail_todo_import.carbon
To test this file alone, run:
bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/check/testdata/class/fail_todo_import.carbon
To view output, run:
bazel run //toolchain/testing:file_test --test_arg=--file_tests=toolchain/check/testdata/class/fail_todo_import.carbon --test_arg=--dump_output
[ OK ] ToolchainFileTest.toolchain/check/testdata/class/fail_todo_import.carbon (64 ms)
```
And here's what a run looks like:
```
$ bazel run //toolchain/testing:file_test --test_arg=--file_tests=toolchain/check/testdata/class/fail_todo_import.carbon --test_arg=--dump_output
(elided bazel output)
Executing tests from //toolchain/testing:file_test
-----------------------------------------------------------------------------
===============================================================================
= toolchain/check/testdata/class/fail_todo_import.carbon
===============================================================================
= stderr
===============================================================================
b.carbon: ERROR: Semantics TODO: `TryResolveImportRefUnused on ClassDecl`.
b.carbon: ERROR: Semantics TODO: `TryResolveImportRefUnused on ClassDecl`.
b.carbon: ERROR: Semantics TODO: `TryResolveInst on ClassType`.
b.carbon: ERROR: Semantics TODO: `TryResolveInst on ClassType`.
b.carbon:18:29: ERROR: Name `d_ref` not found.
var d: (ForwardDeclared,) = d_ref;
^~~~~
===============================================================================
= stdout
===============================================================================
--- a.carbon
(eliding semir output)
}
===============================================================================
= Exit with success: false
===============================================================================
Done!
```
This also switches from PrettyStackTraceFormat to
PrettyStackTraceString. This is partly so that we can share the produced
command (avoiding two different format strings corresponding to the same
value), but also it's in my mind to step away from
https://github.com/llvm/llvm-project/pull/77351.
The `command_library` provides functionality for end users to query
documentation and help information about commands through the command
line interface itself. At present, this is implemented as:
* A flag, `--help`, on every command
* A meta subcommand, `help`, on every command that has other subcommands
To match with common functionality in other command line interfaces,
this PR amends the `help` meta subcommand to accept an optional
positional string argument. This argument specifies which subcommand to
print help information for. When not specified, the present behavior is
maintained, printing the help information for its parent command.
A potentially unexpected consequence of this implementation is that you
can query for help on meta subcommands as well: `help help` is a valid
input which prints the help information about the `help` meta subcommand
(e.g., same for `version`). I don't see any harm in this behavior so I
chose not to explicitly prevent it, but I'm open to preventing it if
deemed unwanted. (I'm also happy to workshop the strings in this PR.)
Closes#3694.
#### Before
```
$ carbon help compile
```
```
ERROR: Found unexpected positional argument or subcommand: 'compile'
```
#### After
```
$ carbon help compile
```
```
Compile Carbon source code.
This subcommand runs the Carbon compiler over input source code, checking it for errors and producing the requested output.
Error messages are written to the standard error stream.
Different phases of the compiler can be selected to run, and intermediate state can be written to standard output as these phases progress.
Subcommand 'compile' usage:
carbon [-v] compile [OPTIONS] <FILE>...
...
```
Note we may also want to do this with NameId, maybe some other things,
but the TypeId use is pretty broad and repetitive -- I thought I'd start
with it first.
`GlobalInit` block is now static block within a `SemIR` which will be
used to emit initialization instructions for variables in the `Package`
scope.
inst_block_stack now has additional methods to handle `GlobalInit` block
separately, this block can be popped without being finalized allowing to
accumulate between all instances of variables.
At the end of the `check` phase, if this block is not empty , the
function `__global_init` will be added with this block being inserted
into it.
This block is pushed to `inst_block_scope` at the end `BindName`,
allowing instruction to be emitted into it, then popped at the semicolon
(VariableDecl).
This significantly changes the `SemIR` output, that's why this commit
updates a lot of the test cases.
This provides support for Const, Pointer, Struct, and Tuple types. It
does not cover Class, Function, or Interface which have their own Id and
are tracked slightly differently.
They don't have names, but using the DeclNameStack anyway keeps our
behavior more consistent, and keeps track of the enclosing name scope
and the prior state of the scope stack for us.
Depends on #3683.
Collect the contents of an `impl` into a scope, and start doing very
basic checking for `impl` declarations and definitions.
This change adds two new `Id` types to the set of type that `NodeStack`
supports -- `ImplId` and `NameScopeId`.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
- Rename `ScopeIndex` to `ScopeId`, because the order is not meaningful.
This avoids collisions with `SemIR::ScopeIndex`.
- Change `GetScopeFor` to use an `if constexpr` chain rather than
duplicating the numbering logic across multiple functions.
- Simplify `GetNameFor`, avoiding multiple identical overloads.
As requested in review of #3683.
Add a type representing a non-discriminated union of IDs. Refactor the
node stack to use it. Plus a few other refactorings aiming to clean up
and simplify the code. The overall goal here is that adding a new kind
of ID, node category, or instruction should only require changing one
place in the node stack rather than a bunch of different changes.
One minor functionality change: crash backtraces now use the correct
type for IDs when dumping the node stack rather than using `InstID`
printing for all but one case.
Improve the exposition of the design changes from #3646 to integrate
better into the overall description of member access design.
This fixes the incorrect description of the rules for `->` by instead
relying on the general rule that `->` is rewritten to use `*` and `.`
before any other processing is done, and generally makes
*integer-literal* names be less of a special case.
Right now, ConstantValueStore defaults to having unknown values use
NotConstant. This generally works for the current IR, but with imports
we're expecting sparse entries which are generally unknown -- and
distinguishing between NotConstant and simply unset would be helpful. As
a consequence, add Invalid.
We discussed whether to simply have ConstantValueStore default to
Invalid going forward, or to make the default flexible. The upside to
the former is consistency, the upside to the latter is that it should
result in fewer Sets when operating on the current IR (which will more
frequently have known non-constant values). This PR offers both
approaches in separate commits, but I somewhat lean towards the latter
for fewer array resizes.
Note, a totally different approach would be to use a different class
(not ConstantValueStore) for imported IRs -- then the default of Invalid
versus NotConstant would be type-dependent. However, I expect we're
going to want to do at least somewhat consistent lookups, and using the
same ConstantValueStore for both cases allows avoiding a virtual
interface or templating. Also, I'm hoping to only maintain the
ConstantValueStore for an imported IR as part of Context (not File),
which would mean the SmallVector storage overhead is ephemeral,
mitigating one of the potential advantages of using a different type for
imported IRs.
We already do this with things like llvm::DenseMapInfo, I don't know why
I was doing this with format_provider. But this should be more
consistent, and slightly better for not entering another library's
namespace.
Use two different nodes for "<type> followed by `as`" and "<type>
omitted before `as`, use `self`", so it is easier to determine which
case. Later the second case will push the type id for `self` onto the
node stack, making the two paths more similar.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
We had some amazing GSoC participants last year, but because Carbon is
still pretty small, we ended up stetched a bit too much to be
sustainable. And this year, we're trying to have an even narrower focus
on the toolchain.
Between these aspects, we sadly don't have the bandwidth to run an
effective GSoC project this year. We think it's really important that we
can give mentees an excellent experience on the project, and don't want
to overcommit ourselves, or worse, let them down.
We're really hopeful to be back when the project is a bit larger though,
and we have good bandwidth to host folks.
Add support for extracting elements of a tuple by their numerical index.
Also formally add the well-established basic syntactic and semantic
rules for
tuples, for which we have had leads issues but no proposal, into the
design.
Consume the components of the `impl` declaration, and set up scopes for
the child elements. We don't yet build a representation for the impl
itself.
Also, add an interface type value. This is necessary so that we have a
value for the expression on the right-hand side of `as` in an `impl`.
Previously, we created scopes for implicit parameter lists and tuple
patterns, but that meant that bindings went out of scope too soon. We
now keep them in scope until the end of the enclosing declaration. This
is accomplished by pushing a scope for parameters when we handle a name
that might have them, and then popping the scope again if it turns out
that there were no parameters.
For a case such as:
```carbon
fn A(T:! type).B(U:! type).F(x: T, y: U) {
var z: T;
}
```
... we now have the following scopes in the stack:
- A parameter scope containing `T`.
- A class scope for `A(T:! type)`.
- A parameter scope containing `U`.
- A class scope for `A(T:! type).B(U:! type)`.
- A parameter scope containing `x: T` and `y: U`.
- A function body scope containing `z: T`.
The innermost scope when check processes a declaration of a function,
class, or similar is now often a parameter scope rather than the
enclosing scope in which the class or function is declared, so the
target scope is now passed explicitly into the modifier checking code
that wants to inspect that enclosing scope.
This doesn't matter too much as these were expanded by the LLVM iterator
facade, but eventually this should enable that facade to be a little
less fancy and should also simplify the dispatch to directly use the
three-way comparison.
One case is a bit subtle and didn't have a comment so I added one to
explain a bit what is going on there.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
This is one of the nicer improvements to boiler-plate as we benefit from
concepts simplifying the constraints on the templates, defaulted `!=`
finding `==`, and the `<=>` operator for a single definition of all
relational operators.
I've checked and the `!=` usages do seem to be effective at finding the
`==` definitions, but we still need both the RHS- and LHS-based
overloads of `==` as far as I can tell.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
This removes the use of `enable_if` and tries to adopt concepts instead
of type traits when available.
The `ostream.h` change is a bit subtle as it adds a restriction not
previously in place -- that the stream is *contvertible* to
`std::ostream` as well as having it as a base class. This seems to match
the intent of the code.
The `hashing.h` code adds an implementation detail concept, and so I've
also clarified that the dispatch namespace is an internal one that isn't
part of the public API.
I'm basically just nudging down the path I think is right here. Adding a
small bit more support, but also more tests to capture cases that I
think will need to be verified as working.
With diagnostics like "Value of type `<function>` is not callable.",
that's because it expects a FunctionDecl but is instead finding a
ImportRefUsed. I'll need to work out the necessary support for a
callable function.
One more (hopefully last) rename on the Import instruction renaming.
I was kind of tempted to rename to just "IRId", since the IRs aren't all
imports. However, this felt easier to read, and a better choice than
CrossRef because it's more consistent with the other ways imports exist
in code. (even if IRs aren't all imports, most use-cases are derived
from imports)
Note though that import_irs may include IRs not just from direct
imports. Beyond the builtin IR, I'm thinking that for indirect imports,
or the prelude, we may end up adding them. e.g., so that constants can
be generated for indirect imports and still correspond to a directly
known IR, and for a given IR that's indirectly imported multiple times
to be deduplicated locally. I'm not there yet, I'm just mentioning this
to help give background for naming thoughts.
This required adding a few headers that were found transitively before,
but not too many. This is sadly a fairly manual process of opening every
file in my IDE, but I think I got everything in `//common` and
`//toolchain`.
There are a few cases where technically we don't need `foo.h` to be
included into `foo.cpp`, but I've forced those to stay with a pragma.
I've tried to catch the places where we can cut deps in Bazel as well,
but not sure I got all of those.
I had been noticing these in other PRs and it seemed better to isolate
the change.
C++20 mode exposes bugs in `clang-tidy` 16 that are challenging to work
around, but this should manage to do so. The patch to `bazel_clang_tidy`
has been sent upstream but no need to wait for it.
We don't actually specify a specific version of C++ in our style guide
docs, and the Google style guide has already been updated to be based on
C++20 so nothing to be done there.
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Richard Smith
CJ Johnson
Jon Ross-Perkins
josh11bandJosh L
czapiga
Calvin
Gıyaseddin Tanrıkulu
Chandler Carruth