`scripts/jj_push.sh` takes the same arguments as `jj git push`, runs
prek over the commits that push would send, and pushes only if they
pass. It learns what is being sent by running `jj git push --dry-run`
and reading back the plan, so `--bookmark`, `--change`, `--all` and the
rest work without reimplementing how they select commits.
Hooks that rewrite files need a commit to write into, so the checks run
with the working copy on top of the commit being pushed. When the
working copy is already an empty commit there, which is the common case,
it is used directly; otherwise one is created, and named in the error so
the fixes can be squashed.
`scripts/jj_prek.sh` gets two changes. It forwards its arguments to
`prek run`, so `jj_push.sh` can ask for a specific range, and it now
changes to the workspace root before running. It exports `GIT_DIR`,
which makes git treat the current directory as the work tree, so prek
could not find its configuration from a subdirectory.
`jj` does not expand aliases when completing arguments, so `jj push`
completed file names. `scripts/completions` has Bash, Zsh, and Fish
completions that give it the same completions as `jj git push`.
`docs/project/contribution_tools.md` documents the `push` alias, and a
`prek` alias for `jj_prek.sh`, with the other per-repository `jj`
configuration. Both are opt-in.
Assisted-by: Claude Code
When evaluating a deferred member access action, the scope stack cannot
be relied on, so `LookupUnqualifiedName` cannot be used in
`GetHighestAllowedAccess` to get the `Self` type.
Instead, store the `Self` type in the `Context` when evaluating a
method, and use that in `GetHighestAllowedAccess`.
Add rules to not overwrite git/jj history without asking, since this
destroys the reviewer's view of things. And some information on dealing
with stacks of commits within a single bookmark/PR.
Prek can make fixes for whatever caused a failure, and then pass when
you run it again, even though the user didn't change anything, and that
is now explained.
Assisted-by: Opus 5
If a Carbon class overrides virtual functions from a C++ base class but
is never referenced from C++, it is never exported to Clang. During
lowering, `BuildVtable` then fails to find a `CXXRecordDecl` and crashes
when attempting to get the vtable from Clang's code generator.
Ensure dynamic classes with foreign vtables are exported to Clang when
completing the class definition in `CheckCompleteClassType`, and look up
`first_decl_id()` in `BuildVtable`.
Fixes#7721
---------
Co-authored-by: Dana Jansens <danakj@orodu.net>
Per feedback on #7665, this PR switches the default value
table storage from canonical constant inst_ids to
non-canonical.
Furthermore, this PR simplifies the default value support in
check by requiring that the first owned declaration of a
function completely specify all of its default values.
Updates the diagnostic code and tests to reflect this new
stricter requirement.
Remove claim from script that it takes minutes; this caused Claude to
decide to not run it. It only takes a few seconds these days. Add note
in toolchain development skill that lints won't be accurate if the
compilation database is outdated.
Assisted-by: Claude Opus 5 via Antigravity
This action was created to wrap any `MetaInstId` operand of an action
instruction. This served two purposes:
1) It had a special hook in `OperandIsDependent` to allow it to be
performed while it had a dependent operand (the reference to the
instruction in the generic).
2) It created a `specific_inst` so that the downstream action saw an
instruction in the specific instead of one in the generic.
These are both replaced: the special case in `OperandIsDependent` for
`RefineInstAction` is replaced by a special case for `MetaInstId`s in
general, and the `SpecificInst` is now created as part of performing the
downstream action, rather than as a separate step carried out
beforehand.
This simplifies the produced SemIR and reduces the number of splices
significantly. It also prepares us to handle actions like
initialization, where we don't actually want to create `SpecificInst`s
immediately in the location where the action is performed, because they
actually belong somewhere else in the IR.
Assisted-by: Claude Opus 5 and Gemini via Antigravity
Replaces several quadratic-time steps in the algorithm with linear or n
log n implementations. Worst-case runtime before hitting the complexity
bailout drops from 25ms to about 12ms, and typical runtime is
single-digit ms on my development machine.
Assisted-by: Claude Code
Instead of always printing types as canonical, attempt to find a sugared
type where possible, and include that type in the diagnostic. We can
only do this when given the instruction whose type is being printed
(`TypeOfInstId`) rather than the canonical type ID.
Initial support here is intentionally minimal: just looking through
calls to the callee's declared return type, and looking through pointer
dereferences and corresponding pointer types, to build out the initial
infrastructure. More cases can be added later; this degrades gracefully
to using the canonical type if a better type can't be found.
Assisted-by: Claude Opus 5 via Antigravity
We use the same conversion codepath to handle both qualification
conversions and derived-to-base conversions, because we allow both to be
performed at once. However, we were previously modeling the
qualification conversion as happening *first*, and producing a result
whose type is the target type of the overall conversion (that is, the
base class type). That led to bogus SemIR, where a `Derived` -> `const
Base` conversion would first have a "compatible" conversion from
`Derived` to `const Base`, *then* an access of the base subobject (of
type `const Base`, within an object of type `const Base`).
We now reverse the order: first we do a derived-to-base conversion,
which already has logic to preserve qualifiers, and then we do any
necessary qualification conversions on the result to reach the overall
target type.
In passing, we now skip forming the `as_compatible` instruction at all
for a pure derived-to-base conversion that has no qualification
conversion, simplifying the SemIR by one instruction in the common case.
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
Four regions had `end` patterns that could fail to match, so `return
var;`, a `fn` with no parameter list, and an unterminated `"` each
swallowed the rest of the file; 77 of 1696 testdata files lost their
highlighting partway through. Operators were wrapped in `\b`, which only
holds next to a word character, so `a + b` highlighted nothing. And the
keywords had drifted about two years behind the lexer.
Identifiers are now classified by naming convention plus a call-site
lookahead, the way the Rust grammar does it, so nothing carries between
lines. Regions survive only for strings and embedded C++, where a
terminator reliably turns up, and raw strings spell out hash levels 0
through 2, so `\n` is an escape in `"..."` and plain text in `#"..."#`.
Trailing comments, character literals, raw identifiers, `$0`, `0o`
octal, arbitrary integer widths and six missing keywords are covered
now, `destructor` is gone, and `i32` reads as a type rather than as a
keyword.
Highlighting stays forgiving rather than diagnostic: anything after `//`
is a comment and odd numeric spellings still read as numbers. Pointing
out mistakes is the toolchain's job, and lenient rules hold steady while
you are still typing.
Every keyword and symbol in `token_kind.def` is covered, and unscoped
tokens across examples and the toolchain drop from 39% to 21%.
Note that I haven't tried to read and reason about every minute change
here as there are just too many. But I'm working on a follow-up PR that
adds testing that should be significantly easier te review.
Assisted-by: Claude Code
We were passing a SpecificInterfaceId which just makes code have to do a
lookup to get the actual SpecificInterface. The caller already has the
SpecificInterface, so plumb that around.
SpecificInterfaceId really only exists when we need to stick a
SpecificInterface into an instruction as an operand.
Fixes#7731, at least under `--share-cpp-ast` which is expected to be
the future direction.
When --share-cpp-ast is enabled and any compilation unit has C++
imports, include all compilation units in the shared CppDomain inputs
and assign the domain to every unit. In ImportCpp, when a unit has no
direct C++ imports but is covered by a shared CppDomain, initialize
its C++ AST context and import namespace. This ensures units without
direct C++ imports have access to the C++ AST and code generator when
instantiating generics or referencing declarations from units that do.
Assisted-by: Antigravity with Gemini
The images beside the samples were screenshots taken by hand, so they
drifted: they still showed `package Carbon api;`, syntax the language
dropped in 2024.
`render_sample.py` renders a sample the way the grammar in this
repository actually highlights it, using `tmlanguage.py`, a small
TextMate tokenizer. VS Code runs grammars under Oniguruma, which we
cannot depend on here, but this grammar uses no Oniguruma-only syntax,
so `re` runs its regexes unchanged and the two agree on every character
of every Carbon file in the repository. The output is SVG, so
regenerating needs nothing but Python, and a later grammar change gets a
same-path image diff showing what it did to real code.
The samples change where a construct left the language (`api`, `Carbon`
as the package name, `StringView`, `destructor`), refresh the
`keywords.carbon` inventory against `token_kind.def`, and add sections
for octal, raw and block literals, character literals, lambdas, and raw
identifiers. `interop.carbon` is new, covering inline C++. These are
highlighting fixtures rather than programs, so the deliberately invalid
lines stay.
Assisted-by: Claude Code
---------
Co-authored-by: Dana Jansens <danakj@orodu.net>
Use a single `SemIR::Function` per `Core` interface method, whether it's
generated locally or imported. This prevents generating duplicate
functions, which lead to different types when the witness appears in a
`FacetValue` as part of a specific for a class.
We use a `CanonicalValueStore` of `GeneratedFunction` objects that allow
finding an existing FunctionId for a `Generated` special function before
(re-)generating it. Mangling for `Generated` functions is also moved to
use the values from the `GeneratedFunction`'s canonicalization key, so
that we have a consistent source of truth for the unique ID of a
`Generated` function across all files.
New tests are in
`toolchain/check/testdata/impl/custom_witness/destroy.carbon`.
Update the rules for member access:
- Simple member access `a.b`
- If `a` names a scope, performs name lookup and optionally `impl`
lookup.
- Otherwise, `a.b` is shorthand for `a.(typeof(a).b)` and always
performs instance binding.
- Compound member access `a.(m)` does optional `impl` lookup and always
performs instance binding.
- This is a change from only performing instance binding if `m` is an
instance member.
- New operation `a.impl(m)` is introduced. It always performs `impl`
lookup, and nothing else.
- The `BindToType` interface is removed. Only instance binding may be
customized (using the `BindToValue` and `BindToRef` interfaces).
As a result, member access doesn't use whether the right operand is an
instance member anymore. Instead, instance binding is performed whenever
it would be plausible, and a new syntax is used to opt out.
Assisted-by: Gemini via Antigravity
---------
Co-authored-by: Josh L <josh11b@users.noreply.github.com>
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Some `google-` prefixed rules have been renamed to rules without the
prefix. The `google-` prefix still remains as an alias to these new
rules. Since we turn on the `google-` prefix rules and use those in
NOLINT expressions, disable the new aliased names. Otherwise we have to
put both names in NOLINT expressions.
This is a style choice we often agree with, and call out in code review.
But there are many cases where we do want to split apart nested ifs,
such as when working with LLVM apis like `dyn_cast`:
```
if (auto* thing = dyn_cast<Thing>(other)) {
if (thing->foo()) {
...
}
}
```
Or we may have TODOs or other comments in the scope of the outer if,
which the tidy check ignores.
Since this doesn't lead to bugs, we disable the check and leave this to
reviews and authors for their discretion.
The diagnostics documentation covers how a diagnostic is produced and
worded, not the form it takes on a terminal. This adds
`toolchain/docs/diagnostics_rendering.md`: what a diagnostic is made
of, its layout, color, character set, and width, and how each degrades
when the terminal can't render it.
A diagnostic is one message plus labels read against the code they
mark, `Primary` or `Info`; a context and a path are set out apart. It
renders as one frame, anchored per file, with no headline: the message
hangs off the range that is wrong, led by the level word on a heavy
mark, with `->` in the margin on the reported lines. Labels read in
the order of their ranges, connectors that cross earlier rows break
around them, and a path draws only for the message's own location.
Ten rows on the shared example, against Clang's six and rustc's eleven.
Nothing a diagnostic says is dropped for width: source is windowed,
words wrap past the level word, connectors slide, and a label that
still can't hang is out-dented. Under sixty columns the compact form
takes over, one located line per part. Color is named per element with
a ramp per level and a light-background palette; ASCII keeps every
distinction with `^`, `.`, and the level word. The `file:line:column:`
header is the road not taken: structure comes from the language
server, and the compact form covers a grep-able line.
Assisted-by: Claude Code
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Instead of running on the "current" `@` (the state at which the previous
`jj` command happened to be run), intentionally trigger a working copy
snapshot from `jj_prek`.
Assisted-by: Claude Opus 5 via Antigravity
The hashing code is generic over the type of the value being given to
sizeof() so ideally this warning would not happen at all, but it does.
Possibly because the value is the return of an overload set, so it's not
obvious that it's the templated type. One of those overloads returns
`const void*` but change that to an integer does not remove the warning
still.
Clang is synthesizing a cast when using an enum value from a template
parameter, and then clang-tidy is finding and reporting that cast.
Upstream bug: https://github.com/llvm/llvm-project/issues/222793
clang-tidy 24 warns on C-style variadics, and we don't need to use one
here anymore. Instead of a function call with an argument list, use a
concept to determine if a type can be list initialized.
This check allows styles that we don't use so it's not really useful for
enforcing our style guide. And prevents the use of `_1` or similar in
destructuring declarations where want to use `_` for multiple variables,
such as `auto [_1, _2, foo] = bar()`.
The warning wants all classes inherited as CRTP base classes to hide all
their constrcuctors and friend all uses of them. We use CRTP quite a lot
and across different components of the toolchain, which would make
maintaining friend lists frustrating.
### Description
When looking up default initializers for class elements in
`ConvertStructToClass`, the compiler previously assumed that every
member looked up from the class scope was a `FieldDecl` and called
`GetAs<SemIR::FieldDecl>` directly.
For a derived class with a base class, looking up `base` returns a
`BaseDecl`, which caused a `CHECK` assertion failure when casting to
`FieldDecl`. Use `TryGetAs<SemIR::FieldDecl>` instead so non-`FieldDecl`
entries like `BaseDecl` are recognized as having no default initializer,
cleanly diagnosing that the `base` field is missing.
Fixes#7722
Assisted-by: Google Deepmind Antigravity
The StringRef represents a bounded region of a string, but using data()
drops the end bound, and makes LLVM construct a new StringRef starting
in the same position and going until a nul terminator. If this worked
correctly before, it was because the StringRef was always pointing to a
full `std::string` or the tail of one.
While we do adhere to its expectations most of the time, we don't
always. This is a low value check, and it's a stylistic choice to use
`#if defined(...)` when paired with `#elif defined(...)`.
Chandler Carruth
Nicholas Bishop
Dana Jansens
ATHARVAandDana Jansens
David Blaikie
Lucile Rose Nihlen
Richard Smith
simontran7
