For example, this would allow using `std::string::size`, which is
templated because it's a member of the class template
`std::basic_string`, but isn't itself a function template.
This is instead of having one test file that has different return types
and two test files for `i16` and `i32` param tests, which doesn't seem
consistent.
First commit does file renames for easier review.
Part of #5063.
Added test coverage to demonstrate name lookup following import of a
type of a function parameter.
SemIR changes show that the same decl isn't imported multiple times.
Part of #5533.
Create a `self` parameter when importing a C++ non-static member
function. That seems to be all we need to get basic method calls
working! For now, `const` methods have by-value self parameters, and
non-`const` methods get an `addr self: Self*` parameter.
Incidentally also supports import of pointers-to-pointers.
Imported const-qualified types aren't especially useful just yet,
because on the Carbon side we don't yet permit conversions from
non-const to const types, so most of the tests still fail, but for
different reasons now.
Some specific features:
* Use `SpecificFunction` for vtable entries for generic classes.
* Create specific constants for vtable entries in classes derived from
generic classes to reference the appropriate specific of the function
in the context of such a derived class.
* Create specific constants for vtable_ptrs for uses of specific generic
classes.
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
There are some very useful default arguments, so teach the runner script
to directly provide them. They can be easily overridden if needed. As
part of this, change the default run count to 10 which much more often
produces statistically significant error bars and results.
Also, tweak the processing of the results to provide a stable order
based on the source order, even when randomized interleaving is enabled.
The randomized interleaving improves the statistical strength of the
benchmarks significantly, but displaying the results in the source order
is much more understandable. This should give roughly the best of both
worlds.
Direct use of an error means we can delay looking at the actual type,
and also use the `self_id` of the stored `ImplInfo` directly.
Note, I'm assuming we should be okay with this not always being a
`NameRef`. In error cases though, it seems like we shouldn't mind it
being an error instead of a name reference to an error.
In `BuildUnaryOperator`, `GetOperatorOpFunction` is treated as
desugaring, but `PerformCompoundMemberAccess` and `PerformCall` are not.
This treats all of destruction as desugaring.
This leads to some instructions being elided, because of `GetOrAddInst`
behaviors:
> // If the instruction has a desugared location and a constant value,
returns
> // the constant value's instruction ID. Otherwise, same as AddInst.
This changes instructions that previously had a non-desugared location
to instead have a desugared location, so if they also have a constant
value then the constant value can be used directly.
Unify code paths for importing classes by name and importing them
indirectly when their type is referenced. Switch to using the general
type import machinery to import all type declarations, which allows
typedef declarations naming importable types to be used too.
Fix up handling of error cases to consistently only produce an Error
InstId after actually producing an error message, so that we can produce
exactly one diagnostic in failure cases.
Remove TODO error for unions that previously was only produced when
importing them indirectly, not when importing them by name. Import of
unions is exactly as complete / incomplete as import of other class
types, so treating them differently doesn't seem necessary.
This just catches uses equivalent to `return;` and `return <expr>;`.
Note it's just extending the current implicit return logic, not really
adding much unique here.
I'm still delaying break and continue because those require partial
destruction, which is more work and I want to be careful to get it
right.
The other generic `ValueStore` types are in base; this is for
consistency, to make it easier to find. I think it's only in sem_ir for
historical reasons, since it was probably the first bespoke ValueStore
variant added.
Stop using the clang tooling library to build an ASTUnit; that library
is set up to process clang frontend arguments, assuming that something
has already built frontend arguments from the compiler arguments. It is
also too encapsulated and doesn't let us inspect and modify the compiler
invocation before it's executed.
Instead, build the AST unit directly in two phases:
* FIrst, take a list of clang driver arguments and convert them into a
list of compiler arguments, using `clang::createInvocation`. Internally,
this uses the clang driver to build a frontend invocation, including
building system-specific include paths as needed.
* Then, directly build an ASTUnit from this compiler invocation.
I've factored this so that we can split out the `createInvocation` step,
with the intention that we may want to move it out of check and into the
carbon driver with the rest of the driver-level argument handling, and
we may want to customize some of the clang options before we invoke the
clang frontend with that set of options.
In order to make the invocation reusable, it no longer depends on the
name of the carbon file importing the C++ code. In place of synthesizing
a header file name as `<foo.carbon>.generated.cpp_imports.h`, we now
insert line marker directives into the generated header so that errors
in that header cause Clang to point a diagnostic back at the Carbon
source file itself. This results in a minor improvement in the
diagnostic output: we no longer refer to a nonexistent generated file.
But the snippet still contains text that doesn't match the source code,
so it remains imperfect.
---------
Co-authored-by: Chandler Carruth <chandlerc@gmail.com>
When diagnosing a problem with C++ code imported from Carbon, include
the location of the specific `import Cpp` statement that imported the
C++ code as part of the backtrace, rather than providing a location in a
generated file that doesn't exist on disk.
Also fix handling in autoupdate of check lines that contain multiple
file name and line number pairs to use the matched file name for
remapping of locations rather than the first file name in the line.
When mapping parameter types, we assume that if the `clang::Decl` isn't
mapped, the name wasn't added, so this fixes a bug that triggers a crash
otherwise.
Part of #5533.
`SpecificInterface` seems oddly placed. It appears to be in ids.h just
because it's used by typed_insts.h, but maybe that should be factored
differently? We typically aren't having typed_insts.h depend on non-ID
types. To that end, I'm splitting it out to its own file so that at
least I'm not adding a `ValueStore` dep inside ids.h
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Ensure `vtable_ptr`s(and the vtables they refer to) aren't
imported if the type is imported but the vtable isn't
needed (no initialization of a value of that type is required).
Currently if the first operand contains an error, we will return error,
even though the second operands contains a runtime, and it has a
stronger priority (the phase always goes up if possible).
Import is only allowed on instructions with compile-time values, so we
crash if we ever try to import a runtime value. Importable instructions
must diagnose unexpected runtime values and produce errors in the semir
from which they would be imported so that runtime values are never
imported by another semir.
If we had an instruction where you had an error value from the first
operand, and runtime from the second, and we imported it:
- Before https://github.com/carbon-language/carbon-lang/pull/5728 we
would crash in import, but only because we treated errors as runtime
- After https://github.com/carbon-language/carbon-lang/pull/5728 we
would import ErrorInst because we propagate errors. This is desirable
for cases with compile-time values and errors present only.
- After this PR, we would crash again, cuz you're importing a runtime
thing.
This change means that instructions containing an
`InstConstantKind::Never` instruction like`ValueParam` will consistently
evaluate to a runtime value, even if there are errors present. This is
visible in the `BindName` instructions changing in the semir, where they
became constant `ErrorInst` values previously but no longer do.
Also modify CopyOnWriteBlock to just pull block type information from
the return type of the function it receives, rather than taking some as
a parameter.
I chose the `RefType`/`ConstRefType` names based on other similar
`ValueStore` uses which I think are equivalent.
This is reducing ValueStore inference of types from `using`, and removes
`using ValueType = ...` from affected id types.
I'm adding a number of `using FooStore = ValueStore<FooId, Foo>` because
I think it's a little repetitive otherwise; often 4 cases where I'm
doing this: getter, const getter, member, and getter on `Context`. Note
we also have a number of `-> decltype(auto)` that were added I think
mainly to avoid repeating the type, but I'm not sure whether there'll be
agreement on replacing those and so am not changing them here.
I'm placing these aliases with the value type in general, because I
think it's probably easier to view that way. An alternative would be to
put all the types on `File`, but:
- That would be inconsistent with things like `InstStore`, which are
very `ValueStore`-adjacent and put with their value type.
- `File` would have a _lot_ of using's, and the accessors are already
noisy -- I think it would just make the file harder to skim.
Note this is the heart of what I'd brought up [on
Discord](https://discord.com/channels/655572317891461132/655578254970716160/1388199282250613019).
This PR still leaves CanonicalValueStore and BlockValueStore as things
to also add parameters to, but I thought it best to try breaking the set
of changes apart by type. Both of those rely on ValueStore, so
ValueStore needs to change first.
ValueStoreChunk and ValueStoreRange are implemented in a way that's
closely tied to ValueStore, and the separation makes for a lot of
additional template parameter passing, which seems easy to make mistakes
on. Combine types in order to make the close association more implicit.
I also considered passing `ValueT` everywhere, as an additional template
parameter. Note I believe the simplification is important. I'll
highlight four notes that I think favor this approach:
- `ValueStore`, with the chunk type in the same file, now has more of
the closely related implementation features in the same file. I think we
probably will want any chunking to continue to be done by `ValueStore`
itself, with related types using the implementation on `ValueStore` and
never creating their own.
- Making `ValueT` a template parameter on `ValueStore` -- my next step
-- will only change a couple lines of code on this type, instead of
sweeping changes. That should make it easier to be confident of the
correctness of those changes.
- Simpler to verify correctness. For example, `ValueStoreChunk` takes a
`ValueType` parameter that it doesn't forward; other functions assume
they can use `IdT::ValueType`. With the changes, this also no longer
benefits from separating out `IdHasValueType`, which was inconsistently
applied to related types (e.g., `ValueStoreRange` didn't use it).
- Template parameters often lead to `sizeof`, where we can't rely on
type checking to catch mistakes.
- The reduction of code is significant, with 8 `template<...>` removed
(including 1 forward declaration for `ValueStoreRange`), and also the
related `requires`. Correspondingly, places specifying template
parameters also decreased.
As I'm looking at splitting value type setting out, this is to make it a
bit easier to see what's part of each type. Note, I expect
`ValueStoreTypes` to remain because of the `StringRef` logic it does --
I'm giving that its own file.
By moving dumping, we can have dumping occur before verification that
might CHECK-fail (e.g. parse tree and llvm IR verification).
I'm dropping vlogging of raw semir. It was only done when dumping, so
`-v` would print zero copies and `-v --dump-raw-sem-ir` would print two
copies. The lack of complaints about this suggests it's not needed.
I'm making a small change to drop newlines between textual and raw
semir. This is an edge case so I don't expect people to really notice in
general, but it seemed unusually aware of what's on a stream, and it
made it harder to do the dump_stream/raw_dump_stream approach, which I
felt would be decent in general, since check is the only phase that can
emit two different things (which I could also just drop -- we don't
really use raw semir anymore, it doesn't seem like a big need to be able
to print it with textual semir, but I'm assuming to just maintain
existing behavior).
In parse, we were previously dumping the tree on verification errors.
I'm removing that because now `--dump-parse-tree` should work fine,
where previously it wouldn't.
Is it worth having a distinct diagnostic or phrasing for non-class types
(like tuples, structs, pointers, etc), also for declared-but-not-defined
class types (where we can't tell if they're final or not)? Happy to add
it, but not sure how much detail to put in here at this stage at least.
I chose "non-final type" as somewhat vague wording so it sort of applies
even to pointers/tuples/structs.
David Blaikie
Jon Ross-Perkins
Richard Smith
Boaz Brickner
Chandler Carruth
Ivana Ivanovska