Fix a bunch of cases where we use the same external name to mean
multiple different things in the same test. We've historically gotten
away with this, but under `--share-cpp-ast`, it becomes an error, at
least if the entity is either defined in, or used from, C++ code.
Assisted-by: Gemini via Antigravity (original change) and Claude Code
(suggested edits in review)
---------
Co-authored-by: Chandler Carruth <chandlerc@gmail.com>
Implementing interface modifiers causes an infinite loop when generating
fingerprints because the witness value generates a fingerprint that's
dependent on something dependent on the witness value. We've debugged
this to the witness table's `elements_id` field.
This hack is a workaround for creating a new block type whose value is
not codependent with its identity.
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Point symbolic witnesses into `.Self` written inside an impl decl at the
impl that is being declared. This is tricky because the impl does not
yet exist. So we use a new instruction `ImplSelfWitness` which _will_ be
replaced by the `ImplWitness` once it becomes available. The
`ImplSelfWitness` acts like a symbolic witness, except it does not
perform lookup, since we know which impl we will get a witness from.
This prevents us from finding other impls when performing lookups into
`.Self` in an impl decl, which produces incorrect/incoherent results.
Instead of tracking the cleanup scope depth on entry to each scope,
track an "ambient" cleanup scope depth that's *after* the destructors of
local variables in that scope. This gets increased to include the
destructors of local variables when we create a name-binding
declaration. Then, when we reach a point where temporaries should be
destroyed, run cleanups that are after the ambient cleanup scope depth
on the stack. This happens:
* At the `;` of a statement expression.
* At the `)` of an `if` or `while` statement.
* After performing the implied `HasValue()` call in a `for` statement.
Per informal agreement with leads, this means we lifetime-extend all
temporaries created in the initializer of a name-binding declaration to
the full scope of that declaration, but that temporaries created in an
expression statement are destroyed at the `;`.
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>
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.
The bulk of this change is changing most pattern insts to be `Always`
rather than `AlwaysUnique` constants, so that they can be wrapped in
`SpecificConstant`s to perform substitution. That then lets thunking
rely much more on `SpecificConstant` wrappers instead of deep-copying
the inst tree with modified types.
This approach to thunking should scale better, particularly as things
like form generics make function signatures more complex, because we can
leverage the existing support for constant evaluation and substitution.
Unfortunately, applying this approach to binding patterns will require
more work; see the TODO near the top of `thunk.cpp` for details.
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Adapters were erroneously satisfying `Core.Destroy` because we were
directly getting the object's representation without consideration for
abstract and adapted types. This change ensures that adapted types'
representations are used instead of the adapter types.
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.
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`.
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
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>
Fixes link failures when referencing a symbol involving a fingerprint
from a different package.
Previously we included the `Namespace`'s `import_id` as part of its
fingerprint, which caused local and imported namespaces to get different
fingerprints. We now store the `import_id` on the `NameScope` instead of
on the `Namespace` inst to avoid this problem.
Also, when we reach a package-level `NameScopeId`, consistently
fingerprint it as a (package name, library name) pair. Previously the
fingerprinting depended on whether it was imported or not, as an
imported `NameScopeId` had a parent scope (the current package). We need
to include the library name here so that private entities with the same
name in different libraries have different fingerprints.
See
[here](https://docs.google.com/document/d/1rWcueFwIfZox6GKVGxiUG4cBzjrZ6djXiIDGyJDtrE4/edit?tab=t.0)
for the design doc.
This also removes the default value of the `result_type_inst_id`
parameter of `HandleAction`, moves it before the action in the parameter
list, and documents it. This solves two problems:
- The default made it easy to forget, leading to unnecessary
`TypeOfInst` instructions.
- When it was present, putting it after the fairly "bulky" action
argument tended to make the callsite harder to read.
Add import/ and export/ under function/. Move most top-level tests to a
new basics/ with subdirectories for `import` directives and `inline
Cpp`. Add subdirectory for primitive type handling. Move all `reverse/`
tests to somewhere else, typically under an `export/` directory.
I split two test files up: constexpr.carbon got split into var/ and
function/ pieces, and reverse/simple.carbon was inlined into
namespace/export.carbon. The rest are just simple renames.
Richard SmithandChandler Carruth
Christopher Di BellaandRichard Smith
Dana Jansens
Özgür T. ÖnsoyandDana Jansens
Geoff Romer
