Fix a few API issues. There's also a newly-added file in compiler-rt
that is not supposed to be built by default but is not being excluded
properly by a glob. Added a patch to exclude that and sent
https://github.com/llvm/llvm-project/pull/208861 upstream.
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.
Each file dump now starts with a `; ---` comment and ends with a blank
line. This makes it easier to visually scan the dump for a file of
interest. The comment format is somewhat arbitrary; I chose `---` to
align with the `--- filename.carbon` separator in SemIR dumps, but
without the filename, because that appears on each of the next two lines
already.
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.
Previously, we picked a single Carbon parameter pattern for each C++
parameter pattern. This doesn't work well in cases where the Carbon
semantics and the C++ semantics are not perfectly aligned. In
particular, when a parameter is passed by value in C++, that might mean
either pass-by-move (which in Carbon would best be modeled by a `var`
pattern, as no other form of parameter would perform a move) or
pass-by-copy (which in Carbon would best be modeled by a value
parameter, as a `var` parameter would force an extra copy).
After this change, we compute a passing mode for each parameter based on
the implicit conversion sequence from the argument to the parameter as
determined by C++ overload resolution, and use that to determine the
Carbon pattern corresponding to each C++ parameter. This results in
potentially generating multiple different thunks for the same C++
function if it's called in different ways, but we already did that to
handle default arguments and list-initialization. The passing modes are
included in the thunk mangling.
Add a new value store for clang decl signatures, which capture the
information about parameter passing mode as well as the other existing
information about different ways that a C++ function might be imported
to Carbon.
Most of the rules for computing passing modes are the same as before:
const references use pass by value, non-const lvalue references use
pass-by-ref, non-const rvalue references use pass-by-var. But for C++
non-reference parameters, pick between pass-by-value and pass-by-var
based on whether the implicit conversion sequence was effectively
performing a copy. Prefer pass-by-value if either would work and they'd
do the same thing. We still use pass-by-value for const references, even
when the argument is an lvalue and we could pass a reference; we may
want to change this in future.
For virtual functions, we try to pick a worst-case passing mode, as we
can only pick a single signature for what goes in the vtable. Calls to
virtual functions will still use a thunk to C++, allowing variance in
the calling convention at call sites. We don't allow variance in the
overriders as we don't implement support for thunks for virtual
functions yet. We currently use pass-by-value for const reference
parameters here, but that should probably change at some point.
Assisted-by: Gemini via Antigravity
Also declare them `inline` since we're putting the `always_inline`
attribute on them. Use the `internal_linkage` attribute rather than
`SC_Static` since it's a more precise mechanism and matches what we do
for static member functions in reverse interop (where `SC_Static` means
something else and would not give the function internal linkage).
This is already allowed as a builtin conversion, but the impl allows the
generics system to know about it, so that conversions like
`Optional(T*)` to `Optional(const T*)` are allowed. This in turn allows
a C++ `T*` to be implicitly converted to a C++ `const T*` in Carbon
code.
This is only fixing the decision about *whether* to produce a witness.
Implementation of the witness is still a TODO, though where a body is
generated, it should also precisely reflect where one _needs_ to be
generated.
Note the tests:
- toolchain/lower/testdata/function/generic/import_core_witness.carbon
- toolchain/lower/testdata/function/generic/import_unused_def.carbon
These tests can probably be produced _without_ Core.Destroy, but I found
the essence of them while trying to build //examples with Core.Destroy
and a simpler minimization wasn't striking me.
Assisted-by: Google Antigravity with Gemini
---------
Co-authored-by: jonmeow <jperkins@google.com>
When a `var` is not explicitly given an initializer, initialize it in
one of two ways:
* If its type implements the new interface `Core.Default`, call
`Core.Default.Op` to initialize it.
* Otherwise, if its type implements `UnformedInit`, leave it in an
unformed state. For now, this is always an uninitialized state, but that
will change in the future.
* If neither of those apply, the `var` declaration is ill-formed.
This is a step towards implementing leads decision #6739 and proposals
#257 and #5913.
Assisted-by: Gemini 3.1 Pro via Antigravity
---------
Co-authored-by: Geoff Romer <gromer@google.com>
Roll LLVM to `6811a83c81500ee373adfc0d9978ff9625a4cf1c`.
This includes https://github.com/llvm/llvm-project/pull/183831 which
moved the functionality of `finish()` on `DiagnosticConsumer`s into the
destructors, and removed the `finish()` method. So, our callers to
`finish()` are migrated to cause the destructor to run at that time
instead.
---------
Co-authored-by: Chandler Carruth <chandlerc@gmail.com>
This is related to #6727, but is generally a necessary fix even without
that issue. I'm not adding a specific test of #6727 because it should
also be covered by the tests in #6726.
Assisted-by: Google Antigravity with Gemini 3 Flash
Some module metadata changed - because rather than linking one module
with one module metadata value (eg: PIC Level 0, or unspecified) and one
module with a different one (PIC level 2, in clang) - we use Clang's
Module as-is, no merging required, so Clang's module metadata sticks
rather than being merged with default values from Carbon.
Also tweaked the name we use for Clang's module name so it matches the
carbon file name.
Otherwise the IR changes seem to be just reorderings - C++ interop goes
first, then Carbon, rather than the other way around.
Background:
https://docs.google.com/document/d/1wi85FRiWh4X9A-gCYMVGKR40-q5fM6-3JaSpePk-XCY/edit?usp=sharing
And specifically this work is essentially an alternative to #5543
Clang's code generation is implemented through an ASTListener
(clang::CodeGenerator) that is attached throughout Clang's
parsing/sema/code
generation phases and acts on Clang AST incrementally throughout that
process.
Prior to this patch, Carbon has only created the CodeGenerator during
Carbon's
`lower` phase, missing out on key callbacks that would be made by Clang
during
`check`. Some of these issues were addressed by #6237 and #6483 - but
there were
still remaining cases where the delayed processing lead to missing
functionality.
With #6483 much of the Clang code that made multithreaded complexity of
#5543 is
no longer present, and we have access to the point of ASTListener
registration
so we can register the CodeGenerator there and consume its resulting
llvm::Module during lower.
Examples of some of the bugs this addresses are seen in the linked doc,
and
checked in as tests in this change in
`clang_code_generator_callbacks.carbon`
An indicental bug that's also fixed, and caused all the other test case
churn,
is that the `CodeGenerator` created during `lower` wasn't getting passed
the
Clang `CodeGenOpts` and was creating its own default - so, most notably,
optimization flags were not respected. This meant that the LLVM IR from
Clang
was always -O0 style IR (optnone, no inlinehint, no TBAA, etc). With
this
change, now the Clang IRGen gets the real `CodeGenOpts` and respects
optimization/other flags specified there.
This is only meant to be a rough proof of concept - I'm totally open to
reworking this in any way (even quite substantially) if folks have ideas
about
how this should be implemented most generally/elegantly/etc.
Support an implicit conversion from `T*` to `Cpp.void*` and to `const
Cpp.void*`, and an `unsafe as` conversion in the opposite direction.
In order to support C++ calls taking and returning `void*` (which get
mapped to Carbon `Optional(Cpp.void*)`, also support conversions from
`Optional(T)` to `Optional(U)` if there's a conversion from `T` to `U`.
Fix a bug in `OptionalStorage` for `T*` where its `HasValue` was exactly
backwards.
### Description
Mangling collisions occur when implementing interfaces with generic
parameters. The mangler does not use the specific id, causing the same
symbol `_C[FunctionName].[PackageName]:[InterfaceName].[PackageName]` to
be generated for all of the implementations below:
```carbon
// Generic interface parameters ignored
impl C as I(A)
impl C as I(B)
// Generic class parameters ignored
impl D(A) as I
impl D(B) as I
// Both ignored
impl D(A) as I(A)
impl D(B) as I(B)
```
### Changes
Updated the mangling logic for `SemIR::ClassDecl` and
`SemIR::InterfaceDecl` to include the specific id. Now the mangling
ensures unique symbols for generic implementations using the format:
`_C[FunctionName].[FunctionSpecificId].[PackageName]:[InterfaceName].[InterfaceSpecificId].[PackageName]`.
Closes#6498
Pursuant to recent decisions on #6124, switch `Destroy` to use a
`CustomWitness` for its implementation. Right now this is manufacturing
no-op implementation functions on each lookup, which obviously isn't
ideal but is intended as a first pass. I'm mostly trying to find the
right balance between updating the approach to reflect new decisions,
while still breaking apart work in a way.
The `CoreInterface` logic is intended to build on `CoreIdentifier`
support. We have a number of additional interfaces that require
specialized logic, and that'll extend pretty far with C++ interop, so it
seemed easiest to have a generic function for it. That's what's
replacing the logic inside C++ interop that was doing string comparisons
(which could have already been moved to `CoreIdentifier`, I just missed
it in my first pass).
This adds `CustomWitness` support because the `Destroy` witnesses can be
imported cross-file. `CustomWitness` was previously only used for C++
types, which don't yet support import, which is why that wasn't
previously an issue. The addition of `query_specific_interface_id` is
similarly needed in order to get correct sorting of witness blocks when
imported.
This PR also removes builtin constraint logic (note this is in a
separate commit to help review; it's not a separate PR because it's
difficult to split apart without tests breaking). This had been made
generic with the expectation that destroy, copy, move, and conversions
would all need related support. Under the new decision, we are not going
to do blanket impls and will instead just manufacture a `CustomWitness`
for everything.
A lot of SemIR fingerprints change, but that's probably because the
addition of `Destroy` on core classes is yielding structural changes.
This updates lower/testdata to use _ instead of proper names, in order
to avoid the "unused binding" warnings from #2022 which are being
implemented. These changes do not depend on the implementation which
should make everything easier to review.
See #6460 with part 1 of the implementation. It was split upon request
in order to make reviewing easier, the original state of the PR was
updating hundreds of test cases.
The PR has thus been split, part 2 including test cases changes can be
viewed at
https://github.com/burakemir/carbon-lang/tree/unused_pattern_bindings_p2022_impl_part2
... many tests need to be updated, so it seems best to get those tests
out of the way that are not interesting.
These are not all tests in lower/testdata - a few of them are
interesting in the sense that they cannot use '_' because it leads to
failed redeclaration check. This is exactly the scenario described in
#3763 which requires the 'unused' marker. Those are left untouched here
but are updated in
https://github.com/burakemir/carbon-lang/tree/unused_pattern_bindings_p2022_impl_part2
Instead of naming the root namespace `package` (because it's accessed by
the `package` keyword), change it to use the current package name. Note,
buried in the checksum changes,
`toolchain/check/testdata/package_expr/fail_not_found.carbon`:
```
- // CHECK:STDERR: fail_not_found.carbon:[[@LINE+4]]:16: error: member name `x` not found in `package` [MemberNameNotFoundInInstScope]
+ // CHECK:STDERR: fail_not_found.carbon:[[@LINE+4]]:16: error: member name `x` not found in `Main` [MemberNameNotFoundInInstScope]
```
for:
```
// CHECK:STDERR: var y: i32 = package.x;
// CHECK:STDERR: ^~~~~~~~~
```
I'll leave it to you if you prefer this; the alternative I see is to
just rename `IsCorePackage` to `IsImportedCorePackage`, and/or change it
to a helper that takes a `Context` and does the right thing with
`parse_tree` (which, I need for `Destroy`-related reasons and was my
default approach).
This is a prerequisite for support for interop with C++ template names.
No behavior change here, except that it sadly changes the fingerprinting
for a lot of tests.
This adds just enough debug info for i32/int parameters and return
values, with a path forward for adding DWARF type metadata for other
types.
As it happens, return type information is carried separately from
parameter information:
* Return type information is carried in the `type` of the `DISubprogram`
(as a `DISubroutineType` - which does carry parameter type information
as well, but that's unused when the DWARF is emitted by LLVM)
* Parameter information is carried by `DILocalVariable`s with a non-zero
`arg` value (representing the order of function parameters)
In the absence of locations for the parameters (future work), nothing
would usually keep the `DILocalVariable` live/reachable when emitting
DWARF - so for cases where this can happen (for clang, this happens in
optimized builds where all references to the parameter variable might be
optimized away) the variables can be "retained" in a list on the
`DISubprogram` - achieved by passing `AlwaysPreserve` parameter to
`createParameterVariable` (adds them to a list, then that list gets
attached to the `DISubprogram` when it's finalized later)
For now, any unsupported types are emitted as `void*` (except void
return, which is implemented as void) as a placeholder.
Given this example:
```
import Core library "io";
class MyClass {
}
fn Unsupported(v: MyClass) {
}
fn Ret() -> i32 {
return 42;
}
fn Arg(x: i32) {
Core.Print(x);
}
fn Run() {
}
```
this is the resulting DWARF:
```
DW_TAG_compile_unit
DW_AT_name ("test.carbon")
DW_TAG_subprogram
DW_AT_name ("Unsupported")
DW_TAG_formal_parameter
DW_AT_type (0x00000066 "void *")
DW_TAG_subprogram
DW_AT_name ("Ret")
DW_AT_type (0x00000062 "int")
DW_TAG_subprogram
DW_AT_name ("Arg")
DW_TAG_formal_parameter
DW_AT_type (0x00000062 "int")
DW_TAG_subprogram
DW_AT_name ("Run")
DW_TAG_base_type
DW_AT_name ("int")
DW_TAG_pointer_type
```
And the debugger:
```
(gdb) p Ret()
$1 = 42
(gdb) p Arg(4)
4
$2 = void
```
I'm not sure if there's a way this logic should be merged with the logic
for making the `llvm::Function` type (which the `DISubroutineType`
building code was inspired by/copied from) - since they're done at
different times/places, I don't think there's an easy way to do it in
one pass, but maybe the code can be shared (even if it's run twice) in
some generic `SemIR::Function` type walker.
---------
Co-authored-by: Dana Jansens <danakj@orodu.net>
This helps at least lldb handle calling functions (currently the debug
info describes every function as `void()`, so no parameters or return
values are supported) - seems gdb and lldb both depend on demangling to
varying degrees in C code (marking a function as "prototyped" in C in
DWARF does seem to also address this problem).
Given:
```
fn PrintThree() {
Core.Print(3);
}
```
Before:
```
(lldb) p PrintThree()
error: Couldn't look up symbols:
PrintThree
Hint: The expression tried to call a function that is not present in
the target, perhaps because it was optimized out by the compiler.
```
After:
```
(lldb) p PrintThree()
3
(lldb)
```
If the value representation of `T` is a copy representation, but it
copies all of the bits of `T`'s object representation, then it's OK to
use that as the value representation of `MaybeUnformed(T)` too.
This fixes the behavior of interop with nullable pointers, which are
represented as an adapter of `MaybeUnformed(T*)`, and need to be passed
to and returned from functions on the Carbon / C++ boundary as `T*`s.
Richard Smith
Dana Jansens
Geoff Romer
David Blaikie
Christopher Di Bella
Chandler CarruthandChristopher Di Bella
Jon Ross-Perkinsandjonmeow
Özgür
Burak Emir