This turns `Cpp` into a keyword, and makes it map to `NameId::Cpp` and
`PackageNameId::Cpp`.
Per discussion with zygoloid, the keyword versus identifier question is
deliberately kept open by #4846. This PR switches to a keyword because
mapping to a specific `PackageNameId` works best with a special `NameId`
not backed by an `IdentifierId`. We could in theory make it work using
`IdentifierId` or a runtime-tracked `PackageNameId` for `Cpp` (e.g.
stored on `SemIR::File`), but this approach is consistent with `Core`
and so seemed like a good starting point.
---------
Co-authored-by: Dana Jansens <danakj@orodu.net>
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.
`const` doesn't mean much on the type of a value expression; it's valid
to remove it because we can't perform modifications to a const value
regardless.
We already allowed most of this, but only as part of adapter conversion
rather than in general, and we didn't previously allow it when the
source of the conversion was a reference expression.
---------
Co-authored-by: josh11b <15258583+josh11b@users.noreply.github.com>
This demonstrates two issues:
1. It seems like we wrongly treat private static data members the same
way we treat protected and allow access to them from within derived
classes member functions.
2. Calling instance member functions of a base C++ class using a derived
class as self (no implicit upcast) is not yet supported. This isn't
related to access control, but prevents us from testing some access
control use cases.
Part of #5859.
Implemented by generalizing the reference type support for parameters
and return values to other use cases.
The changes to the `method.carbon` test are due to to supporting the
reference types but not supporting the necessary conversions.
C++ Interop Demo:
```c++
// global.h
struct C {
int member = 0;
int& member_ref = member;
};
extern C& global;
```
```c++
// global.cpp
#include "global.h"
static C static_c;
C& global= static_c;
```
```carbon
// main.carbon
library "Main";
import Core library "io";
import Cpp library "global.h";
fn Run() -> i32 {
Core.Print(Cpp.global->member);
++(*Cpp.global->member_ref);
Core.Print(Cpp.global->member);
++(*Cpp.global->member_ref);
Core.Print(Cpp.global->member);
return 0;
}
```
```shell
$ clang++ -stdlib=libc++ -c global.cpp
$ bazel build toolchain:carbon && bazel-bin/toolchain/carbon compile main.carbon
$ bazel-bin/toolchain/carbon link global.o main.o --output=demo
$ ./demo
0
1
2
```
**Without this change**:
```shell
main.carbon:10:14: error: semantics TODO: `Unsupported: var type: C &`
Core.Print(Cpp.global->member);
^~~~~~~~~~
main.carbon:10:14: note: in `Cpp` name lookup for `global`
Core.Print(Cpp.global->member);
^~~~~~~~~~
```
Part of #6006 and #6186.
The tests are almost identical and test the same logic so basically
duplicated.
The extra coverage that was in `struct.carbon` is added to
`class.carbon`.
One basic test in `struct.carbon` was left just to make sure `struct` is
supported.
Part of #5150.
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 fixes a bug, which seems to have been introduced in #6108.
In the new test, without this change, we will diagnose with
```
error: semantics TODO: `Unsupported: parameter type: ExplicitObjectParam` [SemanticsTodo]
```
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.
This also does a little restructuring in the same direction, following
#6124.
Leads want `Destroy` to work similarly now for all types. As a
consequence, there doesn't seem to be as much benefit to splitting off
aggregate destruction. In this PR, the `type.destroy` function can now
be expected to destroy anything that's destructible; that means it'll be
usable for the `final fn` once that support is available.
Similarly, this gets rid of the impls other than the single blanket
impl, now using `type.can_destroy`. Since they all need to use the same
function, there's no benefit to splitting approaches. Also, now it can
just be a `final impl` since there should be no need for people to
create specializations -- if this blanket impl applies, it means the
`final fn` is the same.
This also slips in `partial` support since there's no reason to have it
diverge anymore. Also `abstract`, which I'm not sure is broadly testable
since most cases it'd come up, the `abstract` keyword is explicitly
detected/rejected.
Note though that this doesn't make any really big changes. It's just
realigning on the leads decision. I'm going this way to try to reduce
name-related churn for other changes.
Locations are similarly fragile, because adding a comment changes them.
This has made me pause when making prelude changes in #6144, so dropping
them for those cases.
Instruction ids aren't actually that interesting outside debugging, and
can be churny when doing other structural changes. I've seen this in
particular when doing singleton changes, which bump every instruction
id.
Note there are still other ways fragility from locations can crop up.
This shouldn't be considered a complete fix, but hopefully a small
improvement.
This adds location information and prevents crashes in some cases of
template instantiation in operator lookup.
Removed `InCppOperatorLookup` note as it is no longer necessary.
Part of #5995.
Right now, the class destroy impl is incorrectly generated (first
discussed [in
Discord](https://discord.com/channels/655572317891461132/941071822756143115/1418614787449032826)).
If we want it to be correct, deferred definition logic would need to be
added, and the declaration would need to be moved inside the `class`
scope (along with whatever generic logic that needs).
This instead switches to a blanket impl, to avoid creating latent bugs
with generating the `impl` and function body in the wrong scope. This
approach uses the same blanket impl as aggregate destruction that was
added by #6098.
The intent here is to allow progress on other parts of `Destroy`. For
example, under this model the implementation of the function body could
be done as part of lowering the specific.
This turns out to be quite important, as several important standard
library types (such as `std::string`) have mixed-access overload sets
for their constructors as an implementation detail. The overall approach
here is:
- Use the most permissive access to determine the access of the overload
set itself. This affects whether name lookup finds the member name at
all.
- After overload resolution, re-check the access of the selected member,
if it's protected or private.
---------
Co-authored-by: Dana Jansens <danakj@orodu.net>
If there's a unique "preferred" base class, then treat that as "the"
base class for Carbon's purposes. In particular:
* If there's exactly one polymorphic base class, that's our preferred
base class.
* If there's exactly one non-empty base class, that's our perferred base
class.
* (Degenerate case) If there's exactly one base class, that's our
preferred base class.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
This diagnoses instead of crashing in some cases:
* When one of the operands is an incomplete Carbon type.
* When one of the operands is a C++ class that can't be completed due to
lack of Carbon supported.
The new tests cover these cases.
Part of #5995.
Don't ignore deleted constructors in overload resolution. If one is the
best match, we want an error rather than picking something else. Don't
crash if we find a constructor template or other weird thing; use
`getConstructorInfo` to map it into a constructor and skip it if it
isn't one, like Clang does.
The general strategy here is to force use of a thunk when we want to use
default arguments, and have Clang generate uses of the default arguments
on its side of the thunk.
To support this, change the key type used in `clang_decls` from being
just a `Decl*` to being a pair of `Decl*` and number of parameters in
the case of function decls. Import distinct `SemIR::Function`s for each
number of parameters that's used, and corresponding distinct thunks.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
If a `BindSymbolicName` is converted to `type` and then to its exact
`FacetType`, we get a `FacetValue` wrapping the `BindSymbolicName` but
providing no different information: it has the same witnesses and
`FacetType` as the original `BindSymbolicName`. Yet it is a different
constant value, creating multiple canonical forms with the same meaning.
Now we make that `FacetValue` with the same `FacetType` as the
`BindSymbolicName` it wraps evaluate back to the `BindSymbolicName`,
making it the unique canonical form.
This makes the "shortcut" in convert for avoiding impl lookup when
converting from `FacetAccessType` to `FacetType` in this exact scenario
work the same as doing the full impl lookup.
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Jon Ross-PerkinsandDana Jansens
Boaz Brickner
Richard Smith
Geoff Romer
Ivana Ivanovska
