These changes are aimed at improving code readability and
maintainability within the `StaticScope` class of the AST folder.
- **Optimized Print and PrintID Methods:**
Introduced a template function `PrintCommon` to handle common logic in
`Print` and `PrintID`. This change simplifies the class interface and
avoids repetition, enhancing code readability.
- **Simplified TryResolveHere Method:**
Streamlined the `TryResolveHere` method by simplifying the conditional
logic. The refactored code is more readable and easier to understand,
improving overall code quality.
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
The parse nodes are still tracked as part of the same value store
interface in order to ensure parity, but they're split out from Inst
itself in order to reduce the size of Inst -- the expectation is that
they don't need to be passed around quite as much.
This change doesn't actually reduce the passing very much, although
there are hints of it: AddInstAndPush doesn't typically need a separate
parse node from the one on the Inst itself, for example. In a couple
spots I changed code to rely a little more on the InstId until the
ParseNode is needed, but it's very low hanging fruit where done. I think
convert could do more to not eagerly fetch the parse node before its
use, but more cleanup felt it would be easier to handle separately. I'm
currently viewing this as making such cleanup _possible_ rather than
executing on it up-front.
But also, I want to make sure there's a consensus to head in this
direction before pulling the trigger. We speculated that this would
result in the parse node being passed around less, and I do think that's
the case, although it's a bit fuzzy in the change.
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
This is accomplished by tracking an extra bit on the ID we store in the
constant values table, and propagating that from subexpressions to the
enclosing expression. This extra bit is not yet computed correctly for
types; that will be addressed in later PRs.
---------
Co-authored-by: Chandler Carruth <chandlerc@gmail.com>
We propose a roadmap for 2024 focused on a working Carbon toolchain that
supports Carbon ↔ C++ interop.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Form a side table with constant values for each instruction. Evaluation
is only supported for a few very simple kinds of instruction for now.
This is not observable outside of the SemIR output, because nothing
depends on expressions having a constant value phase yet.
yash is mainly for flex/bison formatting, which was for explorer: we're
refocusing away from that, so it doesn't make as much sense to include
now.
black I think is relatively new, it was previously part of the python
plugin. I'm hoping inclusion helps with setup.
Per discussion, unqualified lookup should only occur on identifiers that
existed at the time Context was initialized. As a consequence, the
resize logic in LexicalLookup may not be necessary. Even considering
metaprogramming, if metaprogramming is restricted to qualified name
lookup, it may not be necessary in the future. Support should be easy to
add if we need it. Trying to clearly document in the CHECK message what
the rationale is.
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
This is primarily being done for performance reasons, removing hash
lookups. The increased memory consumption is accepted.
Refactors LexicalLookup out to its own structure.
This is currently built on top of #3575. I'm just getting to the point
where imports have enough logic that I'm thinking they may be able to
stand on their own.
Namespaces are copied, which means also adding their name to the
underlying instruction. It happened not to be done previously; the name
was only in name lookup.
Since the only import supported right now is the default import,
functionality is limited; in the future I'll need to deal with namespace
vs package conflicts.
Tests of namespace imports are under "namespace" -- I figured this would
be best for scaling as more instructions get support.
This also improves some debugging-related output that I was trying to
use while trying to build the support.
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
This patches bazel_clang_tidy handling of headers. I found an equivalent
change at https://github.com/erenon/bazel_clang_tidy/pull/13, but that
was [already
rejected](https://github.com/erenon/bazel_clang_tidy/pull/13#issuecomment-1047007424).
Per the criticism, this will result in redundant processing of headers.
The project instead uses `HeaderFilterRegex: ".*"`, but that results in
two problems:
1. When running with `-k`, errors are repeated when a header is included
more than once, which is common.
2. clang-tidy including errors from headers that are included from other
modules (e.g., abseil-cpp); filtering correctly is difficult.
Given the trade-offs and options (including forking), I thought patching
was preferable so long as it remains narrow.
Also stop supporting `var` with initializer inside `for`.
Resolves TODO in `handle_variable.cpp`
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
This change adds a `BindSymbolicName` instruction for generic bindings,
paralleling the existing `BindName`. A mechanism is also added to allow
both kinds of binding to be accessed uniformly, for convenience in the
case where the two different kinds of binding are treated the same.
Generic bindings of type `type` are allowed to be used as types,
although no operations are provided for such types. For now lowering
treats these types as empty structs, which seems like a reasonable
lowering for non-monomorphized unconstrained types.
This is a step towards adding enclosing scopes for imports. It creates
an indirection for all bind names.
We discussed specializing for bindings that are in function scope (i.e.,
not a useful enclosing scope for imports or diagnostics). However, the
thought is to go ahead with this singular approach for now, and only
change structure if it's a performance issues so that we have
incrementally fewer instructions to handle.
Add a mechanism to define instruction categories, to support inspecting
the common representation of similar kinds of instruction. Use that
mechanism to make formatting of branch instructions slightly more
type-safe.
The idea here is to use the existing `Inst` mechanism for converting to
and from structs, extended to operate on a struct representing multiple
different kinds of instruction. In this case, the concrete kind of
instruction is stored in the struct in a `kind` field, rather than being
implied by the type.
Factored out of #3555 where this mechanism is used to provide a common
interface for runtime and symbolic name bindings.
This sets things up to use `bazel` to run `clang-tidy` using
https://github.com/erenon/bazel_clang_tidy.
I'm fixing issues outside of explorer, and disabling clang-tidy for
targets in explorer that have legacy issues. I was going to disable
clang-tidy for targets in explorer such as interpreter anyways, because
they're slow to parse, and just extended that to the currently failing
targets.
Don't first form a `std::tuple` and then convert it to a struct. Speeds
up compilation of extract.cpp by 15-20%.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
Co-authored-by: josh11b <josh11b@users.noreply.github.com>
This involves a number of supporting changes:
* The `parse_node;` member of instruction types may now have any type
derived from `Parse::NodeId` and is no longer required to have that
exact type.
* `Parse::Node::Invalid` is now a singleton object of a separate type
that is convertible to `Parse::NodeId` and its descendants. This
replaces the `Invalid` member of its descendants, and avoids having to
write long `NodeIdOneOf<...>` types when initializing variables to
invalid.
* `IndexBase` now allows `==` and `!=` comparisons between its derived
classes and types that are convertible to those types.
* A number of functions in the check stage have been changed to preserve
more type information instead of using `Parse::NodeId`.
* `NodeIdForKind<K>` (also known as `KId`) now has a `Kind` member so it
may be used to declare `NodeIdOneOf<T, U>` types without #including
`parse/typed_nodes.h`.
* `NodeIdForKind<K>` (also known as `KId`) may be implicitly converted
to `NodeIdOneOf<T, U>` if `T::Kind == K` or `U::Kind == K` (executing a
TODO).
Many of the `parse_node` members were not converted since they would
have required more extensive changes. They have been marked with "TODO"
comments.
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
This is mirroring the structure of codegen/codegen.h, lower/lower.h, and
check/check.h. I recently did lex/lex.h, so parse/parse.h is the last.
Now, the directory's main API file is eponymous with the directory.
I could've used a friend function to avoid making the Tree constructor
public, but in other places we make less use of `friend`, just leaving
things public. This felt more consistent, and simple because it only
affects the constructor.
When formatting special values, we currently have `NameId` doing `if
(*this == SelfValue)`, `BoolValue` doing `case 0:`, and `TypeId` doing
`if (index == TypeType.index)`. I'm suggesting we consolidate onto the
`*this == SelfValue` approach for consistency, it seems the easiest to
see the mapping of values.
This mixes in #3552 because I'm adding `Exports` there. I'm suggesting
reformatting `Empty` and `Exports` consistent with `Unreachable`.
This adds a block for exported InstIds, rather than scanning the package
scope. This working down a path discussed last month, which we'll need
to add enclosing namespaces to the Inst in order to complete import of
something like `namespace NS; var NS.a;`
Exports could've been a separate `vector<InstId>` on `SemIR::File`, but
using an entry in `inst_blocks` felt more consistent.
I think this check doesn't offer enough value to try to work around the
false positives. We probably will, at times, check the contents of an
optional without validating because structurally we know it must have a
value. Here though, I believe the `while` will be doing a check of the
value; we shouldn't need a more explicit check.
Goal is to increase type safety, though more work needs to be done (see
added TODOs).
Note that, after this change, check handlers corresponding to deleted
parse node kinds will no longer compile.
---------
Co-authored-by: Chandler Carruth <chandlerc@gmail.com>
Use this new category to replace the unconstrained `NodeId` child of
`MemberAccessExpr ` and `PointerMemberAccessExpr`. For now this new
category matches `IdentifierName` and `BaseName`, but later this will be
expanded to support `a.(b.c)` and `p->(b.c)` syntactic forms.
QUESTION: Is it time to make a `node_category.def` x-macro file?
ANSWER: Not yet.
Tests previously uncovered code. Fix uncovered problems:
* formatting of trace output
* package & import directives need to be classified as declarations
* the problem that meant the previous problem wasn't caught by existing
tests (since `Tree::Verify` didn't check that top-level declarations
match `AnyDeclId`, as required by `Tree::ExtractFile()`).
The explorer encoded the old runfiles layout in a string in one place
that didn't get updated. This causes our instructions for running the
explorer to break. The fix is at least quite simple.
Fixes#3540Fixes#3541
Previously would fail with:
```
CHECK failure at toolchain/parse/node_kind.cpp:74: Lex::TokenKind::Error == expected_token_kind || token_kind == expected_token_kind: Created parse node with NodeKind NamedConstraintDecl and has_error 1 for lexical token kind Constraint, but expected token kind Semi
```
Issue found by fuzzing.
These are intended to allow the structure of a parse tree node to be
described more precisely in code, to support these use cases:
- Automated checking that the parse tree conforms to the expected
structure. (Added to `Tree::Verify`.)
- Easier reading and understanding of the structure of the parse tree by
toolchain developers. (See `parse/typed_nodes.h`.)
- Easier navigation of the parse tree, for example for tooling uses and
for use when forming diagnostics.
On this last point, an object representing the file may be inspecting
using `Tree::ExtractFile`, as in:
```
auto file = tree->ExtractFile();
for (AnyDeclId decl_id : file.decls) {
// `decl_id` is convertible to a `NodeId`.
if (std::optional<FunctionDecl> fn_decl =
tree->ExtractAs<FunctionDecl>(decl_id)) {
// fn_decl->params is a `TuplePatternId` (which extends `NodeId`)
// that is guaranteed to reference a `TuplePattern`.
std::optional<TuplePattern> params = tree->Extract(fn_decl->params);
// `params` has a value unless there was an error in that node.
} else if (auto class_def = tree->ExtractAs<ClassDefinition>(decl_id)) {
// ...
}
}
```
The `Extract...` functions collect the child nodes into the typed parse
node's fields (internally using a `Tree::SiblingIterator`) for easy
access. However, this is not as fast as directly observing the tree
structure using the postorder strategy being used by the check stage.
These functions rely on using struct reflection on the typed parse node
definitions from `parse/typed_nodes.h` to get the expected structure of
child nodes and then populate them.
Note that validating these in `Tree::Verify` adds significant cost to
it, and is currently included in the parsing stage. Without this change,
a 10 mloc test case of lex & parse takes 4.129 s ± 0.041 s. With this
change, it takes 5.768 s ± 0.036 s.
This builds upon and completes #3393.
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Co-authored-by: Chandler Carruth <chandlerc@gmail.com>
Daniel James SzelogowskiandRichard Smith
Jon Ross-Perkins
josh11b
Geoff Romer