Instead of silently producing an `ErrorInst::InstId` when looking up a
private qualified name in the current package, bypass the access check.
We don't need it -- private names from other libraries are filtered out
by the import logic.
Also fix `DiagnoseInvalidQualifiedNameAccess` to actually always produce
a diagnostic, instead of silently ignoring access control failures in
non-class types. This is a no-op after the fix to the access logic,
since we only allow access control at class and namespace scope
currently, but should avoid this issue from recurring when that changes.
Assisted-by: Gemini via Antigravity
We exposed `Core.IntLiteral()`, `Core.FloatLiteral()`,
`Core.CharLiteral()`, and `Core.Bool()` as functions as a workaround,
because we had no way to provide the type names without parentheses that
the design requests. But now we can do so, by using an alias. Switch all
of these over from being functions to simply being names of the
corresponding types.
Assisted-by: Gemini via Antigravity
Change `Lookup` by InstId to return a ClangDecl pointer. All callers
were immediately calling `Get` anyway, so this makes call sites a little
shorter. The other `Lookup` method, by ClangDeclKey, is sometimes called
without calling `Get`, so left that as-is, but renamed to `LookupId`.
Also add a `decl` method to ClangDecl so that the commonly repeated
`clang_decl->key.decl` can be written `clang_decl->decl()`.
My understanding is that `auto` is encouraged even when there's no
applicable naming convention for the variable, and that suffixes like
`_id` are encouraged where applicable, even if the type is explicit, so
there's really no connection between the two policies.
This ensures the clang_decls map is used as a cache - without this,
visiting the same entity twice could cause it to be
re-exported/duplicated. See attached test case.
Replace all uses of CppGlobalVarStore store with ClangDeclStore.
Adding a VarStorage->VarDecl mapping to ClangDeclStore is now done with
the `AddVar` method, which takes an extra `pattern_id` arg. While the
corresponding `ClangDecl` is unchanged from before, the reverse mapping
in `inst_id_to_clang_decl_id_` now uses the `pattern_id` as the key.
This is necessary because in some places the original VarStorage
instructions gets replaced (e.g. by a call to `Convert`). The
`pattern_id` remains stable in those cases.
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
This proposal clarifies some unclear aspects of the interop support for
object-like macros. In particular:
- Carbon supports importing an object-like macro if its definition can
be evaluated as a constant expression, without further restrictions on
that definition.
- When importing the result of that evaluation, C++ lvalues are imported
as references, and rvalues are imported as values.
Removing the clang_decl_id on SemIR::Function - using only the
clang_decls map to create the association between SemIR::Function and
clang::FunctionDecls.
This adds an `is_external` flag to ClangDecl to indicate whether the
entity originated from Carbon or was imported from another language.
(I'm open to names - I guess for now we mostly use "is this from C++" to
be more specific than "is this external" - eg: NameScope::is_cpp_scope)
Fix import logic to make all imported packages be children of the
`NameScopeId::Package` scope. Previously, indirectly-imported packages
would end up as children of their importing package's scope, which
resulted in them not being treated as packages at all, and in particular
not being fingerprinted as packages.
Fixing that caused a failure in the fingerprinting logic as we started
to encounter packages with no correspoding import scopes. Instead of
looking for import scopes, use a simpler mechanism to map packages to
their package names, and clean up.
Unfortunately the latter change churns all the fingerprints again :(
Hopefully this is the last time for a while.
Fix a lowering crash when lowering a return by reference of a type with
an in-place initializing representation. We previously misinterpreted
this as an in-place initializing return.
This is addressed by changing lowering to interpret a `ReturnExpr` of a
reference expression as a reference return. However, that exposes
another issue: `return var;` produces a `ReturnExpr` of a reference
expression in the case where it returns in place! To fix that, we switch
`return var;` to producing a `ReturnExpr` of a value expression
regardless of whether the function has a return slot. This makes the
representation of `return var;` more uniform:
* If the expression is a reference, we're performing a `ref` return.
* If the expression is an initializing expression, we're performing a
normal by-initialization return.
* If the expression is a value expression, we're performing a `return
var;`.
Implement support for floating-point <-> integer type conversions as
described in #820 and #845, extended to support `unsafe as` conversions
for the conversions that can't be expressed as either implicit
conversions or `as` conversions.
One tricky part here is conversions from floating-point literals to
integer types. Such literals may have both a very large mantissa and a
corresponding somewhat large negative exponent, and still produce a
result that is in the range of values that a small integer type can
represent. In order to support that while avoiding building very large
2^N or 10^N constants in general, we first compute a conservative
approximation of the number of bits necessary to represent the integer
result, with an early exit if the number is either definitely too large
or definitely zero. The remaining cases have a reasonable bound on the
size of integer necessary to compute the base^exponent multiplicand.
Assisted-by: Gemini via Antigravity
Add `ExportVarToCpp`. This checks the `clang_decls` mapping and returns
an existing decl if found. Otherwise, it creates a new `VarDecl` and
adds it to the `clang_decls` mapping.
When lowering, in `FileContext::BuildGlobalVariableDecl`, the
`clang_decls` mapping is used to lookup an existing
`llvm::GlobalVariable` for the instruction. If found, use that rather
than creating a new one to avoid an unwanted second definition in the
llvm IR.
This provides a native harness with several advantages over
`pre-commit`:
- Faster when initializing the cache
- Smaller cache sizes: under 43mb compared to over 62mb
- Better integration with `uv` for Python usage
Steps for migrating for existing contributors:
1. Install `prek` following instructions in the updated docs.
2. Replace hooks in an existing checkout with a special flag:
```sh
prek install --overwrite
```
The `--overwrite` flag is what removes the old hooks.
If you used the pre-push variant:
```sh
prek install --hook-type pre-push --overwrite
```
3. Optional cleanups:
```sh
rm -rf ~/.cache/pre-commit # reclaim the old hook-environment cache
pipx uninstall pre-commit # if installed via pipx; or `brew uninstall
pre-commit`
```
Assisted-by: Antigravity with Gemini
Not sure how these got missed when moving other things to `uv`, but this
should clean them up.
The bump to Python 3.12 is so that we can use `@override` with the
simple import from `typing`. This is needed by the newest versions of
`ty` to do type checking. Added the relevant `@override` annotations.
Assisted-by: Antigravity with Gemini
This change adds rudimentary support for C++ ranges in Carbon range-for
loops. Since C++ ranges are exposed through `Core.Iterate`, C++ ranges
have the same limitations as Carbon ranges (e.g. can't return
references).
`Iterate.CursorType` now requires `Destroy`, since types that implement
`CppRangeForIterate` can't be used in range-for loops unless their
cursor type can be verifiably destructible.
Since we can't currently iterate over ranges as references, the
`CppUnsafeDeref` interface can't require types that implement it to take
or return references.
Update the orphan rule to require a name to be defined within, or by,
the same scope as the impl declaration. Since libraries can not be
nested, this also enforces the old rule, but rejects more impl
declarations. In particular, it rejects an impl declaration in a generic
context which would have no way to provide a value for the generic
bindings it inherited from its enclosing scope, making the impl
unusable.
Discussed in open discussion
[2026-05-04](https://docs.google.com/document/d/1mjllGO3ZCL4qGt9uJHUtcxKoHAGEY7Y999ie4EtBWB8/edit?tab=t.3ifnhz83n73d#heading=h.p45zfugbmdih).
When we import from another library in the same package, its entities
end up with our library as their parent scope, resulting in cross-file
fingerprint mismatches. Instead, only include the library ID when
fingerprinting either a package-private entity or an `ImportIRId` that
refers to a particular `SemIR::File`.
Instead of requiring just one to have a designator, require each one.
You can't write `(type where A == B) & (type where C == .Self)` because
`A == B` has no designator. If the two facet types are combined into a
single `where` syntactically, their meaning does not change, and what we
allow should not change either. That is, `type where A == B and C ==
.Self` should be rejected since `A == B` does not contain a designator.
The design is also updated to make this clear.
Pass a function's self specific in `ScopeStack::PushForFunctionBody` and
remove assertion preventing lexical scopes from having specific IDs.
This allows lexical lookup within the function to find entities
associated with its self specific.
Closes#6793
Use the Carbon-computed alignment for allocas, loads, stores, and
memcpys. Previously we used whatever LLVM felt like giving us, which
would result in ABI mismatches and runtime crashes due to misalignment
when creating objects of imported C++ class types, as well as resulting
in some surprising choices like `(i32, i32)` and `()` having 8-byte
alignment instead of 4 and 1, respectively.
For now, disable the use of array types as by-var paramters and by-init
return types when exporting Carbon functions to C++, as C++ does not
support raw arrays being passed or returned by value.
Assisted-by: Gemini via Antigravity
Note that #7266 has already updated the toolchain for `char`, but not
`Core.CharLiteral`.
Assisted-by: Gemini via Antigravity
---------
Co-authored-by: Josh L <josh11b@users.noreply.github.com>
Co-authored-by: Geoff Romer <gromer@google.com>
Include the library name in the fingerprint of an entity declared
`private` at namespace scope. Include the entity's fingerprint in the
mangling of a library-private entity.
This fixes miscompiles if two libraries in the same package declare
`private` entites with the same name. We can't fix this with internal
linkage because library-private entities can be reachable through
generics defined in the API file of the library.
Assisted-by: Gemini via Antigravity
As suggested in [1], replace `FieldInitializerMap` with a `FieldStore`.
The corresponding `FieldId` is now stored in `FieldDecl`. To make room
for the `FieldId`, the `ElementIndex` is now stored in the `Field`,
along with the initializer.
In import_ref.cpp, resolving `FieldDecl` initializers is now supported,
and in convert.cpp `LoadImportRef` is called to do so. The
`field_initializer_import.carbon` test now passes.
Printing a `FieldDecl` instruction now prints the initializer as well,
if present. See field_initializer.carbon for an example.
[1]:
https://github.com/carbon-language/carbon-lang/pull/7238#discussion_r3283217158
The `unused` modifier is rejected on parameters of a function
declaration, but the check only covered the explicit parameter list, so
an implicit parameter (such as self or a compile-time binding) could
carry unused without a definition. Check the implicit parameter list
too.
The code changes and the test updates are split into two commits for
easier review.
Assisted-by: Claude Code with Claude Opus 4.7
---------
Co-authored-by: Christopher Di Bella <cjdb.ns@gmail.com>
Carbon-side thunks (for example the `Copy`/`Destroy` witness thunks
generated for imported C++ types) are mangled by Carbon, and their names
incorporate a fingerprint of the involved types. The instruction
fingerprinter identifies a class only by its name and parent scope,
which is sufficient for Carbon classes but not for imported C++ classes:
different specializations of one class template (and other cases such as
types in anonymous namespaces) share a Carbon name and parent scope. As
a result, the thunks for two distinct specializations could mangle to
the same name, producing a single LLVM function with two definitions and
failing `verifyModule` during lowering.
When fingerprinting a class imported from C++, also include the Clang
mangled name of its type.
Test: toolchain/lower/testdata/interop/cpp/thunks.carbon gains a split
with two specializations of one class template, each requiring a thunk;
their thunks now get distinct mangled names instead of colliding.
Assisted-by: Claude Code
---------
Co-authored-by: Christopher Di Bella <cjdb.ns@gmail.com>
The `--dump-cpp-ast` file tests strip references to Clang builtins so
that the expected output is target-independent. The filter anchored a
`__`-prefixed builtin identifier on a preceding space or quote, which
matches the x86-64 `__va_list_tag` spelling but not the AArch64
`std::__va_list`, where `__` is preceded by the `::` namespace
qualifier. That left a single `RecordType 'std::__va_list'` line
unfiltered on AArch64, producing a spurious autoupdate diff for
`thunk_ast.carbon`.
Anchor the match on a preceding `:` as well so namespace-qualified
builtins are also filtered.
Carbon's test workflow covered Linux on x86-64 and macOS on AArch64, but
had no Linux AArch64 coverage, so AArch64-specific issues that don't
reproduce on macOS could land unnoticed. Add an `ubuntu-22.04-arm`
runner to the matrix.
The release used for Linux does not publish the monolithic
`LLVM-*-Linux-ARM64` package, only a `clang+llvm-*-aarch64-linux-gnu`
community build with a smaller tool set, so the Ubuntu setup now selects
the tarball by `runner.arch`. The prune step uses `rm -f` since the two
packages do not ship an identical set of tools to remove.
Assisted-by: Claude Code with Claude Opus 4.7
We've talked about adding the title to the filename several times over
the years and it seems really valuable. This requires us to compute a
"slug" for the title spelling that can be part of the filename.
Beyond that, we crossed 7000 recently, and so it seems likely that we
will need to add digits sooner rather than later here, so this goes
ahead and moves us to 6 digits so we don't have to adjust again for a
reasonable length of time.
To implement this and ensure we can sustain it going forward this adds a
tool to our pre-commit that validates (and corrects if needed) the
filename.
In order to update everything and keep links working, there are a _lot_
of changes, but the most interesting for direct review are in
`proposals/scripts`.
Assisted-by: Antigravity with Gemini
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Also make some updates and improvements to fine-tune existing skills.
Simplify AGENTS.md to remove redundant instructions that duplicate
information that's already in skill files -- skills should be loaded
automatically and should not need to be redundantly specified in
AGENTS.md.
Assisted-by: Gemini via Antigravity
This removes the need to install any specific version of Python or
figure out how to configure it by instead asking users to install `uv`
and letting it manage Python. Among other advantages, `uv` is designed
to be fast enough to embed directly into our scripts.
We were already using this in `bench_runner.py` so that the script could
import non standard library dependencies. Moving to it for the rest of
our Python unifies the approach and will also enable dependencies
whenever needed.
I've left `github_tools` alone as it has special handling with its own
Bazel setup.
I've updated the contributing tools to explain the approach here.
When forming the constant value of a `where` expression, don't consider
it to be `.Self`-dependent if the dependence only comes from the RHS of
the `where`. More generally, ignore `.Self` dependence when evaluating a
facet type unless it comes from an extended interface or named
constraint. While we can get other kinds of constraint from the
left-hand side of a `where`, such constraints must either come from the
right-hand side of other `where` expressions or be extend constraints.
This fixes a crash in lowering caused by a concrete function containing
a `.Self`-symbolic `where` constant.
Assisted-by: Gemini via Antigravity
When performing deduction for a call to a generic function, we would
previously convert runtime arguments to match the parameter type, then
throw away the result. Instead, track whether deduction needs the value
of the argument, which will be the case only within compile-time
contexts such as generic bindings and types of instructions, and only
perform conversions during deduction for those contexts.
Fixes miscompiles when passing an argument requiring a runtime
conversion with side-effects to a generic function, where previously the
side-effects would have happened twice! (Once from deduction and once
from the real call argument conversion.)
Assisted-by: Gemini via Antigravity
LSP assumes lines are index 0 to n-1, but Carbon Locs are index from 1
to n. We had the logic for this correct for the start of range but not
for the end of range (inclusive range).
Before this was the diagnostic span we would produce:
```carbon
fn F() {
return ();
<~~~~~~~~>
}
<~~~~~~~~~~~>
```
after:
```carbon
fn F() {
return ();
<~~~~~~~~>
}
```
Subst is typically used for constant values, in which case an
`ErrorInst` anywhere results in a final `ErrorInst`. However there are
some use cases for substituting non-canonical instructions. And in that
case we need to take care in two ways:
- Some instructions have no constant value, such as the requirements in
a (non-canonical) `WhereExpr` instruction. When we rebuild them, we
can't do so by building a constant value and getting the canonical inst
id, since the constant value will be "runtime". For rebuilding a
non-canonical instruction, we should `AddInst` instead.
- When substituting something in a non-canonical instruction with an
`ErrorInst` inside it, we want to preserve the structure of the
non-canonical instruction, and leave the `ErrorInst` in place. As such,
we remove the early out in Subst so that we keep substituting after
encountering an error.
However the rewrite constraint resolution was relying on the `ErrorInst`
early out to correctly stop recursing when it found a cycle. So now we
do the checks for a cycle in its `SubstCallbacks` subclass, and avoid
substituting and rebuilding instructions once we have encountered a
cycle.
A lot of this is based on a few experiments using tools to help
synthesize a cohesive proposal or improve them. Likely more that can be
done here to get the most out of our tools in this space.
Assisted-by: Antigravity with Gemini
Richard Smith
Nicholas Bishop
Geoff Romer
David Blaikie
Dana Jansens
Chandler Carruth
Christopher Di Bella
antangelo
dependabot[bot]
DavidLoftus