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
When impl lookup results in an `ImplWitness`, either directly or later
through `LookupImplWitness` evaluating to an `ImplWitness` inside some
specific, it represents a dependency on the eval blocks of the
`ImplDecl` that the `ImplWitness` comes from. The `ImplWitness` contains
a `SpecificId` of the `ImplDecl`, and the constants in the witness table
must be viewed through that `SpecificId`. The constants may depend on
the declaration or the definition block of the `ImplDecl`'s specific, so
any use of the witness table from `ImplWitness` requires the specific
from the `ImplWitness` to be fully resolved: both the decl and
definition blocks.
The specific declaration block is always resolved immediately when
forming a specific id, so there's nothing to do there. And things are
simple when the `ImplWitness` returned from impl lookup is concrete. We
would simply resolve the specific definition directly in the evaluation
of `LookupImplWitness`.
But when the `ImplWitness` is symbolic, meaning there's a dependency on
generic parameters in its referenced specific, then we have a dependency
on resolving the specific definition in the `ImplWitness` for every
specific that gets applied to the `ImplWitness`, eventually resulting in
a concrete specific.
Previously we attempted to do this by representing a dependency edge
from the eval block containing the `ImplWitness` instruction to the
definition block of the specific inside the `ImplWitness`. We
represented this dependency edge with the `RequireSpecificDefinition`
instruction.
The challenge with the `RequireSpecificDefinition` instruction was
ensuring it always landed in the _same eval block_ as the one holding
the symbolic `ImplWitness`. Ultimately, we've found a way to do this
without the `RequireSpecificDefinition`. The witness table is accessed
through `ImplWitnessAccess`, so we resolve the specific definition
before each access to the witness table. If the definition is already
resolved, this just costs a lookup of the Specific from the value store.
We document the other things we tried, which don't work or were too
complex, below.
In PR #6236, we added the `RequireSpecificDefinition` instruction in the
eval of `LookupImplWitness`, but this is too late. If impl lookup
resolved to a symbolic `LookupImplWitness`, the `LookupImplWitness` is
added some eval block. Later, we form specifics and re-evaluate that
eval block, and the `LookupImplWitness` can evaluate to an
`ImplWitness`. Adding the `RequireSpecificDefinition` instruction at
this point inserts the instruction into whatever generic's eval block
happens to be on the top of the generic stack, not the eval block of the
generic we are building a specific for. And indeed we can't add
instructions to a generic's eval block later like this, as it would be
missing from specifics that were already resolved for the generic. This
incorrect insertion shows up as a crash in eval at the CHECK introduced
but commented out in [eval.cpp in
#7981](https://github.com/carbon-language/carbon-lang/blob/56d9bbc40add1e9263f2f9b2555c3a27292d1725/toolchain/check/eval.cpp#L3507-L3512).
This is covered by the
`require_specific_impl_definition_through_specific_instantiation.carbon`
test.
We tried moving the insertion of `RequireSpecificDefinition` to be
earlier - at the point where the `LookupImplWitness` is constructed, in
the `LookupImplWitness()` function, instead of inside evaluation. This
seemed like a straightforwardly correct answer, as the eval block that
contains the `LookupImplWitness` will then also contain the
`RequireSpecificDefinition`. However, this can cause us to lose the
`RequireSpecificDefinition` entirely. Because it's a separate
instruction, not an operand of `LookupImplWitness`, it does not get
treated as a dependency of the witness. When we do an impl lookup inside
an `alias`, we build a generic for the alias and then discard it. This
discards the eval block containing the `LookupImplWitness`. Then, in an
enclosing generic, we refer to the constructed `AliasDecl` instruction,
which pulls all its dependent operands into the enclosing eval block.
This pulls in the `LookupImplWitness` into the enclosing eval block, but
it does not preserve the `RequireSpecificDefinition`. This is covered by
the
`require_specific_impl_definition_from_lookup_impl_witness_in_class_definition.carbon`
test.
We tried having `ImplWitness` evaluation resolve the specific definition
of the specific `ImplDecl` it refers to. This seems like the simplest
approach by far as we should always be working with the correct eval
block and nothing can get lost. However it does not work because of the
recursive nature of `ImplWitness`. The `ImplWitness` instruction appears
in the declaration of an `ImplDecl`, and the `ImplWitness` holds a
`SpecificId` for that same `ImplDecl`. When building a specific
`ImplDecl`, we evaluate the `ImplWitness` in the `ImplDecl`'s
declaration block which then immediately resolves the specific
definition before the declaration is finished. This leads to missing
instructions in the specific definition. In short, this puts the
dependency edge to the `ImplDecl`'s specific inside the `ImplDecl`
itself, which is problematic.
We tried wrapping `ImplWitness` in another instruction,
`ImplWitnessUse`. Inside the `ImplDecl` declaration there is still
`ImplWitness`, but then in the definition there is `ImplWitnessUse` and
that is what is returned from the evaluation of `LookupImplWitness`. The
`ImplWitnessUse` is an `ImplWitness` plus it ensures the specific
referred to by the witness has its definition resolved. However this
breaks constant comparisons, as we get different witnesses inside an
impl versus outside.
The first iteration of this PR's solution had us insert the
`RequireSpecificDefinition` into the eval block when we sniffed a
`LookupImplWitness` instruction, however that missed cases where impl
lookup immediately resolved to a symbolic `ImplWitness`, which is why we
now sniff for the `ImplWitness` in the eval block. This is covered by
the
`require_specific_impl_definition_from_symbolic_impl_in_class_definition.carbon`
test.
We also originally made a mistake of not inserting
`RequireSpecificDefinition` when inside the impl definition that the
`ImplWitness` refers to. But we can form specifics ImplWitness values
through function signatures, and these must add the dependency edge on
that specific ImplDecl definition. This is covered by the
`require_specific_impl_definition_from_inside_impl_definition.carbon`
test.
Finally, we did have a working solution. It was to sniff instructions
added to the eval block of a generic, and when an `ImplWitness` is added
to the eval block (it is implied that it is symbolic), also add a
`RequireSpecificDefinition` to that eval block immediately. We also
resolved the definition of a specific referred to by `ImplWitness` when
`LookupImplWitness` evaluates to `ImplWitness` to cover the concrete
witness case. This complicated the construction of the eval block
immensely, however. It caused a situation where adding one inst to the
eval block actually adds two, which we needed to avoid when importing.
And it required inst-kind specific behaviour in the very general
construction of an eval block, completely outside of eval itself.
When a Carbon class from another library was used from C++, its fields
were never exported, so C++ member access failed with 'no member named'.
The class scope of an imported class holds import references rather than
`FieldDecl`s, so `LookupClassFieldByStructField` didn't find them.
Resolve scope entries to their constant `FieldDecl`, load the import
references before exporting fields, and consistently key exported fields
by the `FieldDecl`.
Unfortunately `SemIR::LookupClassFieldByStructField` can't call
`LoadImportRef` because `SemIR::LookupClassFieldByStructField` is in
`SemIR` and `LoadImportRef` needs a `Check::Context` (and `check`
depends on `sem_ir`, not the other way around, so `sem_ir` can't use
check). So instead, make a `Check::LookupClassFieldByStructField` that
wraps the `SemIR` version, and does the `LoadImportRef`
Assisted-By: Claude
---------
Co-authored-by: Christopher Di Bella <cjdb.ns@gmail.com>
A `where` expression can have symbolic references to `.Self` inside it,
but those references do not represent symbolic instructions that should
be part of an enclosing generic eval block. They are internal details of
the facet type, and as such they are PeriodSelfSymbolic, not Symbolic.
The WhereExpr evaluation is careful to produce a Symbolic constant value
only if it has symbolic dependencies on the LHS of the `where`, which
avoids leaking the symbolic nature of `.Self` into he facet type's
constant value.
But until now, those PeriodSelfSymbolic insts could still leak into the
enclosing eval block if they ended up in an operand of a Call
expression. A compile-time Call expr evaluates to a TemplateSymbolic
phase constant if it's not concrete, which includes PeriodSelfSymbolic,
because we don't want to evaluate the Call expr until the `.Self` gets
replaced during identify. But that means the Call constant value, and
its operands including PeriodSelfSymbolic constants, all end up in the
eval block. We fix this by making Call evaluate to a PeriodSelfSymbolic
phase if its input phase is PeriodSelfSymbolic. This keeps the RHS of
the `where` expression self-contained. If the facet type is concrete,
then it doesn't add any instructions to the enclosing eval block (which
may not exist at all in a non-generic context). Regardless, when we
identify the facet type and replace `.Self`, the dependent Call
expression will be rebuilt as well and it will evaluate as appropriate.
We also enclose the `where` expression in its own generic scope. This
means that instructions which are not part of the facet type do not end
up in an eval block. They may be symbolic instructions, and they will
not get updated by a specific when it's applied the enclosing generic
anymore. But since they are not part of the facet type their
substitution is not required.
The new test had an incorrectly elided SpecificImplFunction instruction
in the resulting semir, as SubstPeriodSelf was using the location of the
input canonical instructions as the location for the results which
resulted in an instruction without a location. We fix that by using
`loc_id_` in the subst rebuild.
Formatting now saw the initialization of a value from an `eval fn` call
to a function in a witness table, so it needed to be taught to look
through ImplWitnessAssociatedConstant (which is what we find in the
witness table).
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>
There were a number of iterations on the design that weren't reflected
in the initial port to the terminal rendering library. This PR tries to
complete the rendering side of things to match the design we landed on:
code-focused, with messages attached by lines to the code, and condensed
as much as possible to avoid the vertical space.
It also improves the styling to make things more visually
distinguishable such as adding a `->` to the main error line, and making
the underlines and connectors for it have a heavy weight (when the font
supports that).
Another major change is having multiple messages on a single line be
drawn in source lexical order. This requires the connectors to cross
without joining, but makes it easier to read the messages.
This PR also has some refactorings to enable these improvements, and a
_number_ of improvements to comment prose and terminology to try and
avoid jargon and make things read more naturally. Sorry for the fact
that Claude left a bunch of this mannerly language in comments despite
my efforts, but I'm continuing to try and drive the prose towards
something direct and simple to read. Some of the terminology
improvements also needed to be made in the design doc and those have
been made here.
Note that I don't think this rendering is perfect or should be viewed as
final. I'm happy to iterate on it further, and have a few adjustments in
mind already, but would like to focus this on getting the
_implementation_ of the current rendering design in place, and then we
can make targeted changes or tweaks as needed. For example, I'm not yet
fully happy with the rendering when messages are too long and we move
them all the way to the left, but am leaving that for a follow-up.
Assisted-by: Claude Code
Add access tests for private fields and methods, in both regular classes
and generic.
Modify `BuildCppToCarbonThunkDecl` to properly set the access rather
than always using public.
Also add `static` to a method that isn't used outside export.cpp.
This test generates an infinite recursion between Carbon and C++ making
a new type at each step through the recursion, producing an infinite
number of types. We improve the test in a few ways:
- Remove direct dependencies on min_prelude parts, we should just depend
on a single min_prelude, and not `import Core` explicitly.
- Remove a dependency on `i32` so we rely on less prelude.
- Remove the use of pointers, which currently introduce a dependency on
`Optional`, so we rely on less prelude. Instead use a generic `Wrap(T)`
type that we nest repeatedly to generate types.
- Add some comments and expand function names to help explain the
behaviour being tested
Now the test just depends on the convert.carbon prelude, which makes it
much simpler when running under a debugger.
Determining that a type has trivial destruction requires checking
whether all of its subobjects have trivial destruction and whether its
type has a custom witness for `Destroy.Op`. This isn't possible when
`Destroy` is the custodian for `Destroy.Op` and `SubobjectDestroy.Op`,
so we need to make `SubobjectDestroy` its own interface.
`Destroy.SelfDestruct` becomes a standalone function because it depends
on `SubobjectDestroy` and `Destroy` being implemented, and is also
responsible for generating any custom witnesses for both interfaces.
Making `SelfDestruct` an interface was experimented with, but this
required a blanket impl that caused issues for facet types.
`Destroy` depends on `SubobjectDestroy` to be implemented. We'd ideally
use `require impls SubobjectDestroy`, but it isn't complete yet, and it
also causes problems when `Destroy` is implemented inside the class
definition. We work around this by temporarily renaming the `Destroy`
interface to `DestroyImpl`, and introducing a constraint called
`Destroy` so that user code doesn't need to write `Core.SubobjectDestroy
& Core.Destroy`.
`FinishAst` resolves C++ template instantiation. Template instantiation
can complete Carbon types, generate thunks, and call generic functions.
These operations can add new entries to `definitions_required_by_use()`.
Similarly, resolving any of those can generate more C++ templates that
need instantiating. This change moves `FinishAst` ahead of
`CheckRequiredDefinitions` and loops until no new Carbon definitions
have been added. This prevents later passes from crashing when
evaluating specifics on unresolved generics or uninstantiated templates.
`FinishAst` also disconnects the AST. Subsequent definitions and `impl`s
may depend on C++ interop, so the AST must remain connected until all
the Carbon end-of-file checks complete. We achieve this by decoupling
AST disconnection from finalisation.
Assisted-by: Antigravity with Gemini
Assisted-by: Antigravity with Claude Opus 5.5 Max
Adds support for exporting both the generic function decl with concrete
default arguments to clang, as well as exporting the monomorphized
concrete function def.
This required adding the `SpecificId` to the exported global variable
name, to avoid duplicate names in the global vars. I also now set up an
evaluated context in `clang_sema` when declaring the default arg
expression with clang, to avoid the problem of emitting concrete AST
expressions in an unevaluated context from the in-progress template
function decl context when exporting default values for generics.
Note: Default values for generic parameters of generic functions are not
yet supported.
Drop dependencies on the prelude so it can be debugged with no prelude
import if needed. Ensure the return type of MakeC is used in C++ since
it's part of the imported function signature.
Adds a check for constrained facet types and extracts the type from the
constraints before supplying it for conversion to C++ types. Adds a test
for this functionality.
- Resolve Specifics eagerly during evaluation. Previous import changes
have made deferral unnecessary. This simplifies the code and unblocks
the next item.
- Overload `GetConstantValue` for all ID kinds, rather than having a
separate `GetConstantValueOrPassThrough` function, so that callers don't
have to choose between them.
---------
Co-authored-by: Dana Jansens <danakj@orodu.net>
This allows a generic class to inherit from a virtual C++ class as long
as the subclass doesn't override any methods. See
toolchain/lower/testdata/interop/cpp/class/import/inherit.carbon for a
working example.
The TODO in `CheckCompleteClassType` is now in
`CarbonExternalASTSource::CompleteType`, and only triggers if virtual
function thunks need to be defined.
A fix in `FileContext::BuildVtable` is also needed: when looking up the
clang decl associated with the vtable, if the class is generic then the
`ClassTemplateSpecializationDecl` associated with the specific is
needed. This required adding `ConstantStore::Get`, since only `const`
access is allowed during lowering.
Adds code to track default values for function parameters in C++ export,
and to export them as global variables to C++ that are attached to
default value expressions in the clang AST.
Modifes lower to export global variables from another block, and adds
the variable initialization to the `global_init` block so their value
gets set at load time.
Split the `Carbon::` namespace into one semantic namespace tree, plus
one separate lookup-only namespace tree per Carbon file. Use the
semantic namespaces as the parents of entities, but use the lookup-only
namespaces when C++ name lookup names a Carbon namespace, in order to
use a different C++ name lookup table in each Carbon file.
Assisted-by: Gemini via Antigravity
When a deduced return type depends on the function's generic parameters,
it's computed by the definition's eval block, so resolve the specific's
definition at the call. Calling a specific during its own instantiation,
before the point where its return type is deduced, is diagnosed like any
other use of the function before its return type is known. Also allow
exporting such functions to C++.
Resolving such a specific from within a checked generic can perform
template actions with symbolic arguments. Perform delayed actions in a
discarded generic region so the instructions they create aren't attached
to the generic being checked, and check that evaluating a specific
doesn't add anything to an enclosing generic region.
Assisted-by: Claude via Antigravity
Address some review comments from the review of PR#7935 on the changes
from PR#7933.
Consolidate `const_cast`s to a single place with an explanatory comment.
Improve test clarity and organization. Rename lookup-specific helper
function to avoid suggesting it is more broadly applicable.
When exporting `X` from Carbon package `Foo`, expose it to C++ with
fully-qualified name `Carbon::Foo::X`, not simply `Carbon::X`. This
ensures that C++ mangled names properly distinguish between same-named
Carbon entities in different packages.
Name *lookup* into `Carbon::` still uses the same rules as before: it
finds what a `package.` lookup would find in Carbon. So `X` is named
from C++ as `Carbon::X` still, and a lookup for `Carbon::Foo` fails,
even though `X`'s "true name" is now `Carbon::Foo::X`.
Assisted-by: Gemini via Antigravity
We allow using `InstId` as a `LocId` for diagnostics, which works great
in `handle_*` functions, where we are working with non-canonical insts.
And convert and pattern matching often are given non-canonical insts,
which they can use, or pull apart to find some other inst, and use those
as a location for diagnostics. However these operations can also be
called with canonical insts, which have no location. In that case if we
need to diagnose an error, it ends up not being attached to any code at
all. Since we always have a `LocId` somewhere up the call stack, pass it
along and in these places that handle canonical instructions too, use
`GetInstLocationOrFallback()` to get a valid location for diagnostics.
This is also similar to what eval does with its fallback `LocId`.
The result is that many diagnostics now have a location where they had
none before. It also means some insts don't get `<elided>` in formatted
semir for not having a location at all and thus not being in range.
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
When unqualified lookup finds a name from an enclosing generic, create a
`specific_constant` if it's a constant, or a `specific_inst` otherwise,
to refer to that name from the inner generic. This allows consumers of
generic instructions to always use a specific of the current scope's
generic when accessing the constant value or type of a dependent
instruction.
Fixes crashes in dominance and in `GetExprCategory` caused by using
specifics for the wrong generic.
Assisted-by: Gemini via Antigravity
Instead of only getting rewrite/equality requirements from the declared
facet type, impl lookup now uses the full IdentifiedFacetType of the
query's constraint to get rewrite/equality requirements from named
constraints as well.
When identifying a facet type, defer evaluation of LookupImplWitnesses
by redirecting them to evaluate into IdentifiedWitness. Then the caller
of identify will need to substitute the IdentifiedWitnesses, either with
witnesses of their own, or with LookupImplWitness to cause a lookup
operation.
This allows:
- An `impl as` on a named constraint can subst the witnesses in
designators from the named constraint with a witness to the impl itself.
This prevents them from looking for and finding some other overlapping
impl. (This PR)
- Impl lookup to collect rewrite/equality requirements from all named
constraints and then use the impls it has already found to evaluate any
designators in those requirements instead of performing impl lookup
again (creating a cycle). (Done in
https://github.com/carbon-language/carbon-lang/pull/7944)
- Similarly, an `impl as` can collect impls and equality requirements
from all named constraints and verify them correctly by replacing
witnesses that should refer to the impl with the correct witness instead
of performing a lookup and finding an overlapping impl. (Done in
https://github.com/carbon-language/carbon-lang/pull/7937)
- ImplWitnessAccess to be able to find rewrites from a named constraint,
not just from the initial DeclaredFacetType. (TBD)
To do this we need to introduce the concept of "identified" specifics.
In order to identify a facet type, we must construct a specific for each
require decl in a named constraint. These specifics will have
IdentifiedWitnesses inside them, but their args would otherwise match a
specific for the require decl constructed in another context. To avoid
polluting the canonical specific namespace, we introduce a second
namespace of specifics that are "identified" since they are created by
identifying.
As we have introduced a new global state that modifies eval behaviour,
we must disable that state when resolving a specific (as per
https://github.com/carbon-language/carbon-lang/pull/7923). But inside an
"identified" specific, we preserve the behaviour so that the witnesses
modified by identify (through SubstPeriodSelf or through evaluating a
require specific) have their evaluation deferred.
`ImportRefResolver` wasn't resolving imported `CustomLayoutType`. The
toolchain would crash due to an unhandled instruction kind whenever a
`CustomLayoutType` crossed a library boundary. This change adds the
necessary infrastructure to properly import `CustomLayoutType`.
Assisted-by: Antigravity with Gemini
If Clang encounters a lookup failure, it caches that result, creating
problems with `-share-cpp-ast` where a lookup failure in one file
effectively poisons the same name when looked up in a different file.
Also, if Clang triggers loading a complete name table (which happens,
for example, during typo correction, but Clang could do that any time it
wants to), it clears out the "has external visible results" flag, which
means that introducing new names in Carbon and then attempting to use
them from C++ after that point does not work.
Fix both of these problems by re-enabling the "has external visible
results" flag on callbacks that indicate Clang just cleared it, and by
not caching failed name lookups into Carbon scopes in Clang's lookup
table. (They still get cached in *our* lookup table.)
Assisted-by: Gemini via Antigravity
Change the `cursor` parameter in `Core.Iterate.Next` to be a `ref`
parameter
instead of a pointer parameter.
Assisted-by: Gemini via Antigravity
---------
Co-authored-by: Geoff Romer <gromer@google.com>
We perform the same canonicalization as in LookupImplWitness, which
makes the output of eval more consistent across witness types, and
enables more straightforward constant comparisons between them when
needed.
As described in proposal #4682, make `array(T, N)` be syntactic sugar
for the type `Core.Array` defined in the standard library. This provides
a home for impls that are specific to `Core.Array`.
Assisted-by: Gemini and Claude via Antigravity
C++ interop used `GetFormatted` to turn a Carbon name into a C++ name.
For an identifier that is a Carbon keyword, such as `base`,
`GetFormatted` adds an `r#` prefix, so the C++ name came out as `r#base`
instead of `base`. This showed up in three places:
- A failed call to the C++ function `base` was reported as "no matching
function for call to 'r#base'".
- An exported Carbon function was mangled as `_ZN6CarbonL6r#baseEv`
instead of `_ZN6CarbonL4baseEv`.
- Its thunk was named `r#base__carbon_thunk` instead of
`base__carbon_thunk`.
All three now use `GetClangIdentifierInfo`, which `export.cpp` already
uses for other C++ names. The name is always an identifier in these
places: an overload set is only imported by looking up an identifier in
C++, and only functions declared in Carbon are exported, which are
always named by an identifier. So they check for an identifier, as the
export of a class name already does, rather than falling back to
`GetFormatted`. This removes a TODO about naming a `NameId::CppOperator`
overload set, which can't happen: C++ operators are resolved in
`operators.cpp` without an overload set.
Assisted-by: Claude Code
C++ pointers to Carbon methods are supported using the same mechanisms
as for Carbon functions: we export the method to C++ and then form a
pointer to the exported method. This change does not support invoking a
C++ method pointer from Carbon, which will be more complicated because
we need to model instance binding on the Carbon side.
Base classes will be destroyed in a dedicated change, so we can
trivially confirm that the base is being destroyed.
This is a partial implementation of #7362.
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
A specific's definition block may contain `InstId::None`, indicating it
has no constant value. Display this as `<not constant>` instead of
`invalid`, to avoid it sounding like an error.
This change introduces a new inst kind `CppFunctionPointerType`, which
represents an imported C++ function pointer that can be invoked from
Carbon (support for forming such a pointer from a Carbon function is in
a follow-up PR). This is implemented by treating the operation of
invoking a function pointer in C++ as if it were a call to an `__invoke`
method on the function pointer type, and extending the existing
function-import logic to support importing this fictitious method.
---------
Co-authored-by: Nicholas Bishop <nbishop@nbishop.net>
The AST doesn't really matter here, since it's NRVO, there's no actual
code to generate to return the value. But having a correct AST does
address at least one clang false-positive diagnostic:
```
error: stack memory associated with local variable 'return_storage' is returned
3 | fn F() -> bool {
| ^
```
The `self` parameter block is presumably a relic from when `self` was in
the implicit parameter list; now it's just SemIR bloat.
This also factors out some common code between `self` and the other
parameters.
We use `LocId`s to refer to physical locations in source code. Those
don't exist for toolchain-generated entities, so we instead choose a
related location that can stand in for a physical location. This inlines
the generated entities' constants into their points of use, and reduces
the total amount of generated SemIR.
This is especially important for calls to `Destroy.SelfDestruct` because
these are automatically generated when any destroyable object reaches
the end its lifetime.
Add support for deferring initialization as a template action, and
performing the deferred initialization during template instantiation.
This is substantially more complex than other conversion actions, for
two primary reasons:
* The initializer in the generic may have storage arguments as inputs.
We model an initializing expression as having a "slot" where
initialization writes the location that should be initialized by that
initializing expression, and that needs to be an output of the
initialization action.
* Initialization from a tuple or struct literal needs to recurse into
that literal, and the literal will have been spelled in the generic,
meaning we don't have an `InstId` that can be used to name the specific
version of the initializer as input for nested conversions.
These issues are addressed by introducing two new features to the action
machinery:
In addition to `InstAction`, we now have `MultiInstAction`, which is an
action that produces a tuple of instruction values instead of a single
instruction value. Initialization actions produce one instruction for
the final result, which is spliced at the point of initialization, plus
one instruction for each storage argument, which are spliced into the
storage argument slots in the original generic. During initialization,
if we find one of those splices in the storage argument of an
initializing expression, we return the new storage argument back to the
initialization action to be included in the specific, instead of
overwriting the storage argument in the generic.
Actions whose `PerforrmAction` takes a `SpecificId` as input no longer
perform automatic refinement of their operands to specific instructions.
Instead, the action is given control over when and where it performs
that refinement. In `InitializeAction`, we use this freedom to form a
`SpecificInst` for the initializer in the primary output block, and form
a `SpecificInst` for the target in the target block. When detecting
whether we are initializing from a tuple or struct literal, we step over
the `SpecificInst` and track its `SpecificId`, and if necessary create a
new `SpecificInst` wrapping the sub-initializer when we recurse into the
nested element conversion.
Assisted-by: Claude and Gemini via Antigravity
Chandler Carruth
Nicholas Bishop
Dana Jansens
David BlaikieandChristopher Di Bella
Christopher Di Bella
Lucile Rose Nihlen
Nicholas Bishop
Geoff RomerandDana Jansens