Instead of manually creating a map from symbolic types to concrete
types, create a Specific and look up parameter types via that Specific.
This allows C++ to call a Carbon function like `fn F[T: type](unused t:
T*) {}`. See generic_pointer.carbon.
Document the phase of associated constants, which the discussions of
contextual phase defaults never covered: an associated constant is
always a checked generic binding. This is deliberately not presented as
a contextual default, because no other phase is possible for the
construct; correspondingly, no phase keyword (including `template`) is
allowed on one. Also rework the associated constants section of the
design README where the migration left a non-sequitur ("set to
compile-time values ... and so are defined using a `let` declaration"):
describe the `let` syntax and the binding's contextual phase separately.
Use consistent terminology for bindings versus constants. Bindings are
"checked" or "template" (generic) bindings, replacing the "symbolic
binding" and bare "generic binding" terms, so "symbolic" now only
describes constants and values. The "Symbolic facet bindings" section of
the generics details becomes "Checked facet bindings". The
expression-phase terms "symbolic constant" and "template constant" are
unchanged, and the binding-pattern definitions now name the constant
each kind binds. Template bindings are additionally described as
dependent and late checked, with each instantiation providing the
binding's value.
Also normalize the remaining "regular parameter" mentions to "runtime
parameter" to match the binding terminology, and describe the
compile-time "let template" as introducing a template generic binding
"T: C" whose uses are template constants.
Assisted-by: Claude Code
When we check an `impl` decl, find the containing `match_first` block,
if any, and store a connection to it in the `SemIR::Impl` structure,
along with the impl's position in that `match_first` block so that we
can sort/prioritize the `SemIR::Impl`s later.
Also, update the `is_final` flag if the `match_first` block is modified
as `final`. But ensure we diagnose trying to put a `final impl`, or a
redeclaration of one, in a `match_first` block. Also diagnose if an impl
is attached to a `match_first` block more than once - either in two
different blocks or in the same block at different positions.
Putting the `match_first` connection on the `SemIR::Impl` structure
means we have to import it, so implement import and add a smoke test for
that, which ensures nothing explodes.
Drop the `scope_stack` entry for the `match_first` block, as it was not
needed. Once we started tracking the `match_first` size on the `Context`
class, it became more straightforward to just store the `match_first`
decl `InstId` in the same place. The `match_first` block is not supposed
to act like a different scope for the purpose of redecls anyhow, so it's
a bit simpler this way.
It must be in a namespace, function, or class. In particular, it can't
be in another match_first, doing so immediately associates any impl
inside with two different match_first scopes, but we only want them
associated with one for prioritization.
Push a scope_stack entry for match_first blocks, and handle impls being
inside those scope entries. An impl should not use the `match_first`
block as its "enclosing scope" for the purpose of deciding if the impl
is a redecl of another impl. We should look through it to the class or
namespace the impl (and match_first) are located inside.
We don't yet store the relationship between the impl and its match_first
block, nor then can we use it in impl lookup for prioritization.
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
When formatting an impl definition, include in the textual output if the
impl is final. Since we (mostly) write them as `final impl` in the code,
use the same notation in the textual semir.
The implicit/explicit `.Self` concept is a heuristic at best, so we
should avoid relying on it. The ImplicitOnly option is no longer used,
as it was used to remove/disambiguate `.Self` in nested facet types, but
we have banned nesting `where` on the RHS of a `where`. So we no longer
have to worry about ambiguous `.Self`.
We can remove all the designator tracking heuristics in `.Self`
substitution as well now, as they were used for the now-removed
ExplicitOnly (removed in
https://github.com/carbon-language/carbon-lang/pull/7460) and
ImplicitOnly options.
The `style = "fixed"` is partially implied by `indent = 4`, but not
fully in some specific cases. There were also bugs in fully applying it
that are now fixed, and so we can specify it explicitly to get rumdl to
canonicalize nested list indentation more thoroughly.
Assisted-by: Antigravity with Gemini
The self and interface given to TryFindMatchingWitnessFromImplLookup
come from the output of SubstPeriodSelf, so there is no need to do the
substitution again. And the self came from a LookupImplWitness
instruction so it is already fully canonicalized.
The implicit/explicit `.Self` concept is a heuristic at best, so we
should avoid relying on it. We now only have one value of `.Self` in a
facet type (we have banned nested `where` on the RHS of a `where`). So
we don't need to preserve any `.Self` for the purpose of disambiguation,
and we can subst all `.Self` on the the RHS of a rewrite constraint.
`LookupImplWitness` instructions inside the impl declaration can't use
the impl they are apart of. Previously we had an heuristic in eval which
would try to prevent finding the impl for a lookup from inside that
impl. But it breaks when the `.Self` is replaced in a generic impl with
a symbolic, and then that symbolic is replaced in a specific. The
specific's decl block contains that `LookupImplWitness` instruction and
it tries to use the impl it came from. This causes the same specific to
be formed again, but now it exists, so it's used as-is but it has no
decl block yet, and so we crash.
Now we ban an impl while we resolve its specific, both deduction of its
arguments and from any other substitution. The prevents instructions
from inside the impl (which are evaluated when resolving the specific)
from finding their own impl. We do so by adding the ImplId to a stack on
the Context, and then skipping such impls when looking for candidates
during eval.
This fixes a crash, which was demonstrated by the new test being added.
It also makes another todo test pass.
There's a whole lot of other semir churn, which seems to be mostly
reordering of constants. There are some fingerprint changes in
constants, but it appears they are the same canonical instructions, so
they don't represent a behaviour change. For example in
`toolchain/check/testdata/for/actual.carbon` the `%N.patt` constant has
been given its fingerprint suffix now as `%N.patt.aa5`. But they are
both this instruction, so it is just a formatting change:
```
inst6100001A: {kind: SymbolicBindingPattern, arg0: entity_name61000002, type: type(inst61000018)}
- name: `N`
- type: type(inst61000018): <pattern for Core.IntLiteral>; {kind: PatternType, arg0: inst(IntLiteralType), type: type(TypeType)} (concrete)
- value: symbolic_constant61000001
```
I attempted the TODO as stated but we can't remove the `.Self` from the
LHS of a rewrite right now, without causing evaluation to run and
potentially find a concrete value to replace the access with, which then
breaks the association with the associated constant. There's a separate
TODO about that in SubstPeriodSelfInFacetType.
`.Self` will only be replaced in a facet type, as the facet type
constrains a facet. If it's part of a (non-facet) type, then the object
of that type is not a facet, and we can never replace that `.Self`.
`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.
Clarify that the functions that search for a witness in a facet type no
longer take inputs from the query directly, but now take them from an
identified facet type constructed from the query. That means the self
type is already canonicalized.
We used to have to batch these at the end of pattern traversal in order
to avoid accidentally adding them to a block that was speculatively
pushed for an expression within a pattern, but that's no longer a
concern with the more precise handling of those speculative blocks in
#7445.
In #7436 we stopped substituting `.Self` when collecting witnesses out
of a facet type. While this was correct, it did not capture all the
cases that need to avoid substituting `.Self`. And it poisoned the
`IdentifiedFacetType` cache by not replacing `.Self` but storing the
result in the cache. This led to incoherent behaviour, where the result
of an impl lookup would change depending on which ones had been done
previously.
Now we use a flag to track for each `.Self` if we're currently
type-checking inside the scope where it was introduced in a facet type.
While inside that scope, identify should not replace the `.Self`. Any
use of it should remain as-is since we don't yet know what value will
replace it. We call this state "frozen" since it should not be modified
by identify. This requires a substitution step when we leave the scope
that introduced the `.Self`, to remove the flag. The flag is set in the
`EntityName` of the `SymbolicBinding`, and is part of the canonical
value, since `.Self` can become part of types, which are constants, and
the flag needs to follow it for correct behaviour.
We also have to ensure the flag is the same when doing comparison with
constants from inside a facet type and constants from outside. For
instance in `(Z where .Z1 = ()) where .Z2 = .Z1`, when we arrive at the
second `.Z1` its `.Self` will be frozen, while the `.Z1 = ()` contains a
non-frozen `.Self`. So we add the frozen flag to the first when storing
it in `where_stack` in order to compare the constant values of the two
`.Z1`.
The `WhereExpr` requirement inst kinds now have an `InstConstantKind` of
`AlwaysUnique` instead of `Never`. This allows us to add them to the
usual InstBlocks, and in an `eval fn` body they have a constant value,
so eval does not fail when trying to call that function. We have to be
careful to not consider `AlwaysUnique` as being actually concrete
though, since their constant value erases `.Self`-dependence. This
allows us to stop special casing them when thawing the requirements
block in a `WhereExpr`, and we can just thaw each `InstId` in the block
in a straightforward manner.
We add the new flag to the instruction's fingerprint and name in
formatted semir.
This adds SemIR structs and implements building `observe` lists, as well
as naming, formatting, and importing `observe` declarations.
---------
Co-authored-by: Dana Jansens <danakj@orodu.net>
This allows C++ to call Carbon functions with generic type parameters,
with some conditions. Example:
```carbon
interface I {
fn Doit(self);
}
class A {
impl as I { fn Doit(unused self) {} }
}
class B {
impl as I { fn Doit(unused self) {} }
}
fn F[T:! I](t: T) {
t.Doit();
}
inline Cpp '''
void G() {
Carbon::A a;
Carbon::B b;
Carbon::F(a);
Carbon::F(b);
}
''';
```
The initial support is limited; only explicit parameters are handled
currently.
`CarbonExternalASTSource::GetOrExportFunctionToCpp` now generates a
`clang::FunctionTemplateDecl` for generic Carbon functions. If C++ code
attempts to call that templated function,
`CarbonExternalASTSource::LoadExternalSpecializations` will be called
with the template argument types of that call site. Then we can generate
a specialized thunk for those argument types for C++ to call.
This prevents an issue when trying to check in changes top
bazel-generated template files,
which aren't actually valid C++, making clangd_tidy issue errors and
block the commit.
This follows the pattern of our main compile benchmarks, but instead of
running the compile through a library API, it does so by running a
separate process. This lets us see the performance that we would expect
from `make` or another build system invoking the compiler, as opposed to
a minimal view from the library API.
Assisted-by: Claude Code
In some cases the pattern block can depend on the initializer, so it
must be sequenced after it. See #7469 for a more detailed explanation of
why this is necessary.
Each file dump now starts with a `; ---` comment and ends with a blank
line. This makes it easier to visually scan the dump for a file of
interest. The comment format is somewhat arbitrary; I chose `---` to
align with the `--- filename.carbon` separator in SemIR dumps, but
without the filename, because that appears on each of the next two lines
already.
In `SourceGenTest.IdentifierByteSumStableAcrossSeeds`, the `first`
variable was storing `llvm::StringRef`s pointing to memory allocated by
a temporary `SourceGen` instance. This memory was freed at the end of
the loop iteration, leaving `first` with dangling references that were
accessed in subsequent iterations.
Fix this by storing `std::string` copies of the identifiers in `first`
to own the memory, and use `llvm::equal` to compare them.
Assisted-by: Antigravity with Gemini
This performs `.Self` substitution in a single step, for the whole facet
type, instead of doing it individually for each constraint visited in
the top-level facet type. Then we don't need to track state to avoid
subst in constraints that come from other named constraints.
The semir changes are because we now generate a whole other FacetType
from the substitution.
Add a result for Subst() to return when you want to recurse into the
instructions operands but not the type_id. This comes up when recursing
and looking for facet types written in an instruction, but not
referenced indirectly through a type_id.
We can't skip adding the instruction to the worklist entirely, since we
need to pop it back off to rebuild the containing instruction later. So
we just mark it with a skip flag, and don't call Subst() on it.
The suggestion for a change to Subst was made in
https://github.com/carbon-language/carbon-lang/pull/7367#discussion_r3423446839.
`GetIdentifiersImpl` unconditionally sliced the 64-entry
`IdentifierLengthCounts` table even for uniform distributions, which the
API documents as having no `max_length` limit. Requesting a uniform
distribution with `max_length > 64` therefore tripped an out-of-bounds
slice assertion. Only compute the table slice on the non-uniform path,
where `max_length <= 64` is already enforced.
Add `IdentifierByteSumStableAcrossSeeds`, which exercises this path (a
uniform request up to length 200) and checks the core invariant that the
total identifier byte count is independent of the random seed across a
spread of parameters.
Assisted-by: Claude Code
---------
Co-authored-by: josh11b <15258583+josh11b@users.noreply.github.com>
`ErrorRecoveryBuffer::Apply` flagged each inserted token via the
pre-insertion index of the token it was inserted before. Once any
earlier insertion had been applied, that index no longer matches the
token's position in the merged list: with two closers inserted at the
same point (`{((}`), the second recovery token was left unflagged, and
with insertions at two separate points (`{(} {(}`), a real token was
flagged in the second one's place.
`IsRecoveryToken` now reports exactly the inserted tokens. This will
matter for `carbon format`: a recovery token's text exists in no source
byte range, so a minimal-edit model must know not to anchor edits on it.
Assisted-by: Claude Code
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
Instead of maintaining a stack of pending subpatterns which might or
might not contain expressions, we mark non-nesting regions during
pattern handling that might contain an expression. The implementation
remains largely the same; the difference is that callers are expected to
end a pending expression region as soon as possible, rather than wait
for the end of the subpattern. This makes it possible to emit
non-pattern insts during pattern handling, without the risk that they
will get caught in a pending expression region further up the stack.
When we find a named constraint during identity, we recurse into it. The
specific args of the named constraint may contain references to `.Self`
which can then make `.Self` appear inside the named constraint, which
was making us replace `.Self` at multiple levels and incorrectly. The
first specific argument replaces `Self` in the named constraint, and we
pass in the self-type of the identify operation. This may contain
`.Self` and we should _not_ be replacing the `.Self` references with the
self-type that they are contained within. This led to infinite cycles.
In the meantime, we have made the toolchain reject any ambiguous `.Self`
from being constructed. So we know there is only one value of `.Self`
around in a facet type.
So now we replace `.Self` only in the top level facet type during
identity. That means replacing `.Self` in the specifics of the named
constraints that we recurse into. But we do _not_ replace `.Self`
anymore inside those named constraints. This resolves the infinite loop.
At the same time, when we are identifying an `impls` constraint from
earlier in the same facet type, like `C impls Z(.Self)` we are
identifying with a self-type of `C`. We want the output to use `C` as
the self-type since we should get back an identified facet type that
says `C impls Z(.Self)`. But we do _not_ want to replace the `.Self`
there since we're inside a facet type and the `.Self` does not refer to
`C`. So we parameterize `TryToIdentifyFacetType` to not replace `.Self`
when identifying an `impls` constraint from the `where_stack()`. This
resolves a large number of `fail_todo_` tests.
Impl lookup identifies the types of facets in the query, replacing
`.Self` in each type with the facet. This allows references using the
facet from outside the facet type to match similar structures inside the
facet type.
However replacing `.Self` with the facet can re-evaluate symbolic impl
lookups inside the facet type. These can perform deduction in generic
`final impl`s, which can attempt to convert the facet. Convert does an
impl lookup with that facet in the query, which causes us to form an
infinite recursion cycle.
Add tests that caused such a cycle, to demonstrate we no longer crash.
Some of these tests fail impl lookups using concrete values in the query
that match values from a `final impl`, with TODOs to address them.
This proposal removes the `:!` syntax for generics and templates in
favor of keywords (`generic`, `template`, `runtime`) and contextual
defaults for phase. It also replaces `:?` with `fwd` and introduces
`exttype` for extended types.
Assisted-by: Antigravity with Gemini, and Claude
---------
Co-authored-by: Geoff Romer <gromer@google.com>
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
One API fix: BumpPtrAllocator no longer tracks the amount of memory it's
handed out separately from the amount of memory it has allocated from
the system.
Assisted-by: Gemini via Antigravity
`IsRawIdentifier` checked `token_text.starts_with("r#")` and then read
`token_text[2]`, but `starts_with` only guarantees a length of two. An
`r` identifier immediately followed by `#` at the end of the source --
so the token text is exactly `r#` -- made the `token_text[2]` read run
off the end. Guard on the length first.
Found by fuzzing. The read is reached only via `GetTokenText`, so the
parser fuzzer, which does not request token text, never hit it.
Assisted-by: Claude Code
Our development instructions now recommend installing a number of
binaries through `cargo`. Updating these binaries has to be done by
hand. So we can provide a script that users can use to update them
easily and regularly.
Dana Jansens
Nicholas Bishop
Richard Smith
Chandler Carruth
Geoff Romer
Özgür T. ÖnsoyandDana Jansens
Lucile Rose Nihlen
