When creating the C++ thunk, make the parameters references if the
corresponding callee parameters are `ref`s.
When creating the Carbon thunk, tag the call arguments as `ref` if the
corresponding callee parameters are `ref`s.
Instead of treating all C++ code as coming from a single synthetic
`CheckIRId`, track the `SemIR::File` associated with each C++ location.
This is necessary since each `SemIR::File` has a distinct `CppFile` and
therefore distinct `SourceLocation`s and `ClangSourceLocId`s.
Assisted-by: Gemini via Antigravity
- A function with a return declaration always has exactly one
`ReturnSlotPattern`, representing the whole return declaration (whereas
previously that was omitted for value and reference returns).
- The `ReturnSlotPattern` always has a subpattern with the same form.
`OutParamPattern` already plays that role for initializing forms, and
`TuplePattern` will play that role for tuple forms. This change
introduces `ValueReturnPattern` and `RefReturnPattern` to represent
value and reference return forms.
- As before, the `ReturnSlotPattern` has a corresponding `ReturnSlot`
that represents the output that is initialized by a `return` statement.
Its structure parallels the structure of the `ReturnSlotPattern`, so we
need `ValueReturn` and `RefReturn` insts that correspond to
`ValueReturnPattern` and `RefReturnPattern`.
This is a step toward supporting generic return forms, where the
`ReturnSlotPattern`'s subpattern may be an action: this change ensures
that evaluating the action for a specific form produces the same SemIR
as if the form were concrete to begin with. More speculatively, this
should simplify the implementation of `return` statements with compound
return forms.
---------
Co-authored-by: Chandler Carruth <chandlerc@gmail.com>
This shouldn't change any functionality, but simplifies (significantly)
the logic in the installed toolchain, and also provides a better
conceptual balance between these.
I've tried to minimize the changes beyond a pure refactoring, but it was
a bit tricky to get everything working so some things have been mixed
in...
Assisted-by: Antigravity with Gemini
This reverts commit 4babfdbf22.
This was a stacked PR that was merged by accident, losing the commit and
description of the base change. Reverting and will re-land
independently.
Without this, basic `bazel test //...` style wildcards would build
a bunch of extra configurations because of gaps excluding things. With
this, the action count of a normal build should be much more reasonable.
The switch from `target_compatible_with` to tagging is based on looking
at what ends up being most idiomatic and easiest -- trying to articulate
the complex and convoluted compatible with restrictions that would avoid
extraneous build configurations was really painful and this seems much
simpler and easier to deploy in a systematic way.
While here, also change the name of a rule that confused me to no end
while debugging this -- the rule that installs a `.bzl` file that
happens to be spelled `carbon_runtimes` is very different from all of
the other "installed carbon runtimes" kind of things in the tree. Adding
the file extension helps make that (much) more obvious.
Assisted-by: Antigravity with Gemini
ImplWitnessAccess can contain a LookupImplWitness instruction as an
operand, which used to be SymbolicOnly but has now become Conditional in
#6915. This opens up the possibility for LookupImplWitness to have a
concrete value, without resolving to a different instruction kind. If
this occurs when it's the operand of an ImplWitnessAccess, and the
access is unable to find a different value to resolve to through the
witness' self type, then the ImplWitnessAccess can also become concrete.
This can happen in particular when:
- You have an ImplWitnessAccess into a `.Self` symbolic in a facet type.
- You convert a concrete type to the facet type.
- The `.Self` is replaced by a concrete type, causing the impl lookup to
be on that concrete type.
- The (now concrete) impl lookup fails to find any impl witness, so it
remains a concrete LookupImplWitness
This results in a diagnostic, as the incoming type does not satisfy the
facet type, but in the meantime we have a concrete ImplWitnessAccess,
which we do not want to crash.
For example in this test:
```carbon
interface I {
let I1:! type;
}
interface J {}
fn F(T:! I where .I1 impls J) {}
fn G() {
class C;
// This identifies `C as (I where .I1 impls J)`, which replaces `.Self.I1`
// with `C.(I.I1)`. This is a concrete lookup since C and I are both concrete,
// but it doesn't find anything as there is no impl.
//
// As such, the call to F fails to deduce a value for T.
F(C);
}
```
This PR splits out the change to ImplWitnessAccess from the larger
change of replacing `.Self` in identified facet types (and comparisons
with identified facet types).
The FunctionDecl created for calling the Carbon thunk now takes a `self`
parameter for non-static methods, and the C++ thunk now passes an extra
argument for that `self` parameter when needed.
The CXXMethodDecl thunk created for calling methods now sets the storage
class appropriate depending on whether the method is static or not.
To reduce the number of parameters being passed around to thunk-building
functions, added a `FunctionInfo` struct and pass that around instead.
This is (far) from robust -- particularly with repeated compiles in the
same address space. But it appears to be sufficient in the short term,
and we already have the relevant TODOs to factor this upstream into
something that we can use here.
This also somehow uncovered a bug in how we were logging failed commands
-- the failed commands are destroyed when we destroy the driver object
(and its diagnostics object), so we simply cannot do the logging _after_
flushing diagnostics. That's probably ok, and just doing this in the
other order makes the code simpler.
Assisted-by: Antigravity with Gemini
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.
Use constant values in code that can work with a canonical value
(`TypeStructureReferencesSelf`).
Give explicit location ids for the full require decl and the constraint
to `ValidateRequire`.
Go directly from `InstId` to `TypeId` in `ValidateRequire`.
Correctly/explicitly handle constraint instructions which are not types
instead of calling `SemIR::TypeId::ForTypeConstant` and hoping for the
best.
Leave a clear spot where we will subst `.Self` out of the contraint.
This takes the bootstrap support that was added and makes it available
under convenient user-facing flags for while we're doing development.
For example, to build a bootstrap compiler and use it to build and run
the tests under `//common/...` you can now use:
```
bazel test --//:bootstrap_stage=1 --//:bootstrap_exec_config=true //common/...
```
This will use the stage1 bootstrap compiler, and it will build that
compiler in the exec config (so it is optimized and the above even works
when cross-building with Bazel).
Assisted-by: Antigravity with Gemini
- Rename `ActionIsDependent` to `ActionIsPerformable` (with negated
meaning), because that name is more concrete and, um, actionable.
- Replace `OperandIsDependent` with `OperandDependence`, which returns a
`ConstantDependence` instead of a bool. We need this additional
generality for handling form actions, where we sometimes need to ask
whether something has _any_ dependence, not just whether it has template
dependence.
Instead, use the inst category to select the right block stack. This
simplifies the API for adding insts, and in subsequent changes it will
enable certain inst kinds like `SpliceInst` to seamlessly function as
either procedural insts or pattern insts.
Instead of allowing lower to pick whatever type layout it desires,
compute the layouts of types as part of completing the type, and make
lower build types that match that representation.
For now we assume that all pointers are 64-bit, since we don't have
access to target information. We allow tail padding reuse for structs
and tuple types (and by extension, for classes, since they use structs
as their object representation), but not for arrays.
In order to build matching LLVM types, we create LLVM packed structs
where necessary, and we insert inter-field padding on the end of the
previous field so that GEP indexes still always match Carbon's
ElementIndexes.
We don't yet use the computed alignment much in LLVM IR generation -- in
particular, `alloca`s, `load`s, and `store`s should probably use the
computed type alignment, but don't.
Assisted-by: Gemini via Antigravity
Noticed this when testing the Carbon toolchain with a more complex
environment, don't have any way to observe this at the moment in Bazel
though.
Assisted-by: Antigravity with Gemini
This worked correctly in the system Clang toolchain, but was not
configured correctly in the Carbon toolchains. The test is designed to
let us cover all of these.
Assisted-by: Antigravity with Gemini
These turn up frequently in real-world code, for example when converting
a mutable global `Cpp.std.string_view` to a `Cpp.std.string`. Only
reject a non-constant call if the callee is `consteval`, not if it's
`constexpr`.
For calling non-`()` functions, the Carbon->Carbon thunk now takes an
extra reference parameter and writes the target function's return value
out to that parameter. (At the SemIR level this is how returns already
work, but adding this extra reference parameter is needed so that the
function is lowered correctly.) The C++ thunk now creates a local
variable to be initialized by the Carbon thunk, and then returns that
value to the original C++ caller.
The runtimes and bootstrap Bazel logic was previously built around
defining custom Bazel platforms constrained with `constraint_settings`.
The use of platforms added significant complexity, including the need to
"save" and "restore" the original platform, and other complexity
stemming from changing the platform as a whole.
This PR switches to use the simpler tool of build settings, and
`target_settings` on the toolchain rather than platform compatibility.
This remove the entire need to save and restore the platform, and also
generally simplifies things.
This PR also fixes some bugs in the bootstrap that were hidden by the
use of platforms, such as the need to carefully manage the different
inputs to the runtimes build so that generated inputs pick up the
correct exec configuration -- the exec transition happened to do this
"automatically", but it seems better to handle explicitly. And it cleans
up an extraneous copy of `carbon_runtimes.bzl` that snuck in somehow.
Assisted-by: Antigravity with Gemini
---------
Co-authored-by: Dana Jansens <danakj@orodu.net>
Allow any type that has a mapping from Carbon to C++ to be exposed to
C++ via name lookup. This also exposes the logic to export Carbon
classes to C++ to apply during type mapping, which gives very slight
support for passing Carbon types to C++ functions from Carbon, but not
really enough to sensibly test yet.
Depends on #7042.
Previously we'd create a *huge* array here as the tagged ID produced a
very large index value, and spend multiple seconds allocating it and
filling it with zeroes the first time `GetCppLocation` was called.
Reduces test runtime from 26s -> 6s wall time, 450s -> 320s total time
on my machine for `-c dbg`.
Instead of exporting a class or namespace each time a new C++ name
lookup discovers it, track that we have exported the entity on its name
scope, and if a new name lookup finds the same entity, produce the same
clang declaration.
`DoVarPreWorkImpl` was provided an incorrect pattern type while trying
to match `var` parameters, which caused the toolchain to crash in
`Convert`. This commit changes `DoVarPreWorkImpl`'s API so it derives
the pattern type from the work item's pattern ID, rather than relying on
an external source.
---------
Co-authored-by: Dana Jansens <danakj@orodu.net>
This makes them part of the identified facet type, and we can see the
constraints as part of stringify and format output.
But this does not do enough to make them useful yet: Any `T impls X`
constraint must contain a reference to `.Self` somewhere. And `.Self`
references do not get substituted, so neither `T(.Self) impls X` and `T
impls X(.Self)` will match against an incoming facet value derived from
an `impl T(U) as X` or `impl T as X(U)`, since `U` and `.Self` are never
the same thing until `.Self` can be substituted.
Now that impl lookup runs into facet values containing `.Self` (a
symbolic binding), such as in `C(.Self)`, we were crashing assuming the
type of `.Self` is a FacetType, but it can be `type` in the case of
`type where C(.Self) impls...`. Instead, use an empty facet type for the
type of `.Self` so it is always a facet. This assists with substituting
other facets into it, without having to insert an extra FacetAccessType.
`MakePeriodSelfFacetValue()` now enforces this requirement.
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Fixes#7031
Also switches the previous symlinks test to be a more full integration
test. While a bit slow, it does seem worthwhile to have something that
tests things end-to-end, both with the prebuilt runtimes and the
on-demand runtimes. This test is already reasonably well separated from
the rest of the toolchain so incremental development shouldn't be
negatively impacted. And since we turned off ASan by default, it isn't
completely infeasibly expensive.
Assisted-by: Antigravity with Gemini
TypeIterator has both SymbolicType and SymbolicBinding and these overlap
in their meaning. Clarify the API by removing SymbolicBinding and just
using SymbolicType for `SymbolicBinding` insts and when they are
converted to `type` to make a `SymbolicBindingType` inst. Add the
EntityNameId to the SymbolicType for when it is available, when the
instruction is just a simple reference to a binding.
---------
Co-authored-by: Chandler Carruth <chandlerc@gmail.com>
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).
The type must be complete to look for a witness for Destroy. Do this
check through type completion rather than just checking to see if the
ClassInfo says the definition is closed, since completing the type has
side effects (resolves the self specific definition).
Then look for whether the class is abstract through the CompleteTypeInfo
instead of just looking at the inheritance type on ClassInfo, like type
completion does.
Last, FacetTypes are trivially destroyed just like TypeType.
We don't yet populate the bases or fields, so the class types show up as
empty classes in C++ for now. But we do allow calls to static member
functions.
This works by generating two thunks, one in C++ and one in Carbon. For
example, given this input:
```c++
// Carbon:
fn Callme(f: f32) {}
// C++:
void F() {
// This will call `Callme__cpp_thunk`
Carbon::Callme(1.0);
}
```
These functions are generated:
```c++
// Carbon:
fn Callme__carbon_thunk(ref f: f32) {
// Call the target function.
Callme(f);
}
// C++:
// C++ declaration for the Carbon thunk.
void Callme__carbon_thunk(float& f);
void Callme__cpp_thunk(float f) {
// Call the Carbon thunk with args passed by reference.
Callme__carbon_thunk(f);
}
```
For now, all arguments are passed by reference, even if they are simple
types like pointers or i32.
Functions with non-void return types are not supported yet.
This splits off the functionality to handle the base facet type,
rewrites, and impls constraints into separate functions.
We use the Context instead of EvalContext throughout, as the goal is to
move this code to EvalConstantInst in time. That means we do not apply
specifics to the functions in the requirements inst block. That is fine
because WhereExpr never evaluates to an WhereExpr, so this instruction
never survives as a constant value long enough to be re-evaluated with a
specific applied to it.
Use the same C++ -> Carbon map for both interop directions, and when
importing an entity from Carbon -> C++, check whether it was originally
a C++ entity and if so return the original.
Assisted-by: Gemini via Google Antigravity
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.
Fix some situations where we'd drop the storage argument when building
an in-place initializing expression. We now guarantee that an expression
with the in-place initializing category always has a storage argument.
Nicholas Bishop
Richard Smith
dependabot[bot]
David Blaikie
Geoff RomerandChandler Carruth
Dana Jansens
rit
Christopher Di BellaandDana Jansens