Historically, we disabled the rendered form in tests outside of
`toolchain/diagnostics`. The idea was to keep tests shorter and avoid
very long chains of autoupdate before stabilizing due to all the line
numbers and other artifacts in the rendered form.
This PR adds `--diagnostic-line-numbers=[all|first]` and defaults to
`first` in tests. This keeps the rendered form so that we can easily
code review that form and the visual cues it does or doesn't provide.
But it removes all but the first location's line numbers to provide
significantly more stability in tests.
By default, tests get the full unicode line-drawing form of diagnostics
so we can ensure all of the artifacts there make sense with the specific
case being diagnosed, but we do retain some focused ASCII-rendering
tests to ensure that fallback remains usable.
There are some issues here that will be fixed in subsequent PRs.
Notably, we try to keep the frame together in the check lines, and so
force some parts to use the first location. This largely works, but
fails in some edge cases, causing the entire frame to be located
incorrectly. There is a TODO about this that we need to fix, and it only
occurs 6 times across the test suite, so it didn't seem worth
front-loading a fix for here.
Assisted-by: Claude Code
Lexer diagnostics used a single `const char*` as their source location,
which only allowed pointing at a single character rather than a run of
characters. This limited the underlining of unrecognized tokens, and
also made escape sequence diagnostics particularly strange by stopping
at whatever point the lexer discovered the error and pointing only at
that location. For example: `"\w"` was reported at the `w`, and `"\x"`
at the closing `"`.
This change tries to always keep the primary location of a diagnostic
the point at which typing (inserting or deleting) might fix the error.
So for `"\x"`, `\x` is underlined, but the position is at the `"` as
that's where you would type a hex code.
The lexer's emitter now takes a special lexing source location that
represents either a single position _between_ two characters, a single
character, or a range of the source buffer. The lexer diagnostics are
updated to construct these for the character range as appropriate.
Parser diagnostics for a missing token were located at the _next_ token,
which is particularly confusing for terminators like missing `;`s. Now
they are located at a position after the previous token, with or without
a space depending on the token, but typically on the same line.
Diagnostics still sort by how far the parser has read.
The parser diagnostics for unclosed groups also now attaches a label to
the opening token when it is on a separate line. And the diagnostics for
mismatched indent in a multi-line string attaches a label to the closing
`'''` that establishes that indent.
There are likely more location improvements we should do here, but this
seemed like a reasonable incremental improvement and is what Claude
found when doing an initial audit. One example where I think we should
make further improvements is around suggesting raw identifiers when we
find an unexpected `Cpp` or `Core` or `base`. We do this consistently
now, but raw identifiers should be rare. A TODO is added for these
cases.
Assisted-by: Claude Code
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Implements check and pattern matching support for struct patterns.
This PR implements complete patterns, and leaves `_` field handling for
future work.
Treat the function as having a `-> auto` return type. Reject calls to
functions that still have a non-deduced `-> auto` return type, and
rewrite the function to have the actual return type once it is known.
Assisted-by: Gemini via Antigravity
Diagnostic rendering was part of the data model: `Loc::FormatLocation`
and
`Loc::FormatSnippet` wrote plain text to a stream, so no capability,
color, or
terminal width could reach them.
A diagnostic is now one message with labels, contexts, and location
information
attached, and `Diagnostics::Renderer` draws it into a
`Terminal::Buffer`. The
design is in `toolchain/docs/diagnostics_rendering.md` (#7668),
authoring in
`toolchain/docs/diagnostics.md`.
Before:
```
example.carbon:6:3: error: 1 argument passed to function expecting 0 arguments
Run0(1);
^~~~~~~
example.carbon:3:1: note: calling function declared here
fn Run0() {}
^~~~~~~~~~~
```
After:
```
error: 1 argument passed to function expecting 0 arguments
╭─┤ example.carbon:6:3
┆
3 │ fn Run0() {}
· ─────┬─────
· ╰──┤ calling function declared here
┆
-> 6 │ Run0(1);
· ───────
╰────────────
```
`--no-diagnostic-snippets` gives a compact form, `example.carbon:6:3-9`.
File
tests use it; the rendering tests under `toolchain/diagnostics/testdata`
ask for
snippets.
C++ interop diagnostics are data now rather than text Clang drew:
`SemIR::ClangSourceLocId` names a `clang::SourceRange`, and a Clang
location is
marked as the point it names.
`StreamConsumer` moves to a `:renderer` target that `:emitter` does not
depend
on, keeping `common/terminal` out of `consumer.h`. Capabilities come
from the
file behind the error stream, so tests get plain ASCII.
`renderer_fuzzer` draws diagnostics built from fuzzer bytes with every
capability combination, since a shape the renderer doesn't expect must
never be
why the compiler dies while reporting a problem.
TODOs mark the follow-ups: line weights and crossing lines, wrapping
that keeps
a quoted type whole, and ordering diagnostics that share a byte offset.
Also: `IncorrectExtension`'s hint moves into its message, since its note
had no
location of its own, and `strip_noise` in `check_diagnostics.py` blanks
multi-line `#define`s instead of deleting them.
Assisted-by: Claude Code
Proposal p5434 specifies `-> val T`, `-> ref T`, and `-> var T` as
return forms. `-> ref T` is parsed as a `RefPrimitiveForm`; extend that
to parse a leading `val` or `var` after `->` as a `ValPrimitiveForm` or
`VarPrimitiveForm`, reusing the nodes produced by `form(val T)` and
`form(var T)`, and check them as forms.
---------
Co-authored-by: josh11b <15258583+josh11b@users.noreply.github.com>
We previously discarded a `generic` applied to `self`, resulting in
silently treating `generic self: Self` as a runtime parameter and
producing a weird error on `template self: Self`.
Instead of forcibly making all `self` parameters non-runtime, change the
*default* for a parameter named `self` to be runtime.
This is only a partial fix; more work is required to support `fn
F(generic self)` with an omitted type.
Previously `-> ref T` was parsed as a type expression whose outermost
node is a `ref` prefix operator (the same one used for `F(ref x)`
arguments), and `ReturnExprAsForm` recognized the resulting `RefTagExpr`
and turned it into a `RefForm`.
Per p5434, a return form that begins with `val`, `ref`, or `var` is not
a type expression. Parse a leading `ref` after `->` as a
`RefPrimitiveForm` node wrapping the type expression, reusing the node
produced by `form(ref T)`, and check it as a form. This prepares for
supporting `-> val T` and `-> var T` in the same way.
The only SemIR change is that the `ref_tag` instruction is no longer
emitted, and the return slot pattern refers to a `type_component_of` the
form, matching `->? form(ref T)`.
---------
Co-authored-by: Nicholas Bishop <nbishop@nbishop.net>
Support for `fn` expressions with the same feature set as `fn`
declarations. In particular, there is no support for captures,
positional parameters, `=>` bodies, or `auto` return types.
Note that this results in two parse nodes for each `=>` token. That's a
bug, but it matches the behavior of function declarations, and will be
fixed in a follow-up change.
Assisted-by: Gemini via Antigravity
No support for `each` yet, but this should be enough that we can try out
an implementation approach supporting expanding a statement across
elements of a tuple without building "real" variadics.
Assisted-by: Claude via Antigravity
Implements `typeof(expr)`, as described in #7697. `expr` is treated as
an unevaluated operand, and is kept in an `ExprRegion` separate from the
enclosing scope.
Assisted-by: Claude via Antigravity
Removes support for unspecified default values. Fixes canonicalization
of the `DefaultValuePattern` instruction by making them immutable after
they are issued, and by removing the `DefaultValueId` operand which
wasn't being canonicalized.
Fixes the malformed parse tree produced for an invalid let struct
pattern containing a single identifier (e.g., `let {s};`).
As pointed out by @DavidLoftus, the parser should produce a parse tree
similar to that of `let {ref s};`, since both are missing a binding
power operator `:`, and both do not have a `.` preceding the identifier
(i.e., `state.in_field_shorthand_pattern == true`).
This means that the parser can produce an the `InvalidParse` node just
as it does for `let {ref s};`.
Closes#7674
Adds a virtual node to `DefaultValuePattern` to end the
`ExprRegionForPattern` before checking the expression
for the default value.
When checking the default value expression, the context
was still configured to interpret expressions as patterns,
which caused some corruption of state with tuple-pattern
subpatterns.
Corrects an assertion failure I found while working on
feedback from #7665.
Adds parsing support only for default value expressions in pattern
lists, including leaving the default value unspecified with an
underscore `_`.
Per #7521.
Adds an algorithm to compute where to insert brackets to repair
bracketing mismatches during lexing. This takes indentation, as well as
a number of other cues, into account to predict where the brackets
should have gone. Detects when there is ambiguity between solutions and
makes no suggestion in that case. Reduces the problem by splitting on
properly bracketed top-level constructs, then uses a beam search to find
good candidate solutions quickly.
This includes both a fuzzer and an eval tool that can be used to
determine how well the algorithm fares against a given corpus of valid
Carbon code, by damaging it in various ways and seeing whether the
algorithm can correctly fix it. On all the eval modes, this algorithm
can correctly infer the positions for over 80% of lost brackets (and can
correctly restore 95+% of brackets in some modes), with low rates of
incorrect suggestions.
See added documentation for full details.
Assisted-by: Gemini via Antigravity, Claude via Claude Code
Found by fuzz testing, root caused to #7479 - fixing this issue for Var
in a similar way to how it was fixed for Let in that patch (by adding
`RuntimeBindingName` to the possible things in the name binding.
Not sure if this would be better fixed by adding `RuntimeBindingName` to
`AnyRuntimeBindingPatternName` or something else?
Assisted-By: Gemini with Antigravity
It's hard to track 2-space indent levels across large vertical gaps, and
it can be hard to suppress the instinct to read the dump as if it were
preorder. This change introduces a new dump mode that addresses both
problems by using box-drawing characters to explicitly represent the
parent-child edges of the tree. This new mode is the default, but the
old behavior remains available with
`--parse-dump-format=yaml-postorder`.
This is in addition to finding a `where` on the RHS of another `where`.
Since a generic binding introduces `.Self`, any `where` expression that
isn't part of a facet type modifying the binding itself would introduce
an ambiguous `.Self`.
Add virtual parse nodes for let, var, and form bindings, which goes
before the type. This allows us to track if `where` appears in the
binding's type. We only need to look for an invalid `where` if any
appeared in the type. We combine these three nodes together into a
single node kind, which requires us to remove the name from it as a
child. We move it up to the Pattern node again, and rename the
PatternStart nodes to PatternTypeStart as they are now located in the
middle of the Pattern nodes, just before the type.
And we only need to thaw `.Self` in generic bindings. Non-generic
bindings can only have `.Self` through a `where` expression, since the
name is not provided otherwise to non-generic bindings. And `where`
expressions thaw their `.Self` independently. So the binding only needs
to thaw a `.Self` that it introduced, which is only for generic
bindings.
If the `match_first` is not followed by `{` avoid consuming whatever
comes after it. Recover by leaving whatever comes next alone. It could
even be the `FileEnd` token, and then we would crash when we read off
the end of the token stream looking for `FileEnd`.
Implement the toolchain side of proposal #7254, removing the `:!`
binding
syntax for generic and template parameters in favor of the keywords
`generic`,
`template`, and `runtime` plus contextual defaults for phase.
For valid programs this is semantics-preserving: each binding resolves
to the
same phase, and produces the same SemIR, as it did under `:!`/`:`. The
parser
derives a binding's phase from its syntactic context plus any explicit
phase
keyword; new diagnostics and error recovery for misused keywords are
described
below.
Implementation details for each component:
- Lexer: remove the `:!` (`ColonExclaim`) token, move its virtual
parse-node
budget onto `:`, and add the `generic` and `runtime` keywords.
- Parser: thread a `BindingContext` (`ExplicitParam`, `DeducedParam`, or
`CompileTimeEntityParam`) from declaration introducers down through
parameter
lists to each binding pattern, using a one-token lookahead to
distinguish a
name-qualifier parameter list from a declaration's own final list.
Parameters
of a compile-time entity (`class`, `interface`, `constraint`, `choice`,
`alias`, `export`, `namespace`) and deduced `[]` parameters default to
checked
generic; explicit function parameters and local bindings default to
runtime.
`HandleBindingPattern` resolves the phase from that context plus the
keyword: a
`generic` keyword needs no node of its own (the phase is carried by the
binding's node kind), while a `runtime` keyword is preserved as a
`RuntimeBindingName` node so `check` can name it in a diagnostic. A
phase
keyword that is merely redundant with the contextual default is
diagnosed
here, without invalidating the parse tree.
- Check: a phase keyword that is invalid for its context (for example
`runtime`
on a checked-generic parameter) is diagnosed here, and recovers by
building an
error binding that still introduces the name so that later uses of it do
not
produce cascading errors.
The removed `:!` syntax is now rejected as an ordinary parse error.
The `form`/`:?`/`->?` ("extended types") portion of proposal #7254 is
left for a
separate change.
Assisted-by: Claude Code
`match_first` is a declaration followed by a curly-brace block of
declarations.
The check phase will ensure only impl decls (or defns) are inside the
block.
Carbon currently requires a comment to be the only non-whitespace on its
line. A `//` comment that follows other content on a line, called a
_trailing comment_, is a lexer error. This proposal removes that
restriction, allowing a comment to follow other content on a line.
Everything else about comments is unchanged: a comment still begins with
`//`, still requires whitespace after the `//`, and still runs to the
end of the line. Carbon continues to provide only line comments; no
block or intra-line comments are added.
Three observations motivate the change. First, trailing comments are
well suited to short _annotations_ attached to a specific entity or
value on a line. Second, the lexer design now makes it trivial to lex
trailing comments, and in fact requires extra logic and potentially cost
to reject them. Third, C++ code routinely uses trailing comments, so
allowing them lets Carbon carry the layout of migrated code over
directly, rather than reworking each comment to read well in a different
structure.
Implementation notes (beyond the proposal's design):
Keeping trailing comments cheap to lex required a few supporting
changes, all of which keep the cost off the lexer's hot path:
- The lexer already dispatches `//` to comment lexing wherever it
appears, so classifying a comment as trailing is a single O(1) check of
whether the `//` is the line's first non-whitespace (`start + indent`).
The hot comment path is otherwise unchanged.
- That check relies on each line's recorded indentation being its real
leading whitespace. Multi-line string literals previously recorded the
column where the literal opened for the lines they span; they now record
the true (closing-delimiter) indentation instead.
- Parser error recovery (`SkipPastLikelyEnd`) had relied on that
opening-column indentation to keep tokens following a multi-line string
literal attached to the same construct. It now reconstructs that
relationship directly by consulting the line on which the literal
opened, including when other tokens follow the closing delimiter (such
as `''' + "more"`). This is on the cold recovery path.
- `CommentData` records the trailing bit in the high bit of its length
field, keeping it at 8 bytes.
Assisted-by: Claude Code
A lambda introducer (`fn` in expression context) with no parameters and
no body -- for example `(fn)`, where `)` immediately follows `fn` --
left `HandleLambdaAfterParams` calling `ReturnErrorOnState()` without
ever emitting a `Lambda` node. The orphaned `LambdaIntroducer` leaf was
then left where an expression was required, so `Parse` produced a tree
that failed its own verification and aborted via `CARBON_FATAL`.
Recover the way `HandleLambdaBody` already does for a missing body after
a return type: emit a placeholder `InvalidParse` body and finish a
complete `Lambda` node, so the lambda stays a valid expression and the
surrounding construct (here a `ParenExpr`) extracts cleanly.
Found by fuzzing.
Assisted-by: Claude Code
Instead of traversing the entire pattern block looking for
`VarPattern`s, we keep track of them on creation, and then build
`VarStorage`s directly from that list.
This also gets rid of the global `var_storage_map`, and instead keep
that information narrowly scoped to each full-pattern, and consume it in
a single linear traversal instead of with random-access lookups. To
enable that, this fixes a parse bug where nested `var` patterns were
getting diagnosed but not marked as errors.
Malformed `base` declarations with an omitted colon need two different
recovery paths. For `extend base`, the consumed `extend` modifier
requires the parse tree to retain its `BaseColon` and base expression
children, so this synthesizes an errored `BaseColon` and continues
parsing the expression.
Other malformed forms, such as `base calss X {}`, now use the standard
declaration-error recovery: emit `ExpectedAfterBase`, skip past the
likely declaration end, and form an errored `BaseDecl` without inventing
a colon or cascading diagnostics.
The regression covers `extend base Foo;`, bare `base;`, and the reviewer
counterexample `base calss X {}`.
Tests:
- `prek run --files toolchain/parse/handle_base.cpp
toolchain/parse/testdata/class/fail_base.carbon`
- `./scripts/run_bazelisk.py test -c dbg //toolchain/parse/...`
- `./scripts/run_bazelisk.py test -c dbg //toolchain/testing:file_test`
AI assistance: OpenAI Codex helped inspect the parser recovery path,
implement the change, and run verification. The operator reviewed and
authorized the contribution.
Assisted-by: OpenAI Codex
Implements proposal #7016: `self` moves from the deduced implicit list
(`fn F[self: Self]()`) to the front of the explicit list. Its type may
be written explicitly (`fn F(self: Self)`) or omitted, in which case it
defaults to `Self` (`fn F(self)`, `fn F(ref self)`); `self` in the
implicit list is rejected.
Throughout checking, `self` is modeled as the first explicit parameter.
Because a method is just a function whose first parameter is `self`, it
can also be called as an ordinary function with the receiver passed
explicitly (`Type.M(obj, ...)`), not only as `obj.M(...)`. A new
`SemIR::CallArgParamPatterns` helper chooses the parameters matched
against the explicit arguments, excluding a leading `self` only when it
is supplied as a method-call receiver; arity checking, conversion, and
generic deduction use it. The resulting SemIR and lowering are
unchanged: `self` is still `call_param0`, and witnesses, thunks, and
vtables are unaffected.
An omitted `self` type is parsed as a `SelfBindingPattern` node with no
type expression; checking synthesizes the `Self` type so it behaves
exactly like `self: Self`. However, the exact spelling used must match
between a forward declaration and a definition, following #3763's rules
around declaration matching.
Generated functions, thunks, and C++ interop import/export build `self`
as the first explicit parameter, and the `self`-type override (e.g.
Derived->Base for a virtual override) applies to the explicit `self`.
Placement is validated by new diagnostics: `SelfInImplicitParamList`,
`SelfNotFirstParam`, and `SelfOutsideParamList`. The benchmark source
generator and the documentation adopt the `(self)` shorthand; the
prelude, the examples, and the test data are migrated in the following
commits.
Assisted-by: Claude Code with Claude Opus 4.7
---------
Co-authored-by: josh11b <15258583+josh11b@users.noreply.github.com>
If a keyword or a sized type literal (eg, `f2`) is used in a context
where we are confident that we are expecting an identifier -- either
before a `:` in a binding pattern or after a `.` in a member access or
designator -- then recover as if a raw identifier was used.
This appears to be a particular stumbling block for coding agents, so
seems worth paying special attention to.
Add a mechanism to the tokenized buffer to track additional tokens
synthesized for error recovery so that we can keep the lexed token
sequence immutable and still satisfy the invariants throughout the rest
of the toolchain for recovery tokens. Thanks to chandlerc for suggesting
this approach!
This adds the `static` token to the lexer and parses it as a modifier.
In check, `FullPatternStack::Kind::FieldDecl` is now used for both
static and non-static vars. Static vars get treated basically the same
as `NameBindingDecl`s.
Global initialization is used for static var initializers. To make the
necessary stack information available to `pattern_match.cpp`, the
`full_pattern_stack` and `decl_introducer_state_stack` are now popped
later in `handle_let_and_var.cpp`.
In lowering, each class's body is checked for `VarStorage` insts and
lowered the same as global vars.
Class vars are still restricted to simple `name: type` bindings, not
full patterns. This is now handled in the check phase instead of during
parsing.
This is in preparation for supporting `static var`.
For #6830, add support for inline C++ fragments as a declaration rather
than as a packaging directive. For now, this uses `inline Cpp
<string-literal>;` as syntax. The prior `import Cpp inline
<string-literal>;` is left alone for the time being. We can decide
separately whether to remove that.
`inline Cpp` requires that there was at least one `import Cpp`. It's not
clear to me if that's the right design long-term, but it seems
reasonable for now.
Assisted-by: Gemini via Google Antigravity
When parsing a pattern, if we encounter something that isn't pattern
syntax, try parsing as an expression instead. We only need one-token
lookahead to distinguish pattern syntax from expression syntax.
Track a precedence group through pattern parsing so that we can allow
different kinds of expressions in a top-level pattern (such as the
operand of `let`) and in a nested pattern (such as a subpattern of a
tuple pattern or within grouping parens). For example, we do not allow
`case if ...`, and for now I've chosen to also not allow logical or
relational operators at the top level of a pattern, so `case 1 + 1` is
OK, but `case 1 == 1` and `case true and false` require parentheses.
This decision should be ratified or revisited by a design proposal.
Very basic check support is also provided, only sufficient to form an
`ExprPattern` instruction and nothing beyond that. For now, all pattern
matching against an `ExprPattern` fails with a TODO error. To support
that, I've switched from calling `BeginSubpattern` in the parent handler
of a pattern and `EndSubpatternAs*` in the pattern handler itself to
calling both functions in parent handlers, with `EndSubpattern`
converting an expression into an expression pattern where needed.
Depends on #6976.
Assisted-by: Gemini via Google Antigravity
Do not treat it as a 1-tuple pattern as we used to. The design indicates
that `(pattern)` is invalid, but this appears to be an oversight, and
grouping parens appear to be the intended interpretation.
---------
Co-authored-by: Geoff Romer <gromer@google.com>
Use the same node kind for the body of `case` and `default` handlers. We
don't need to distinguish these in check, so don't create extra node
kinds for them.
In order to make the nodes properly delimited, make the label (`case
...` or `default`) nodes be children of the `=>` node rather than
siblings. This allows us to use the node kind of the `=>` as the
bracketing node for the complete handler, rather than having two
different bracketing node kinds, one for each kind of label.
Fixes a compiler crash that occurs when a malformed lambda is provided
as an operand to an operator that strictly expects an expression
Changes:
- Emits an `InvalidParse` dummy node at the current position to act as a
placeholder for the missing body
- Changed state transitions so that `LambdaIntroducer` gets properly
wrapped into a `Lambda` node
Closes#6823
---------
Co-authored-by: Dana Jansens <danakj@orodu.net>
Co-authored-by: Carbon Infra Bot <carbon-external-infra@google.com>
This change ensures that a function signature always starts with an
`IdentifierNameMaybeBeforeSignature` node (renamed from
`IdentifierNameBeforeParams`), even in the case of function declarations
like `fn F -> T` that have no parameter list. As a consequence, this
ensures that we push new entries onto `pattern_block_stack` and
`full_pattern_stack` when we start processing the function signature.
Implementation of unused pattern bindings #2022, continued.
Whereas previous PR #6460 took care of parsing, and PR #6479 prepared
the stage by using _ in some test cases, this PR has the the actual
implementation, using a simple dataflow analysis.
---------
Co-authored-by: Burak Emir <bqe@google.com>
Co-authored-by: jonmeow <jperkins@google.com>
Add support for compile-time functions. `eval fn` is analogous to C++
`constexpr`, and is evaluated at compile time when it has compile-time
arguments. `musteval fn` is analogous to C++ `consteval`, and requires
that its arguments be available at compile time and is always evaluated
at compile time. For now we require the modifier to match across
redeclarations of the function. The specific modifier syntax here is a
placeholder and not yet part of an approved design.
Limitations: Only very basic support for evaluation is provided. So far
there's no support for mutable state or `if` expressions, but otherwise
control flow and passing and returning values should work. Carbon
evaluation recursion is modeled by C++ recursion for now, so you can
overflow the toolchain stack easily. Functions that use in-place
initialization will generally not work yet, as they are modeled as
passing a non-compile-time-constant reference to a temporary to the
call.
Add missing categorization of `name_binding_decl` as `NotExpr` to match
other similar declaration instructions like `FunctionDecl`, so that we
can uniformly skip over them when they occur within function bodies.
Assisted-by: Gemini 3 Pro and Flash via Antigravity
Chandler Carruth
oli-ej
David Blaikieandjosh11b
Geoff Romer
Lucile Rose Nihlen
simontran7
Özgür T. Önsoy
Dana Jansens
mstr-six
Nicholas Bishop
