Fixes link failures when referencing a symbol involving a fingerprint
from a different package.
Previously we included the `Namespace`'s `import_id` as part of its
fingerprint, which caused local and imported namespaces to get different
fingerprints. We now store the `import_id` on the `NameScope` instead of
on the `Namespace` inst to avoid this problem.
Also, when we reach a package-level `NameScopeId`, consistently
fingerprint it as a (package name, library name) pair. Previously the
fingerprinting depended on whether it was imported or not, as an
imported `NameScopeId` had a parent scope (the current package). We need
to include the library name here so that private entities with the same
name in different libraries have different fingerprints.
Provide a source location to the member expression used when a thunk
calls a member function. This ends up being used as the point of
instantiation when the return type triggers a template instantiation;
the absence of this location previously caused assertion failures within
clang.
Assisted-by: Gemini via Antigravity
Rearrange the file_test infrastructure so that we can customize the
mapping of file names to command line arguments. Map `module.modulemap`
files to corresponding Clang driver flags. In passing, also clean up the
interface for specifying custom argument replacements so that we don't
build a string map for each file we process, and stop using
`SmallVector::insert`.
Assisted-by: Gemini via Antigravity
The intent is to add visibility into how the fingerprint is computed, so
that fingerprinting issues and mangling collisions can be more readily
understood and fixed.
Assisted-by: Gemini via Antigravity
When a class extends an interface, referring to a member name of the
interface as an unqualified name should refer to the class's
corresponding associated entity value, not to the associated entity
itself. Similarly, in an `impl`, unqualified names of associated
entities should refer to the `impl`'s corresponding value for that
entity.
To support this, we treat `impl`s as `extend`ing their implemented facet
type, and we make lookups into an extended facet type use the `Self`
type of the extending `impl` or `class` if lookup finds an associated
entity. We already did the latter if the extending entity was an
interface; this extends the existing support for these other cases.
Previously, this relied on the subtlety that `-- ` (with the trailing
space) didn't get parsed as a flag. But there is support already for
escaping a leading `-` in a format argument, so use that to make the
code more obviously correct.
Assisted-by: Antigravity with Gemini
First, this makes the Bazel invocations not try to uses curses which
prevents running them with `pre-commit run ... -v` showing the timings
for each check. The curses display overwrote the output.
Second, this fixes the main slowdown I was seeing. Because we passed
_all_ files to the check-build-graph hook and there are large number of
files, pre-commit would run the tool over and over on a subset of the
files. This is especially wasteful as the build graph check already
doesn't do anything with the files, it just checks `//...` on each
invocation. So this just added a (large) constant factor of cost.
Third, this tries to reduce the cost of `fix_cc_deps.py` in the case of
large numbers of files. This still isn't _super_ fast -- but the rest of
the cost is in running the `bazel query` and parsing the output. I tried
switching it to jsonproto and it wasn't any faster. I think this would
need to be in a non-Python language and use `proto` directly to
significantly improve the cost here.
Assisted-by: Antigravity with Gemini
We had source generation support for some time, but needed to get all
the runtimes set up correctly so that standard library headers are
available. Now that this is in place, we can benchmark both languages.
Also fixes a bug in the C++ source generation causing compile failures.
Assisted-by: Antigravity with Gemini
Use `Label` to mark labels that are local to this module. Remove
workspace root when forming manifest. Add explicit import for name that
is not available implicitly in an imported module.
Assisted-by: Gemini via Antigravity
We were incorrectly computing the index of the Clang implicit conversion
corresponding to method arguments. This led to wrong code and a crash in
lowering due to a calling convention mismatch.
Fixes#7224.
Assisted-by: Gemini via Antigravity
This adds the `static` token to the lexer and parses it as a modifier.
In check, `FullPatternStack::Kind::FieldDecl` is now used for both
static and non-static vars. Static vars get treated basically the same
as `NameBindingDecl`s.
Global initialization is used for static var initializers. To make the
necessary stack information available to `pattern_match.cpp`, the
`full_pattern_stack` and `decl_introducer_state_stack` are now popped
later in `handle_let_and_var.cpp`.
In lowering, each class's body is checked for `VarStorage` insts and
lowered the same as global vars.
`@` refers to the current workspace's working copy, but if we want the
command to respect the working copies of other workspaces, we should use
`working_copies()` instead; see
https://docs.jj-vcs.dev/latest/revsets/#functions .
Also remove `all() &` while here, since it is always redundant.
Assisted-by: Google Antigravity
Co-authored-by: Josh L <josh11b@users.noreply.github.com>
In particular, bazel builds would previously fail in
`workspace_status.py` if you didn't have a `.git` with this error:
> ```
> ERROR: <builtin>: BazelWorkspaceStatusAction stable-status.txt failed:
Failed to determine workspace status: Process exited with status 1
> fatal: not a git repository (or any of the parent directories): .git
> ```
Assisted-by: Google Antigravity
---------
Co-authored-by: Josh L <josh11b@users.noreply.github.com>
Co-authored-by: Chandler Carruth <chandlerc@gmail.com>
This allows Clang to correctly generate the vtable for the exported
class.
There's still something wrong with new virtual functions in the Carbon
type (left a TODO) - I thought it might be related to not flagging
the CXXMethodDecl as virtual, but my initial experiments don't seem to
back that up, so I'll look into it further separately.
There's also a test regression due to an virtual (well, abstract
specifically, but I think it'd happen with a virtual one too) function
in an abstract class taking `self` by value being rejected since
the abstract class can't be instantiated. Not sure if this is a correct
change - the test's behavior could be preserved by using `ref self`
instnead of `self` in this function. Is that reasonable/expected? Should
we not require a type to be complete when passing by value if we can
compute the value representation without such completeness?
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
This change avoids situations where a variable might be either a pattern
type or a scrutinee type, depending on the pattern matching state, and
makes it clear that the state only affects which specific is selected.
We were assuming that a nested access meant that we'd resolve the outer
one, then the inner one. But it may be that we can just resolve the
entire nested access together. Previously if we encountered this, it led
to a CHECK failure. Now we correctly resolve it.
The benchmarks themselves aren't really specific to `driver`.
Keeping the source generation near to the primary use case of
benchmarking also seems like a more discoverable location.
I feel a little bad doing this reorganization right after I gave a talk
with links to a bunch of this code, but seems good to reorganize a bit
before doing some work to extend things now that we have full standard
library support for C++ benchmarking and other improvements.
Assisted-by: Antigravity with Gemini
If a facet type contains `.X impls Y` then later references to `.X`
should know that it impls `Y`. This is done through a stack on the
context like for rewrites, where impl lookup can find constraints from
the current `where` expression being checked.
Similarly, if a facet type contains `.X impls (Y where .Z = T)`, then
`.X.(Y.Z) = T` should be available to later constraints in the facet
type for early application of rewrite rules. We add these rewrites to
the rewrite stack when handling the `impls` constraint
While this can only happen when some other error is taking place, we
should handle it gracefully and report a concrete (but unmatchable)
value in the type structure instead of CHECK-failing.
This also fixes passing a value expression to a forwarding reference,
since we currently deduce a `const T&&` parameter in that case.
We were accidentally looking at the type of the thunk parameter (which
is never an rvalue reference) rather than the type of the callee
parameter.
Instead of doing a lookup for the identified facet type from the impl's
self+constraint, we can use `impl.interface` to get the specific
interface being implemented by the impl declaration. This is already the
specific interface returned from the identified facet type. And that
means we no longer have to identify the self+constraint for imported
impls.
This is split out from
https://github.com/carbon-language/carbon-lang/pull/7183#discussion_r3230571153
We replace the non-canonical ImplWitnessAccess instruction with an
ImplWitnessAccessSubstituted instruction containing the RHS of a prior
rewrite constraint in the same facet type when possible, in order to
access the value of the prior constraint. We were doing this only in the
designator access path though, not in the compound member access. Join
these two code paths when they construct the ImplWitnessAccess, and
perform the early access there for both.
As part of this, move functions that seem reasonable to make out-of-line
to a separate `_impl.h` header file that is only included where the
explicit instantiation _definition_ is provided.
By using explicit instantiation we can make these templates behave more
like non-template classes in terms of supporting out-of-line definitions
that don't need to be compiled by every translation unit. The set of
eventual instantiations here is fundamentally known, and there tend to
be headers that define a canonical "leaf" type where it makes sense to
trigger the explicit instantiation.
Where we already had a `.cpp` file to put the explicit instantiation
definition, use it. But in some places we didn't have such a `.cpp` file
so this PR adds those.
This also requires that we have precise constraints on APIs that _can't_
be instantiated for specific argument types, as now we don't do this
lazily.
Combined, this appears to reduce the sum of object file sizes in the
`check` directory by almost 40% (122mb -> 74mb) in my measurement.
My actual goal was to improve compile times, but so far I don't have a
great methodology for measuring these... But the object file size
reduction seems to confirm this is a net win and likely represents a
non-trivial improvement in compile time.
Assisted-by: Antigravity with Gemini
Not every entry in a C++ vtable corresponds to a function that we want
to import. For the holes, leave a `SemIR::InstId::None` in the vtable.
Also mark vtables that extend a C++ vtable as being non-Carbon-native so
we don't try to lower them (and crash on the `None` entries).
In particular, we leave holes for destructors, since we don't have
destructor declarations on the Carbon side that need to override them.
Class vars are still restricted to simple `name: type` bindings, not
full patterns. This is now handled in the check phase instead of during
parsing.
This is in preparation for supporting `static var`.
Given this example function:
```carbon
fn F(T:! Z where .Self imps Y(.Self) and E(.Self) impls X(.Self)) {
T as Y(T);
E(T) as X(T);
}
```
When we identify the facet type of `T`, we replace `.Self` with `T`.
However in the initial loop, the self _is_ `T` so we don't want to
substitute `.Self` instances inside `T` with itself, as that creates
cycles.
When we see a `type impls...` constraint such as `E(.Self)` we now have
a different self-type so we can, and want to, substitute the `.Self` in
it with `T`. This was already being done, unless the RHS of the `impls`
was a named constraint. In that case we forgot that we were in a `type
impls...` constraint, and avoided replacing `.Self`. Now the algorithm
remembers and correctly replaces `.Self` for `type impls named
constraint` constraints in a facet type.
When replacing `.Self` in a facet type we don't want to destroy the
structure of designators in rewrite constraints so that rewrite
constraint resolution and other similar code can still find them. We
were doing this only for the LHS, and for the RHS if it was a standalone
designator, like `.X = .Y` but we want to do this more robustly to also
avoid rewrite the `.Self` in `.Y` in the expression `.X = C(.Y)`. This
allows resolution, and ImplWitnessAccess to recognize the designator and
replace the `.Y` with the RHS value of another rewrite constraint
assigning to `.Y`.
* For Carbon `base class C`, export as a regular C++ class.
* For Carbon `class C`, export with the C++ `final` keyword attribute.
* For Carbon `abstract C`, mark the destructor as pure virtual in cases
where no member function is abstract, or emit an error if the destructor
is not virtual.
To support the final point, mark the destructor of an exported class as
virtual if it overrides a virtual destructor from the base class.
In passing, fix a crash exporting fields if the class has an invalid
base type.
For now, hide `override fn`s from name lookup, so that the base class
version is always used, as the derived-class version does not have its
own vptr entry and so would not do the right thing if a further-derived
class adds a new override. This is implemented via a new access kind of
`Hidden`.
When checking the overriding function, pass in the expected `Self` type
and check the `self` parameter against that; the signature that we
generate for the thunk in the derived class is the base class signature
with the `self` parameter's type changed to the derived class.
When we generate a thunk for a virtual function, the thunk is assigned a
`virtual_index`, and the virtual function itself is not. When the thunk
makes a direct call to the virtual function, recognize this situation by
checking for a `virtual_index`, and perform a non-virtual call if there
isn't one.
Assisted-by: Gemini via Antigravity
When an outer type defines an `extend` relationship to an inner type, we
require that inner type to be complete so that we can know that name
lookup can search both scopes as soon as the outer type is complete.
When doing name lookup, we require the type in which we are looking to
be complete. Then, we recursively add extended scopes, but then also
require each of them to be complete again, which inserts
RequireCompleteType instructions into the block doing lookup.
While these new instructions may differ in terms of their specifics,
they are redundant since we already required the type to be complete,
and specifics can not change the completeness of a type. They are also
problematic because a named constraint or interface can extend a scope
with a symbolic specific, by using `Self` as an argument. This inserts a
symbolic instruction into the block doing name lookup, even though that
block may not be generic.
A nested designator like `.(X.X1).(Y.Y1)` results in nested
ImplWitnessAccess instructions, which can produce cycles in the
toolchain easily when replacing `.Self`.
First, when constructing a facet type like `V:! Z where .Z1 impls (Y
where .Y1 = U)` we substitute replace `.Self` in the nested facet type,
and in this case we replace `.Self` with `.Z1` which contains a `.Self`
of its own. This was coming from us being lazy about replacing `.Self`
in an `impl as` declaration, such as `impl C as Z where .Z1 = .Self`.
The self type is known there, so we can more eagerly replace `.Self` as
we do in a `require impls` declaration. Then the replacement for `.Self`
never comes with a `.Self` that needs to also be replaced. Any resulting
`.Self` would always be the top-level one.
Second, when evaluating ImplWitnessAccess, we were replacing .Self in
the LHS of rewrite constraints, but the `.Self` may itself have a type
that contains rewrite constraints. If one of those rewrite constraints
has nested ImplWitnessAccess instructions, we evaluate the new
ImplWitnessAccess, which again finds rewrite constraints to replace
`.Self` in, and we repeat forever. For this one we just stop replacing
.Self in the LHS of rewrite constraints. Since they are always against
.Self, we can always look in the access facet's type for a value.
While fixing ImplWitness access, also correct the lookup to search
through the types of nested ImplWitnessAccess instructions to find a
rewrite value, since it may find it at any level up to the eventual
`.Self`.
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Previously, we picked a single Carbon parameter pattern for each C++
parameter pattern. This doesn't work well in cases where the Carbon
semantics and the C++ semantics are not perfectly aligned. In
particular, when a parameter is passed by value in C++, that might mean
either pass-by-move (which in Carbon would best be modeled by a `var`
pattern, as no other form of parameter would perform a move) or
pass-by-copy (which in Carbon would best be modeled by a value
parameter, as a `var` parameter would force an extra copy).
After this change, we compute a passing mode for each parameter based on
the implicit conversion sequence from the argument to the parameter as
determined by C++ overload resolution, and use that to determine the
Carbon pattern corresponding to each C++ parameter. This results in
potentially generating multiple different thunks for the same C++
function if it's called in different ways, but we already did that to
handle default arguments and list-initialization. The passing modes are
included in the thunk mangling.
Add a new value store for clang decl signatures, which capture the
information about parameter passing mode as well as the other existing
information about different ways that a C++ function might be imported
to Carbon.
Most of the rules for computing passing modes are the same as before:
const references use pass by value, non-const lvalue references use
pass-by-ref, non-const rvalue references use pass-by-var. But for C++
non-reference parameters, pick between pass-by-value and pass-by-var
based on whether the implicit conversion sequence was effectively
performing a copy. Prefer pass-by-value if either would work and they'd
do the same thing. We still use pass-by-value for const references, even
when the argument is an lvalue and we could pass a reference; we may
want to change this in future.
For virtual functions, we try to pick a worst-case passing mode, as we
can only pick a single signature for what goes in the vtable. Calls to
virtual functions will still use a thunk to C++, allowing variance in
the calling convention at call sites. We don't allow variance in the
overriders as we don't implement support for thunks for virtual
functions yet. We currently use pass-by-value for const reference
parameters here, but that should probably change at some point.
Assisted-by: Gemini via Antigravity
Update the syntax for class (and interface/`impl`) methods to move
`self` into
the parameter parentheses `()` and make the type in its binding optional
(defaulting to `Self`). Introduce the `static` keyword for non-instance
member
variables to indicate static storage. Reflects the decision in leads
issue
[#6931](https://github.com/carbon-language/carbon-lang/issues/6931).
Updates the directly relevant design, but leaves a systematic update of
examples
to a future PR.
Assisted-by: Antigravity with Gemini
---------
Co-authored-by: Geoff Romer <gromer@google.com>
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Any non-extend require decls must be satisfied when the impl definition
starts, but they are not checked. We only check for require decls in the
target interface being impld.
This commit adds support for range-based for loops using C++ types. It's
currently limited to detecting that `r.begin()` and `r.end()` are
available. We should be able to add full support for methods after #7181
is merged.
Support for ADL is still a work-in-progress, and will be added at a
later time.
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Implement the alias rules from proposal #5389, wherein an alias is
permitted so long as the target has a constant value. While that
proposal is not yet accepted, this seems like a reasonable basis for
further iteration, and will be useful for the examples we're currently
pursuing.
Also cleans up redundant code in the `sem_ir` dump methods that I missed
initially. Now we share as much logic as we can for dumping the non-ID
and ID components.
Assisted-by: Antigravity with Gemini
---------
Co-authored-by: Dana Jansens <danakj@orodu.net>
This correctly renders the vtable in SemIR, including allowing overrides
in
Carbon-derived-from-C++ classes.
It doesn't work in lowering because clang walks the methods of the
CXXRecordDecl - and we currently don't export anything into the
CXXRecordDecl's methods (we do export the fields) - so that's next.
This also doesn't teach Clang to affirmatively emit the vtable
regardless of the types use in C++ code - or to have Carbon use the
vtable in an object's initialization.
Roundtrip (export/reimport) class declarations
The remapping was previously implemented using name_scopes, which aren't
created for class declarations, only definitions - causing the reimport
to import a fresh copy of the type that mismatched with the original (as
seen in the test baseline).
By changing the mapping to use the reverse part of the clang_decls
mapping this should generalize better (& we probably should further
migrate to that mapping). Though it did trip over some issue with
exactly which instruction is used as the key in the clang_decls map -
this change moves towards standardizing on the first decl id of the
class as its map key.
We were treating ImplWitnessAccess as a concrete type, but that is
incorrect if its accessing a symbolic type value. This results in
concrete impl lookup queries failing to match a generic impl that is
built with a symbolic ImplWitnessAccess in its type structure, when the
query does not have the equivalent ImplWitnessAccess in its own type
structure.
We need to look in the top level facet being accessed through
ImplWitnessAccess for witnesses, such as in `T:! Z where .Z1 impls Y`
where `T` provides the witness for `T.Z1 as Y`. But we also need to look
in the facet type of the ImplWitnessAccess for witnesses, such as in
`T:! Z` for `interface Z { let Z1:! Y }`, where `T.Z1` provides the
witness for `T.Z1 as Y`.
To support that we give TypeIterator an iteration step for
ImplWitnessAccess before recursing into it, like we do for FacetValue.
While doing this, we make TypeIterator more recursive, by making less
special casing around the step from one inst into the next. Instead of
eagerly finding a SymbolicType, we consistently recurse back into the
big switch statement and have it decide the next iteration step. This
allows it to recurse into instructions like ImplWitnessAccess and
FacetValue in a consistent manner.
Richard Smith
Chandler Carruth
Nicholas Bishop
David Blaikie
josh11bandJosh L
Geoff Romer
Dana Jansens
Christopher Di BellaandRichard Smith
dependabot[bot]