Instead of creating a CodeGenerator per CppDomain, and then crashing in
lowering when we try to consume the same llvm Module multiple times,
create a CodeGenerator for each CppFile within the domain.
For now, we mulitplex all of Clang's ASTConsumer output to all code
generators, which means that any strong external definitions within a
Carbon file (for example, in an inline `Cpp` fragment) will be emitted
to all output files in the same `CppDomain`, resulting in link errors
due to symbol redefinitions. This will be addressed later. But this
should be sufficient for Carbon compilations in which such symbols are
not defined.
We also don't yet attempt to classify which compilations will need C++
code generation, and instead create a clang `CodeGenerator` for every
Carbon file that has C++ imports. For `carbom compile`, only one Carbon
file will need code generation, and yet we still build multiple
`CodeGenerator` objects in general. Fixing this requires more plumbing
from the driver, and this will also be handled in a follow-up.
Assisted-by: Gemini via Antigravity
We propose a principle that it's always possible to inline the import of
a library from within the same package without changing the meaning of
or diagnostics applied to the code. This is similar to the textual
inlining of an `#include` statement in C++, but is slightly less
general. Cross-package imports place the imported names inside the name
scope of the package, so inlining those necessarily changes the paths
for name lookup.
---------
Co-authored-by: josh11b <15258583+josh11b@users.noreply.github.com>
Co-authored-by: Carbon Infra Bot <carbon-external-infra@google.com>
This isolates the C++ imports in different Carbon files from each other
in `--share-cpp-ast` mode, so that a Carbon file can only see the
portions of the shared Clang `ASTContext` that it actually imported.
Assisted-by: Gemini via Antigravity
When implementing a named constraint, like `impl as N`, any accesses
through `Self` in the named constraint need to get the value of the
associated constant from the impl's witness table. This isn't possible
immediately, since the impl does not even exist until the declaration is
complete. We use the same model as for accesses found directly in the
impl declaration, but applied to the point where accesses in the named
constraint are substituted during identify to point at the impl's self
type. To get there, we need LookupImplWitness instructions in the named
constraint, when re-evaluated during construction of their enclosing
specific, to evaluate to ImplSelfWitness when they are a reference to
the type and interface being implemented.
This reduces object sizes which is desirable for linking speed.
zygoloid did some analysis to determine if any of our code requires RTTI
for `dynamic_cast` here:
https://github.com/carbon-language/carbon-lang/pull/7532#discussion_r3611196721:
> The only thing I found is that libc++ requires dynamic_cast in order
for std::print to correctly write Unicode to terminals on Windows
We use `llvm::print` functionality, not `std::print`, so this doesn't
affect our toolchain.
Note that libc++ and libc++abi are built with RTTI enabled. It is
explicitly allowed to use different compiler flags when building these
libraries even though they share some headers with users of the
libraries, so this does not cause ODR violations.
Inject the name of a macro rather than its contents when computing its
expansion. If the macro refers to itself, it will not expand within its
own body, rather than expanding once.
Switching from `EnterTokenStream` to `EnterToken` exposed that our Clang
preprocessing environment was a little broken -- we reached the end of
the primary source file and starting tearing stuff down before we
actually finished parsing, which we were mostly getting away with before
but aren't any more. Enabled Clang's incremental processing mode to fix
this. This causes Clang to remain in the main source file when it
reaches EOF instead of popping it. This also causes the diagnostics for
invalid `module;` declarations to change, but in a way that seems not
really any worse than before.
Also slightly changes the diagnostics produced from macro expansion
failures. The new diagnostics are a bit more precise -- they now capture
the outermost level of macro expansion -- but we don't do a good job of
rendering the Clang snippet attached to the "in macro expansion" context
note yet, so the context looks a bit weird: we get two different
snippets attached to the same diagnostic.
Start tracking the domain within `CppContext`s instead of having them
duplicate its fields. This allows us to remove the shared ownership of
the clang parser.
---------
Co-authored-by: Christopher Di Bella <cjdb.ns@gmail.com>
In Convert, we require the source value's type to be compete so that we
can look for `base` classes and `adapt` relationships. However these can
only be present in a `ClassType`, so we only need `ClassType`s to be
complete.
Reduce the requirement in Convert to not complete types that are not a
`ClassType`, and which can not contain a `ClassType` as part of their
class.
Ideally we would only _only_ require the `ClassType` itself to be
complete, and only if we're looking for a base or adapt. However lower
depends on us completing all Convert source types that contain a class.
This seems to suggest we're lacking checks for complete types somewhere
else and Convert is making up for it. A TODO has been added. The
`toolchain/driver/testdata/compile/optimize/optimize_debug.carbon` test
is an example that CHECKs due to failing to verify the LLVM module if we
do not compute the complete type of all class-containing types in
Convert.
The critical step this PR is doing is to stop trying to complete a
`FacetType` when converting from a facet. This avoids trying to complete
a named constraint when converting `Self` inside that named constraint.
Doing so causes a cycle when the conversion of `Self` is performed in
eval of an `extend require` decl, since requiring the named constraint
to be complete re-evaluates the `extend require` decl again. A test is
added that crashed in an infinite loop before this change.
It also depends on #7584, which was intended to be an optimization but
is now load bearing. Because converting `Self` leaves an impl lookup
inst behind, and if that inst is re-evaluated inside impl lookup (by
forming a specific of a `require` decl through identify) then we have a
similar cycle.
It's hard to track 2-space indent levels across large vertical gaps, and
it can be hard to suppress the instinct to read the dump as if it were
preorder. This change introduces a new dump mode that addresses both
problems by using box-drawing characters to explicitly represent the
parent-child edges of the tree. This new mode is the default, but the
old behavior remains available with
`--parse-dump-format=yaml-postorder`.
Add a `SpecificID` arg to `Class::GetStructTypeFields`, and use that in
`CalculateCppFieldOffsets`. Also include the specific in the
`clang_decls` lookup. This has no immediate effect since no specific
class fields are being exported yet, but will be useful for exporting
generic classes.
Since we started bootstrapping, our stage-1 toolchain binaries have been
inputs to stage-2 compilations, and to the compilation database logic,
they're indistinguishable from generated sources. This caused us to get
compilation command lines for them, which clangd would try to index by
parsing the binary, and would either run incredibly slowly or crash.
Filter out non-source files from the compilation database.
In passing, also emit the JSON dump without whitespace, which makes the
database a bit smaller and faster for clangd to parse.
Assisted-by: Gemini via Antigravity
---------
Co-authored-by: Geoff Romer <gromer@google.com>
Don't CHECK-fail if the file contains mismatched braces and generates an
unbalanced parse tree.
Properly balance the start and end of symbols. This previously caused
errors to be reported in clients such as VS Code.
Compute a correct range for definitions. Don't assume we can find the
matching `}` for an opening symbol, since some of our definitions are
delimited by a `;` instead.
Assisted-by: Gemini via Antigravity
Pass `-fno-exceptions` when building the toolchain. We do not use
exceptions, so we do not have any try/catch in main, so uncaught
exceptions just unwind and exit. They do not hit our signal handler and
we do not print the stack trace.
Instead of adding a try/catch in main, and redirecting that, we can
build with `-fno-exceptions`. This turns any throw into an `abort()`.
And indeed with that flag, the following code crashes and prints a stack
trace:
```cpp
std::variant<int, bool> a = {1};
std::get<bool>(a);
```
Note that libc++ and libc++abi need to be built with exceptions enabled.
It is explicitly allowed to use different compiler flags when building
these libraries even though they share some headers with users of the
libraries, so this does not cause ODR violations.
Fixes#5225
During impl lookup, for each (generic) impl candidate, we form a
specific for that impl by deducing its generic arguments. Then we
compare the query interface against the impl's specific interface. That
comparison needs the deduced arguments applied to the impl's specific
interface. Previously we were doing this by getting the impl's
constraint facet type with the impl's specific applied (via
`GetConstantValueInSpecific()`) and then identifying that facet type
with the impl's deduced self.
Identify is a fairly expensive operation. It runs subst, trying to
replace `.Self` references. It walks named constraints. It collects
require declarations. We're looking at making it do _more_ in the future
too, including rewrite constraint resolution and collecting rewrite and
same-type constraints. For this reason we have a cache to make it cheap
on the second run, but it's still a very heavyweight operation to
involve in impl lookup, when all we want is to apply the impl's specific
to its target interface.
We almost have all the information we need to avoid the identification
step. We have the impl's specific after deduction. And we have the
SpecificInterface that the impl is targeting in the `Impl` struct. When
we form the specific for the impl itself, we resolve the declaration
block and form new constant values for all instructions in there, but
that does not cover the SpecificInterface that we're storing in the
`Impl` struct. So we add a new instruction to the impl's eval block,
which will be symbolic when the impl is generic and the target interface
depends on a generic parameter. And we store the `InstId` in the `Impl`
struct. This allows us to gets its constant value later with the impl's
specific applied. From that constant value we can then pull out the
SpecificInterface that the impl is targeting.
`abstract fn` was exported to C++ as a plain virtual function rather
than a pure virtual one, so the class wasn't abstract and could be
instantiated from C++.
Abstract functions no longer get a thunk since there is no definition to
call. The tests are prefixed with `fail_` since an abstract class still
errors on `Core.Destroy` regardless.
Exporting class fields in class specifics will require looking up
`ClangDecl`s by the field's `InstId` and the class's `SpecificId`. Add
the `specific_id` to ClangeDecl, and rework the reverse lookup to use a
`Set` with a `KeyContext` rather than a `Map`. The `Lookup` method now
takes an optional `SpecificId` argument, although currently it is always
`None`.
For `VarStorage`, reverse lookup is performed by the pattern `InstId`
rather than the `InstId` of the `VarStorage` itself, so also add
`pattern_inst_id` to `ClangDecl`, and provide a separate
`LookupByPatternInstId` method for reverse lookups. For this lookup, the
`inst_id` part of the key is set to `None`, so only the pattern's
`InstId` is used for lookup.
In preparation for exporting specific classes, remove `fields_exported`.
A class's fields will be exported for each specific, so a single boolean
won't work.
Instead, check in `ExportAllFieldsToCpp` if `clang_decls` already has an
export for this field, and skip if so. Once specifics are supported,
this lookup will use both the field's `InstId` and `SpecificId`.
The above changes aren't quite sufficient, because if a field fails to
be exported, it will keep being attempted on each call to
`ExportAllFieldsToCpp` resulting in multiple errors. Fix this by storing
an invalid `FieldDecl` in `ClangDeclStore` if an error occurs. This lets
`ExportAllFieldsToCpp` know not to reattempt export, and
`ExportFieldToCpp` returns `nullptr` for invalid fields same as before.
It's come up that `carbon_library` better represents a linkage unit
instead of having to closely mirror the Carbon language library concept.
Given that we can have more than one API file in a linkage unit, and
that `srcs` and `hdrs` gives better compatibility with other C++
tooling, this PR switches `impls` and `api` back to `srcs` and `hdrs`,
and fixes up the broken code.
A method declared with `self` does not modify the object, but it was
exported to C++ as a non-const member function, so calling it on a const
reference would fail.
```carbon
class C {
fn Get(self);
}
inline Cpp '''
void F(const Carbon::C& c) {
c.Get();
}
''';
```
```
error: 'this' argument to member function 'Get' has type 'const Carbon::C', but function is not marked const
```
Import already maps `f() const` to `fn f(self)`, and this PR implements
the same behavior for exporting. No ref-qualifier is added, since that
maps to `ref self`, so that is unchanged.
`GetThisArg()` now builds `this` from the method instead of the parent
record, so that it picks up the method's const-qualifier.
Point symbolic witnesses into `.Self` written inside an impl decl at the
impl that is being declared. This is tricky because the impl does not
yet exist. So we use a new instruction `ImplSelfWitness` which _will_ be
replaced by the `ImplWitness` once it becomes available. The
`ImplSelfWitness` acts like a symbolic witness, except it does not
perform lookup, since we know which impl we will get a witness from.
This prevents us from finding other impls when performing lookups into
`.Self` in an impl decl, which produces incorrect/incoherent results.
As a byproduct, this introduces a new top-level directory
`integration_tests` for tests like this.
This also fixes a latent bug with name mangling on Mac.
Debian packages github.com/libunwind/libunwind as `libunwind-dev`, and
packages LLVM libunwind as `libunwind-N-dev`, with no meta-package to
install the latest version of LLVM libunwind. The libunwind-dev is not
built as PIC, so doesn't work in our build setup. So a specific version
of LLVM libunwind must be installed.
Instead of building one Clang `ASTContext` per compilation, the
`--share-cpp-ast` flag causes us to build a single `ASTContext` and
share it across all contexts. One new abstraction is added: `CppDomain`
represents the Carbon-side view of a Clang AST that might be shared
across multiple `SemIR::File`s. This object owns the Clang instance and
the AST.
For now, we have no isolation between the C++ state exposed to different
Carbon compilations, and we have no multiplexing of generated LLVM IR
from C++ into different Carbon compilations, so the mode is not usable
yet. The plan is to keep it behind a flag until it's ready.
Assisted-by: Gemini via Antigravity
`DiagnoseOrphanImpl` used `definition_id`, but #7140 defines the anchor
as the first owning declaration, so a class declared but not defined was
rejected, which is why three cases in `orphan.carbon` were marked
`fail_todo`, and they now pass.
`fail_use_extern_class` no longer errors, `handle_class.cpp` never
passes the `extern library` name into the class entity, so `C` is
treated as locally owned and counts as an anchor. The expected error is
replaced with a TODO in the test, but it should come back once `extern
library` is implemented for classes.
Split the diagnostic emitter into a separate emitter (regietered with
Clang) and listener (registered with the emitter). The purpose of this
split is to make the Clang emitter not depend on the `Check::Context`,
so that we can use it, and hence the same Clang instance, with multiple
`Check::Context`s. A fallback listener is registered to collect and emit
any diagnostics produced while we don't have a `Check::Context`
registered with the emitter.
Assisted-by: Gemini via Antigravity
Fixes a bug in CarbonPreludeBuilder that caused runtimes build
failures due to a incorrectly created output directory.
Also removes some debug printing that I mistakenly checked in
last time.
This makes SubstPeriodSelf more generally useful, with one less gotcha.
Previously calling it with a facet type would just do nothing.
This is possible now because we
- Have SubstOperandsSkipType to make use of
- Have banned constructs which introduce ambiguous .Self, so we don't
need to try avoid finding undesired .Self insts in facet types in other
positions (like in specifics).
This produces prettier, demangled type names, and works when building
with `-fno-rtti`.
Before:
```
Optional N6Carbon5Parse13NodeIdForKindIL_ZNS0_8NodeKind16LibrarySpecifierEEEE: begin
```
After:
```
Optional Carbon::Parse::NodeIdForKind<Carbon::Parse::NodeKind::LibrarySpecifier>: begin
```
Assisted-by: Gemini via Antigravity
As a follow up to #7546 (and see the discussion there), verify our
assumptions that you can't have an object of type facet where the facet
is not symbolic, and then need to find a Destroy witness for the object.
Added method_alias.carbon test so that the class export code in
`ExportNameScopeToCpp` is tested. Refactored `ExportClassToCpp` so that
`ExportNameScopeToCpp` can reuse that code.
Moved the `identifier_info` code in `ExportNameScopeToCpp` into the
namespace block, because the name scope's name ID is not valid for
classes.
Added a call to `CompleteType` for classes exported via
`ExportNameScopeToCpp`, otherwise a "queried property of class with no
definition" assert is later reached (when adding methods) in the call
chain `BuildCppToCarbonThunkDecl` -> `DeclContext::addHiddenDecl` ->
`CXXRecordDecl::addedMember` -> `CXXRecordDecl::data`.
This is in addition to finding a `where` on the RHS of another `where`.
Since a generic binding introduces `.Self`, any `where` expression that
isn't part of a facet type modifying the binding itself would introduce
an ambiguous `.Self`.
Add virtual parse nodes for let, var, and form bindings, which goes
before the type. This allows us to track if `where` appears in the
binding's type. We only need to look for an invalid `where` if any
appeared in the type. We combine these three nodes together into a
single node kind, which requires us to remove the name from it as a
child. We move it up to the Pattern node again, and rename the
PatternStart nodes to PatternTypeStart as they are now located in the
middle of the Pattern nodes, just before the type.
And we only need to thaw `.Self` in generic bindings. Non-generic
bindings can only have `.Self` through a `where` expression, since the
name is not provided otherwise to non-generic bindings. And `where`
expressions thaw their `.Self` independently. So the binding only needs
to thaw a `.Self` that it introduced, which is only for generic
bindings.
Require rewrite and same-type constraints that do not depend on `.Self`
to be satisfied when a facet type is identified, since those constraints
may not be found later.
For a symbolic value with a type being a facet, we need to find a
witness either from the facet's type or from an impl. Custom witness is
for producing a concrete final witness, but there is no such witness in
this case. We should fail to find a witness, and defer to impl lookup to
find the Destroy witness.
Currently it tries to find a witness from the facet type, but it ignores
named constraints, which means it's incomplete at best. But there's no
value in trying to say that the value is trivially destroyable, when it
has a type that impls Destroy. It may not be trivial, and we can't
actually make a witness for it while symbolic.
We checked that requirements inside the impl-as target interface were
satisfied. But we also need to check that requirements coming from the
constraint facet type, or named constraints that it targets, are
satisfied.
Richard Smith
Geoff Romer
Nicholas Bishop
oli-ej
arhwx
Lucile Rose Nihlen
dependabot[bot]
pjmlp