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.
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 separates the return type from the return pattern, and replaces the
return pattern with a block of return patterns. This is a step toward
support for `ref` returns (where there's no corresponding return
pattern) and compund-form returns (where there may be multiple return
patterns).
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.
We already did this translation in the other direction, but we had no
mapping from `Optional(T)` to anything, so round-tripping a nullable
pointer from C++ through Carbon and back to C++ was previously rejected.
Don't attempt to defer overload resolution by creating a
`CppOverloadSet`; this was incorrect as we weren't saving the complete
clang::OverloadCandidateSet, resulting in template candidates not being
found. Moreover, saving the overload candidate set would be expensive,
as the representation is surprisingly large, and is unnecessary since
we're about to build a call.
In passing, improve the diagnostics for overload resolution failure to
use Clang's operator overload resolution messages rather than its call
overload resolution messages.
This fixes calls to templated operator overloads, which is the final
piece needed for us to successfully compile an iostream-based "Hello
world" program.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
I was trying to figure out the right way to get specifics to be added to
the work.
Technically, we could keep the pending_specific list; this is taking a
different approach of inserting inside the work stack, which will do
extra work moving entries, although typically that should be expected to
be small. One challenge of `pending_specifics` is that if we would need
to shift them to work after both `Done` (for immediate processing) and
`Retry` (for processing after the current instruction is later revisited
and done). That feels kind of awkward as additional tracking to do.
Also, the common case is probably that there's either 0 or 1 specifics
being added, so an additional vector may be significant overhead. That's
why I leaned more in this direction of just inserting them in the vector
of work.
For now, treat such classes as being final, since we can't correctly
derive from them.
This removes the last category of C++ class that we are entirely unable
to interop with, and is a prerequisite for interop with C++ iostreams
(which have a virtual base class).
This continues work to eliminate pending generics/specifics and get them
to be interleaved with instruction imports. I'm trying to use
`FinishGenericOrDone` here as a way to help ensure that code correctly
handles generics, where the simple alternative would be for each
`TryResolveTypedInst` call `SetGenericData` directly (but which might
make it easier to call the wrong `ResolveResult` function, and we do
need the `GenericId`s to be passed).
Following #6357, map C++ `void` to a prelude class type
`Core.CppCompat.VoidBase`, not to a builtin type. This is mostly just
moving logic around, but does notably change `Cpp.void` from being an
incomplete type to being a complete-but-abstract type.
Also change `NullptrT` to be an adapter for `void*` instead of `()*`, to
follow the approved design.
Implicit conversions to `void` and to `void*` are still absent.
Part of #6280.
Give TupleLiteral and StructLiteral a constant value, if their contents
have constant values. Their constant values are TupleValue and
StructValue respectively. This supports their ability to convert to a
constant type (or facet type).
This way when deduce finds a TupleLiteral as the argument to a
_symbolic_ facet type, it can also find a constant value to use for that
argument. This allows deduction to move onto step two, where it can
substitute into the symbolic parameter from previous deduced arguments,
and then perform the conversion from the TupleValue to the desired facet
type.
Allow `PerformBuiltinConversion()` to convert from a canonical
TupleValue or StructValue to `type` instead of only from literals. Then,
also support conversion from a symbolic binding of type TupleType or
StructType to `type`.
Add a `Core.CppCompat.NullptrT` type that C++'s `nullptr_t` maps into.
Map `nullptr` to an uninitialized constant of that type -- `nullptr`
doesn't actually have any defined bits within it, despite having the
same representation as `void*`.
They are not used for impl lookup or verifying anything yet, but now
they appear in the textual semir.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
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.
If `T` implicitly converts to `U`, then:
* `const T` implicitly converts to `U`,
* `T` implicitly converts to `const U`, and
* `T` implicitly converts to `Optional(U)`.
Adds a flag `--optimize=<mode>` that specifies what to optimize for:
* `--optimize=none` turns off the optimizer as much as possible, but
still respects always_inline.
* `--optimize=debug` aims to be the equivalent of `-Og` / `-O1`, and
provides optimizations that don't affect the ability to debug the
program. This is the default.
* `--optimize=size` optimizes for the size of the produced program, and
aims to be the equivalent of `-Oz`.
* `--optimize=speed` optimizes for the execution time of the produced
program, and aims to be the equivalent of `-O3`.
Following the approach taken by Clang, the optimization level feeds into
both the configuration of the LLVM pass pipeline and the attributes
added to function definitions generated by the frontend.
Optimization is performed in a new phase, `optimize`, which runs between
`lower` and `codegen`.
---------
Co-authored-by: Dana Jansens <danakj@orodu.net>
Co-authored-by: Chandler Carruth <chandlerc@gmail.com>
#6289 absentmindedly added fields in more places, and this is undoing
that plus further fixes.
This does some cleanup of types with relation to singletons. For
`TypeType` and `ErrorInst`, they're always complete due to a
`SetComplete` call in `file.cpp`. For `CppVoidType`, it's intended to be
incomplete by construction, and so a `TypeId` should be okay. The intent
though on not generally providing these had been that `GetSingletonType`
needs to be called to get a type to be marked as complete.
In the case of `AutoType`, removing `TypeId`does change a small printing
detail. I think that's old legacy that's just been carried forward.
Otherwise, for both `InstType` and `AutoType`, I've added
`GetSingletonType` calls where they were used in order to ensure
completeness is applied correctly. These calls cause small SemIR
permutations.
This causes `AutoType` to be seen by lowering, so I'm adding a
placeholder for it. Also merging two functions that look like they're
identical in intent -- not sure why they're separate.
---------
Co-authored-by: Dana Jansens <danakj@orodu.net>
This defines `Cpp.void` as a custom type.
`Cpp.void*` is mapped to C++ `void*`.
Not supported yet: Conversions from and to other pointer types.
C++ Interop Demo:
```carbon
// main.carbon
library "Main";
import Core library "io";
import Cpp inline '''
#include <cstdio>
auto GetPointer() -> void* _Nonnull {
static int x = 8;
return &x;
}
auto GetValue(void* _Nonnull ptr) -> int {
return *static_cast<int*>(ptr);
}
''';
fn Run() -> i32 {
let ptr: Cpp.void* = Cpp.GetPointer();
Core.Print(Cpp.GetValue(ptr));
return 0;
}
```
```shell
$ bazel-bin/toolchain/carbon compile main.carbon
$ bazel-bin/toolchain/carbon link main.o --output=demo
$ ./demo
8
```
Part of #6280.
For now, map C++ reference types to const-qualified Carbon pointer types
rather than picking between a (non-const) pointer or a value type. This
fixes misbehavior in lowering for reference members in classes and
reference return types.
Update the special-case handling for references as function parameters
so that it continues to map const reference parameters to Carbon
pass-by-value, and unify the code paths for `self` parameters and other
parameters, which were mostly doing the same thing but had some subtle
differences.
Add references to the list of types that we can pass to and from C++
directly, without needing an additional layer of thunks.
Remove duplication between determining whether a parameter needs custom
thunk mapping and whether a function needs a thunk. Now a function needs
a thunk if any parameter or the return type does.
This fixes some inconsistencies; previously:
- We would not require a thunk when passing an `unsigned int`, but if we
had a thunk we'd pass `unsigned int` indirectly.
- We would always require a thunk for an enum parameter, even though
we'd actually pass it directly if its underlying type is a 32- or
64-bit integer.
- We would require a thunk for a nullable pointer, even though
we arrange for all pointer types to have the same ABI in Carbon and
C++, including nullable pointers / Optional(T*).
This also causes us to use a thunk for rvalue reference return types,
which we used to miscompile.
Depends on #6276.
Don't go through the `PerformCall` machinery a second recursive time --
this is redundant, creates additional unnecessary temporaries, and is in
theory wrong because `PerformCall` takes a syntactic argument list (one
argument per callee parameter pattern), but we have a call argument list
(one argument per callee parameter).
---------
Co-authored-by: David Blaikie <dblaikie@gmail.com>
This resolves a TODO in `expr_info.cpp` by using the inst kind rather
than the bound value to track the binding's category.
Since we're churning all the `bind_name` insts in testdata anyway, I'm
also taking this opportunity to align the inst naming with the design's
terminology, by calling these insts "bindings" (this aspect of the PR is
dependent on #6231 resolving an ambiguity in that terminology). For
consistency we'll need to rename several other insts as well (see the
TODO on `RefBinding`); I'm deferring that to a separate PR to minimize
the review load, but I think those name changes are in-scope for this
review.
This requires changing `ReturnSlotPattern` and `OutParamPattern`
definitions to use untyped node id, so they can have any associated
node.
Follow up of #5197.
Part of #5064.
When deducing arguments for generic parameters of an `impl`, the
deduction calls `Convert` on the input arguments. Often, the input
argument is a facet, and needs to be converted to a type via
FacetAccessType in order to produce a different facet. These
instructions end up being added to the semir, but only their constant
values are needed for the resulting specific returned from Deduce.
In the best case, these extra instructions are just noise in the semir,
or they just cause instruction names to get differentiated with larger
suffixes.
In the worst case, these extra instructions contain references to
instructions from a generic context, and leak them out of that generic
context and into another. In particular, when importing a
LookupImplWitness instruction, the re-evaluation of it can do deduce
(when the lookup is against a generic `impl`). The instructions created
in Deduce are not part of the import, and end up referring to imported
instructions from the local context, which leads to confusion in the
toolchain, and can crash.
The `import_self_specific.carbon` test demonstrates this. It causes the
`I.F` function to be imported from the `I` interface when building the
witness table for the `impl`. Doing so imports the specific of `C` which
includes a LookupImplWitness for `Self.Accoc` in `I`. The `Self` is a
BindSymbolicName with generic binding index 0, in `I`. When Convert
creates instructions in the generic `impl forall D`, however, they end
up referencing and including this BindSymbolicName into its eval block.
But the generic binding 0 in the `impl` is a very different thing (a
value of type `E`). This confusion leads to crashes.
This is a follow up of #6082, which added support for reference types,
but not for return types.
C++ Interop Demo:
```carbon
// main.carbon
library "Main";
import Core library "io";
import Cpp inline '''
struct C {
auto Inc() -> void { ++x; }
int x = 0;
};
auto GetC() -> C& {
static C c;
return c;
}
''';
fn Run() -> i32 {
Core.Print(Cpp.GetC()->x);
Cpp.GetC()->Inc();
Core.Print(Cpp.GetC()->x);
Cpp.GetC()->Inc();
Core.Print(Cpp.GetC()->x);
return 0;
}
```
```shell
$ bazel build toolchain:carbon && bazel-bin/toolchain/carbon compile main.carbon && bazel-bin/toolchain/carbon link main.o --output=demo && ./demo
0
1
2
```
**Without this change**:
```shell
main.carbon:19:14: error: semantics TODO: `Unsupported: return type: C &`
Core.Print(Cpp.GetC()->x);
^~~~~~~~~~
```
Part of #6148.
This allows to find the spaceship `operator<=>` when a comparison
operator is not available, and `operator==` when `operator!=` is not
available.
Support added to both lookup and overload resolution, by adding
`OperatorRewriteInfo` and propagating it in `CppOverloadSet`.
In case overload resolution chooses to use an operator which requires
rewriting, we emit a `TODO` since rewriting is not yet supported.
Part of #6170.
The SymbolicBindingType refers to the type value that will be
substituted in for the BindSymbolicName, but holds onto the EntityNameId
from the BindSymbolicName instead of (or in addition to, for now) the
instruction.
The EntityNameId will be used to look in the ScopeStack to find the
witnesses either from the BindSymbolicName instruction, or other
instructions that specify `impls` constraints against the EntityName.
This will allow us to have the `T` in `I(T)` resolve to a `.Self`
reference in the type so that we get type equality with the binding's
type: `T:! I(.Self)`.
Before this change, we wrongly ignore the decision to generate a thunk
for a function with default args by overriding this decision with the
fact the return type by itself doesn't require a thunk.
This causes not generating a thunk which leads to crashing in lowering.
Add tests that show that now thunk is generated in `check` and it no
longer crashes in `lower`.
Follow up of #6108.
After this change, we correctly increment the offset by the next switch
case type.
Before this change, we accidentally incremented the offset by
`functions()` size instead of `cpp_overload_sets()` size and vice versa.
Also sorted the switch cases according to the order of the enum, for
consistency. This might help prevent a future similar incident.
This fix prevents crashing in the newly introduced test
`multiple_too_few_args_calls`.
This also has the side effect of showing `null name` for
`cpp_overload_set_type` and `cpp_overload_set_value`, instead of having
an arbitrary name.
Examples that demonstrate the old name is arbitrary can easily be seen
in tests like `cpp_namespace.carbon` and `decayed_param.carbon`, but
careful review would show that all old names are arbitrary, though often
luckily almost make sense.
We might want to have a proper name for these, but it's beyond the scope
of this crash fixing change.
See #6156.
Part of #5915.
Jon Ross-Perkins
Geoff Romer
Richard Smith
Chandler Carruth
Boaz Brickner
Dana Jansens
Ivana Ivanovska
