Shorthand names are intended to be used in the textual format of
references to instructions in another IR (which can't happen now, but
will be possible in a forthcoming PR). The shorthand name uses the file
name (minus the `.carbon` suffix) as a base, and if that base name is
ambiguous within the compilation, it is given a numerical suffix `.N`,
just like other textual IR names. Shorthand names are prefixed with `$`
in the textual format, to distinguish them from inst and scope names.
For example, a file `foo/bar.carbon` has the shorthand name `$bar`, and
an instruction `@baz.%qux` in that file can be referenced from another
file as `$bar.@baz.%qux`.
This PR adds `toolchain/check/testdata/basics/file_name.carbon` to test
this behavior. All other test changes are just applying this change to
the existing textual SemIR dumps.
---------
Co-authored-by: Dana Jansens <danakj@orodu.net>
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.
- 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>
Treat the function as having a `-> auto` return type. Reject calls to
functions that still have a non-deduced `-> auto` return type, and
rewrite the function to have the actual return type once it is known.
Assisted-by: Gemini via Antigravity
Diagnostic rendering was part of the data model: `Loc::FormatLocation`
and
`Loc::FormatSnippet` wrote plain text to a stream, so no capability,
color, or
terminal width could reach them.
A diagnostic is now one message with labels, contexts, and location
information
attached, and `Diagnostics::Renderer` draws it into a
`Terminal::Buffer`. The
design is in `toolchain/docs/diagnostics_rendering.md` (#7668),
authoring in
`toolchain/docs/diagnostics.md`.
Before:
```
example.carbon:6:3: error: 1 argument passed to function expecting 0 arguments
Run0(1);
^~~~~~~
example.carbon:3:1: note: calling function declared here
fn Run0() {}
^~~~~~~~~~~
```
After:
```
error: 1 argument passed to function expecting 0 arguments
╭─┤ example.carbon:6:3
┆
3 │ fn Run0() {}
· ─────┬─────
· ╰──┤ calling function declared here
┆
-> 6 │ Run0(1);
· ───────
╰────────────
```
`--no-diagnostic-snippets` gives a compact form, `example.carbon:6:3-9`.
File
tests use it; the rendering tests under `toolchain/diagnostics/testdata`
ask for
snippets.
C++ interop diagnostics are data now rather than text Clang drew:
`SemIR::ClangSourceLocId` names a `clang::SourceRange`, and a Clang
location is
marked as the point it names.
`StreamConsumer` moves to a `:renderer` target that `:emitter` does not
depend
on, keeping `common/terminal` out of `consumer.h`. Capabilities come
from the
file behind the error stream, so tests get plain ASCII.
`renderer_fuzzer` draws diagnostics built from fuzzer bytes with every
capability combination, since a shape the renderer doesn't expect must
never be
why the compiler dies while reporting a problem.
TODOs mark the follow-ups: line weights and crossing lines, wrapping
that keeps
a quoted type whole, and ordering diagnostics that share a byte offset.
Also: `IncorrectExtension`'s hint moves into its message, since its note
had no
location of its own, and `strip_noise` in `check_diagnostics.py` blanks
multi-line `#define`s instead of deleting them.
Assisted-by: Claude Code
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
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.
Support for `fn` expressions with the same feature set as `fn`
declarations. In particular, there is no support for captures,
positional parameters, `=>` bodies, or `auto` return types.
Note that this results in two parse nodes for each `=>` token. That's a
bug, but it matches the behavior of function declarations, and will be
fixed in a follow-up change.
Assisted-by: Gemini via Antigravity
Geoff RomerandDana Jansens
Chandler Carruth
Richard Smith