Generalize the f64 support to support other sizes. Also provide interop
support for `float`, `_Float16`, and `__float128`.
Also lay some groundwork for non-standard floating-point types, though
we don't have any syntax to name them yet.
I noticed while trying to set up an associated constant in the prelude
that we weren't supporting bool value imports; this goes through and
addresses support for simple builtin types.
Array initialization fails on declaration, which seems like a bug but
I'm only documenting it here.
Also fix missing export of `Core.FloatLiteral`
I checked and this doesn't seem to affect #5952, which is doing more
float changes.
The main change here is that a bad type appearing somewhere within a
field or base class of a class shouldn't cause an import of that class
to fail. Instead, only that field or base class becomes inaccessible
from Carbon.
Also improve the way that type importing errors are diagnosed. While we
lose the precision of a diagnostic saying why a type is not supported,
we gain a useful source location for where the type was mentioned in C++
code.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
Don't convert to f64 until we know that's the type that we actually
want. Also reimplement the conversion from RealId to FloatId to perform
an exact conversion with a real check for overflow, rather than
performing an approximate conversion via the host `double` type.
Unfortunately, LLVM doesn't expose its integer mantissa and exponent to
APFloat conversion, so we convert the RealId back to a string for now.
The LLVM conversion also detects overflow only if the literal would
round to having an out-of-range exponent, not if the literal is outside
the range of values of the type as the Carbon design expects. It's not
clear to me which rule we actually want here, so for simplicitly I'm
using the LLVM rule for now.
In preparation for adding other floating-point types beyond f64.
Tidies up extraneous move, unnecessary function style type cast, and
simplifies the temporary directory string construction. These were
noticed during another PR review.
Also corrects support for older glibc versions, including the
GNU-specific quirks of `strerror_r`. Restricts the fancier formatting
with the name of the error number to when a recent glibc is available.
Lastly, filters the benchmarks in the benchmark test down to smaller
ones to avoid test timeout flakiness.
* Rename the type.
* Change lowering to lower FloatLiteralType values as the placeholder
`{}` value we use for literals instead of as an LLVM f64.
* Change eval to convert the type as part of a floating point
conversion, so that lowering can lower converted constants properly.
For now we still represent a value of FloatLiteralType as a
double-precision APFloat. (That will need to change so that we can
losslessly convert literals to f80 / f128 values, and so that we can
convert literals to f32 values without double-rounding.)
We add a virtual node (`CompileTimeBindingPatternStart`) as the first
child of `CompileTimeBindingPattern` which holds the identifier
underneath it, so that it is checked just before the type expression of
the `CompileTimeBindingPattern`. When we reach this virtual node during
check, we add `.Self` as a name in the current scope, and when we reach
`CompileTimeBindingPattern` we remove it from scope, which ensures it's
present during only the checking of the type expression for the compile
time pattern.
At the moment the `.Self` has a different type (it's a `TypeType`) than
other `.Self` in the facet type (which are a single `FacetType`), but
the intention is to immediately substitute it out of the facet type
entirely, replacing it with a reference to the compile time binding (a
`BindSymbolicName`) itself. A TODO has been added for this.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
This removes a bunch of manual filesystem helpers and complexity that
are directly provided by the new library.
It also moves all of the install paths detection to use
`std::filesystem::path` instead of the LLVM path library. The goal is to
consolidate all our logic onto a single stack, and the standard one
seems the best for that purpose. This does give up some of the
optimizations of this code to avoid memory allocation, but in practice
that likely isn't a critical issue. And with the new filesystem library
we can likely do more to avoid that by using directory-object-relative
filesystem access. However, that will have to wait for moving more parts
of the toolchain over to use this set of filesystem abstractions. There
is a related TODO left in the manifest handling code.
The standard filesystem API lacks significant functionality, ranging
from correct and secure creation of directories and files within them by
using `openat` and avoiding [TOCTOU] issues, to support for filesystem
locking.
[TOCTOU]: https://en.wikipedia.org/wiki/Time-of-check_to_time-of-use
The LLVM filesystem library has more functionality, but uses an API that
is increasingly diverging from the standard, and also fails to defend
against TOCTOU.
This library is designed to carefully model the Unix or POSIX filesystem
concepts of `openat` to avoid TOCTOU. However, it also tries to limit
itself to an API subset that LLVM's filesystem library has also
implemneted and so we have a strong reason to expect to be possible to
port to Windows reasonably.
This PR included several benchmarks that show that this implementation
is also faster for the majority of operations than the C++ standard
library. The only places where there is a consistent regression is in
recursively creating directories, and this is directly connected to the
approach of using `openat` as the basis. Even there, while the wall time
regresses, the cycles and instructions are significantly improved.
There are a number of operations not yet included here, I've focused on
a core set of opening, closing, creating, and removing, and then adding
those that I saw the current toolchain code using actively. I'll plan to
expand the operations as needed going forward.
A follow-up PR that I'll finish polishing and send next ports
`//toolchain/install` to consistently use this library and
`std::filesystem::path` to both exercise the library and showcase its
use. I'll be working systematically across the toolchain to converge all
the code, extending this library as needed.
For reference, benchmark results on my macOS laptop:
https://gist.github.com/chandlerc/29d1f4d465a835b8be5174a48dad2e8f
Benchmark results on a Asahi Linux M1 Mac Mini:
https://gist.github.com/chandlerc/c42d43dd6b9b91746ab314b2afa152f7
Benchmark results on a Linux server with weirdly slow FS operations:
https://gist.github.com/chandlerc/48301a7383eb3972d53351b7e35e0561
---------
Co-authored-by: Dana Jansens <danakj@orodu.net>
The self access is important; for the test `generic_class.carbon` being
added to `toolchain/check/testdata/class/destroy_calls.carbon`, it was
using `%T.as.Destroy` instead of `%D.as.Destroy`, indicating the default
blank impl was being used instead of the type-specific version. That
test is trying to focus on the issue, but the delta is visible in a
couple other files in this PR, for example
`toolchain/check/testdata/class/generic/init.carbon`.
I'm separately working on getting rid of the default impl, which is how
I noticed this.
Add missing builtins for float compound assignment, for building a
FloatType, and for converting a float literal to FloatType. Switch
`Core.Float` to being a class and add impls for the various
floating-point operators.
---------
Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
Co-authored-by: Carbon Infra Bot <carbon-external-infra@google.com>
When an error diagnostic has an unattached location, for example because
the diagnostic points into a file that's in the prelude, use the next
attached location to position the error diagnostic's CHECK line. In
particular, if the error is followed by a note, use the position of the
note to determine where to place the error.
This exposes a general mechanism to do final fixups of the CHECK lines
to individual file_test binaries, which the toolchain's binary uses to
special-case error / warning CHECK lines.
With help from Richard Smith debugging/identifying this.
Hmm - looks like maybe the Self type import ref may have the same
problem? (or at least it seems to have the same quirk in the semir dump,
where the inst id is mentioned in the `import_ref` insts, but is not
defined elsewhere, has no name, and says `[no loc]`. I'll look into that
separately. (hmm, maybe this is just an unloaded ImportRef, actually)
When initializing a C++ thunk parameter:
* If we have an initializing expression, materialize a temporary and
pass its address.
* If we have a reference expression, pass its address directly.
* If we have a value expression with a pointer value representation,
pass the pointer.
* Otherwise, create a new temporary and initialize it with a copy of the
argument, and pass its address.
Instead of a bespoke structure based on the `EntityName` in the
`ImplWitnessAccess`' `.Self` type, use the constant value of the
`ImplWitnessAccess` as the map key in `AccessRewriteValues`. This is
okay after #5883 makes the `ImplWitnessAccess` to `.Self` canonically
the same regardless of how it's constructed with nested `where`
expressions.
Introduce `KnownInstId` which tracks in the type system that an `InstId`
is known to refer to a specific typed inst structure. This avoids
writing CHECKs and comments and allows compiler enforcement.
This makes all `.Self` references in a facet type canonically the same
(which will remain true iff they refer to the same `Self` type in the
future), removing the need to do more complex comparisons between them
using the EntityName, interface, and index. This allows the comparison
of types containing `.Self` references to be done correctly regardless
of where the `.Self` appears, as such type expressions will all be
canonically equal if they otherwise equal now, regardless of whether
they are written in the context where `.Self` could have seen different
`Self` facet types.
In order to retain access to constraints on a base `.Self` facet type,
in the case of applying `where` to an existing facet type, we:
- Give the base facet type as a `RequirementBaseFacetType` constraint so
that eval of `WhereExpr` can find and copy all the constraints off of
it.
- Introduce eager/early rewrite constraint resolution, which allows a
constraint to eagerly resolve access to earlier rewrite constraints
(`where .A = () and .B = .A` is eagerly transformed into `where .A = ()
and .B = ()`) before the full constraint resolution step. This allows
use of rewrite constraints in larger type expressions, such as `where .A
= () and .B = C(.A)` and `C` will know that the argument is `()`.
Use it to dump the AST that includes a generated C++ thunk.
Based on #5917.
Also added printing of the full actual text when check fails to make
debugging easier.
Changed line replacement to allow removing complete lines.
Part of #5514.
I was thinking about this for destruction, which I may not be able to
use it for, but still think this may be a good change to keep features
consistent.
toolchain/check/testdata/builtins/char/basics.carbon and
toolchain/lower/testdata/builtins/char.carbon are probably the most
interesting tests here. The parse tests is required because this adds a
new node kind, and we need coverage of it; but the attached info is
minor. There's a fair amount of test churn here because I'm adding the
Core.Char and Core.CharLiteral types as new singletons.
My intent here is that `CharId` is always a unicode code point, even
when the type is a `Char` and thus must be a single UTF-8 code unit
(single byte). This mainly means the stored value of a `CharValue` can
be printed internally without knowing the type.
---------
Co-authored-by: josh11b <15258583+josh11b@users.noreply.github.com>
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Similar to 26ec78ec00 - vtable entries
can't be unattached symbolic constants, because if they are they can't
be `GetValueInSpecific`d because they lack the context for their
generic/specific.
Importing mostly wants to make/import things as unattached constants -
but `ImportRef` supports attached constants, so use those - but we don't
need the laziness, so use `LoadedImportRef`.
When the C++ function has a parameter that is not a pointer and not a
signed integer of 32 or 64 bits, generate a thunk.
Terminology:
* Callee function: The C++ function we actually want to call.
* Thunk function: The C++ function we generated that calls the callee
function.
* A simple ABI type, for now, is one of:
* A pointer
* signed integer with 32 bits
* signed integer with 64 bits
The thunk function is marked `always_inline` and uses the `asm`
attribute to set its mangled to the callee function mangled name
suffixed with `".carbon_thunk"`.
When importing a C++ function, we decide whether calling it requires a
thunk and if so we generate it and import it as well, which is currently
a recursive call.
When calling the thunk function, we initialize a temporary storage for
each non simple ABI parameter type and take its address. This can be
optimized when the variable is already in storage.
Not supported yet:
* Functions with non void return values.
* Member methods.
Moved unsigned int param test from `arithmetic_types_direct.carbon` to
`arithmetic_types_bridged.carbon`, since only signed integers aren't
bridged using a thunk.
C++ Interop Demo:
```c++
// hello_world.h
struct S {
S() {}
S(const S&) { x = 1; }
int x;
};
void hello_world(S s);
```
```c++
// hello_world.cpp
#include "hello_world.h"
#include <cstdio>
void hello_world2(S s) { printf("hello_world2: %d\n", s.x); }
void hello_world(S s) {
printf("hello_world: %d\n", s.x);
hello_world2(s);
}
```
```carbon
// main.carbon
library "Main";
import Cpp library "hello_world.h";
fn Run() -> i32 {
var s : Cpp.S;
Cpp.hello_world(s);
return 0;
}
```
```shell
$ clang -c hello_world.cpp
$ bazel-bin/toolchain/carbon compile main.carbon
$ bazel-bin/toolchain/carbon link hello_world.o main.o --output=demo
$ ./demo
hello_world: 1
hello_world2: 1
```
Before this change (no thunk - copy constructor not called when calling
`hello_world()`):
```shell
$ ./demo
hello_world: -1219172304
hello_world2: 1
```
This adds support for importing C++ code directly from source rather
than via a `#include`.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
In preparation for `FloatValue` being used more generally, and not only
for literals.
---------
Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
This is from a quick discussion with zygoloid. There's a mix of uses,
we're both comfortable with a "minimum syntax" decision here to use ADL.
Note this doesn't touch llvm::cast uses in yaml_test_helpers, but ADL
doesn't work there (because the objects are in a `llvm::yaml`
namespace). This gets to another choice made here: if we specify a
namespace, prefer `llvm::` because it's shorter. `clang` has a using of
them, but it's probably better to point at the canonical version if
we're being explicit about it.
Although this focused on `Destroy` support, some choices here around
`implicit_type_impls` are because copy/move will likely follow a similar
approach. I'm trying not to predict too much about how we'll structure
those, but I'm putting `Destroy` impl logic in a file that could perhaps
be shared with those. They'd likely be interested in similar things,
e.g. traversing members of types (particularly class, struct literal,
tuple literal).
At present this sets the destroy function as `no_op` which is consistent
with current logic, but has a TODO to correctly define.
Constant importing for functions changes slightly due to some issues I
was having with `GetFunctionType`. zygoloid suggested this approach to
avoid `EvalInst` logic.
Adds a flag for controlling whether to generating these impls. While
this does generation for `class`, as noted above this'll also need to be
done for tuples and struct literals, which would leave the `none.carbon`
min_prelude unable to use any types. Note if destruction *would* occur,
it'll still look up `Core.Destroy` for that and fail, but that's already
true of any test using `none.carbon`. I'm trying to use the flag to see
if we can keep `none.carbon` working mostly-consistently.
I'd tried separating out the flag to #5852, but that got a lot of
pushback over whether the behavior was appropriate. I'm hoping that the
interactions here make it clearer why the particular approach -- the
goal is not to enable advanced testing, or create some new end-user
behavior that we really support, it's just to keep no-prelude tests
functional. The main question raised there was why not just keep
generating `impl T as Core.Destroy` if `fn destroy` is present -- but I
think here it should be apparent that would require additional
complexity, as the generation of `impl T as Core.Destroy` is not
currently conditioned based on the implementation of `fn destroy`. I'd
rather add complexity to this flag only if it's enabling interesting
test functionality.
---------
Co-authored-by: Geoff Romer <gromer@google.com>
Add documentation describing the problem and algorithm for coalescing
the LLVM functions generated from Carbon generic functions into fewer
LLVM functions, where the LLVM types permit it.
Adapts `StringLiteral` to lex characters. Adds a `CharLiteral` token,
which contains a `CharLiteralValue` which is a straight unicode code
point (suggested by zygoloid).
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
In order to verify rewrite constraints at the end of
`LookupImplWitness()` we need to replace references to associated
constants in the query facet type with values that come from the query's
self. To do this, we find any `ImplWitnessAccess` that is a reference to
`.Self` and replace its witness with the witness found through the impl
lookup process, if the interfaces match. This allows the
`ImplWitnessAccess` to resolve to a concrete value if that witness was
concrete. Then we just need to compare that for each rewrite constraint
the lhs and rhs are the same constant value. If they differ, the self
provided a different value for one side (either through its own facet
value constraints or through an associated impl), or the self did not
provide a value at all.
For now, only .Self references in the top-level facet type are
rewritten. Nested facet types are not, even if they contain a .Self
reference up to the top level facet value. This will be addressed by
adding numbering to the EntityName of of .Self in a BindSymbolicName.
See the third model in
https://docs.google.com/document/d/1Yt-i5AmF76LSvD4TrWRIAE_92kii6j5yFiW-S7ahzlg/edit?tab=t.0
for the plan. For now, there is a TODO addressing this.
Richard Smith
Chandler Carruth
Dana Jansens
Jon Ross-Perkins
David Blaikie
Boaz Brickner
dependabot[bot]
Alina Sbirlea