Note, this supports plurals, but doesn't apply it anywhere. I'm mainly
doing that to demonstrate the approach regarding syntax. See
format_providers.h for details.
A small step to virtual functions - adding vtable pointers to the
layout, but not initializing or otherwise using them at this stage.
A few open design questions I'd love feedback on:
* Is this the right/good enough SemIR representation for now? This patch
adds a `is_dynamic` attribute to `SemIR::Class` and populates/flags it
based on the flag of the base class, or if any virtual function is
declared in the class (or, at least that's my intent). Some other
options include:
* Each `Class` could store a `ClassId` (or `TypeId`?) of the (possibly
indirect, possibly self) base class that is the first one that is
dynamic/has a vtable pointer
* Could make the property narrower, like `has vtable pointer` and have
it `true` only on the type that introduces the vtable - then derived
classes would have to walk their base classes to check if they're the
one that needs to define the vtable pointer or not
* Should the vtable be the first element in the type? If there's a
non-dynamic base type, we could have a layout that's `{<non-dynamic base
type>, vtable ptr, <derived members>}`? Derived types would still be
able to uniquely identify where their vtable pointer is just fine... -
and the vtable pointer is, in a sense, a member of that intermediate
type, so it does seem a bit strange to force it to the front - but I
guess it's probably more efficient in some ways?
Open to any other suggestions/advice/thoughts on the direction, etc.
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
The .tar.gz link is currently broken (I think wget used to work, now it
doesn't); not sure if that's deliberate since it's a download page. I'm
hoping the new location remains more reliable.
Note I tried using SourceForge's git directly. That works locally, but
on the action runners it seems to be blocked:
```
fatal: unable to access 'https://git.code.sf.net/p/perfmon2/libpfm4/': Failed to connect to git.code.sf.net port 443 after 5 ms: Connection refused
```
Also propagate the pattern IR along with the pattern-match IR, and use
it where appropriate.
Strictly speaking, some parts of the pattern-match IR are allocated
eagerly, while traversing the pattern's parse tree, but they still
aren't actually emitted until we traverse the associated pattern insts.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
Undoes most of #4347, because of [performance
complaints](https://discord.com/channels/655572317891461132/707150492370862090/1295527235133898772).
With a 30-ish frame stack trace and `-c dbg`, my installed
`llvm-symbolizer` still seems slow (~6s), but the hermetic
`llvm-symbolizer` adds ~4s (i.e., ~10s total). I don't think we can
easily force the hermetic version to build in opt configuration, so I'm
backing it out.
A good first-pass, at least. (abstract adapters are rejected with this
change, though pending further language design discussion)
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
@bazel_tools//tools/cpp:malloc is equivalent and comes from
https://bazel.build/reference/be/c-cpp#cc_binary.malloc. This also
avoids some confusion in the documentation, since while system_malloc
_can_ be used with `malloc`, it has no effect when it's used with
`--custom_malloc`. But, rather than trying to adjust that, maybe we can
just use @bazel_tools//tools/cpp:malloc
Generate declarations of specific functions on demand. Definitions are
not emitted yet, and I'm using a temporary, known-broken scheme for name
mangling.
Here I'm trying to add some simple formatting based on the token kind,
aiming mainly to keep the implementation short for now.
This approach won't generalize to arbitrary structures (e.g., it doesn't
discern between braces for a function body and a struct literal). I
think we'll probably want to build a parse tree and associate parse
kinds with tokens in order to format, additionally doing something less
linear. But my essential goal at present is to just get a
proof-of-concept that the basics can yield something that looks okay.
This is improving the comment production to produce fewer distinct
comments.
At present, comment processing uses strict prefix matching. It either
expects `// ` (with a space) for valid comments, or just `//` (without a
space) for invalid comments that lacked the space.
As a consequence, the following would be three comments:
```
// Comment 1
//
//
// Comment 4
```
This is because a 3-character prefix is used for valid comments. The
prefix switches between lines 1 and 2, and again between lines 3 and 4,
each resulting in a separate comment.
For contrast, this is one comment because only a 2-character prefix is
used:
```
//Comment 1
//
//
//Comment 4
```
That's because all lines lack a suffix space.
Additionally, with SIMD 16-byte boundaries, further splits can occur if
processing needs to transition to non-SIMD.
Here, I'm trying to just address all of this by:
1. Stitching together adjacent comments. Since a lexed comment starts at
the `//` excluding the indent, the delta from the prior comment must be
precisely the indent.
2. Adding support for switching from SIMD to non-SIMD on file
boundaries.
I considered trying to have a separate `//\n` prefix for SIMD processing
of `// `, but I wasn't sure about the tradeoff of doing both at the same
time (in particular, it'd require constructing a string for the
different prefix), thus this stitch approach. This does mean multiple
passes will be required for a typical long comment structure using blank
comment lines to separate paragraphs (for performance reasons, I will
recommend engineers not write comm... nevermind).
---------
Co-authored-by: Chandler Carruth <chandlerc@gmail.com>
Add a new `specific_function` instruction that represents a generic
function plus its deduced argument list as a callee in a function call.
The new instruction can only appear as the immediate operand of a call
instruction, so we give it a builtin placeholder type.
At the end of each file, require definitions for all specific functions
used in that file. Resolve the generic with the argument list to produce
those specific function definitions as needed, and diagnose if the
generic doesn't have a definition available.
A few tests are updated in cases where they declared and used generic
functions but didn't previously provide a function definition.
Note in particular that this fixes an issue where SharedValueStore had
been shared across files, when they should be per-file. This is only
visible when doing multiple compilations in a single test, which was
rare before.
This also moves these tests into the Testing namespace. My memory of the
various namespacing changes is that we'd generally agreed to have tests
in Testing so that we'd see SemIR:: and similar, same as we would in a
lot of the implementation.
In order to format comments, it's helpful if they're tracked. This
tracks them separately from tokens in order to avoid interfering with
parse; it'd be inconvenient if comment tokens could show up in arbitrary
locations, albeit possible to support.
This additionally extracts out the TokenIterator support into a template
in order to generally have it available for IndexBase types. I'm only
adding it for CommentInfo, not sure if we'll want it elsewhere, but this
structure still felt like a good fit.
The locations point to the first instruction in the generic that needed
the relevant constant value or type.
For now, this must makes the formatted SemIR a bit more useful, but in
the future it will also provide locations for diagnostics caused by
monomorphization failure.
Since formatting covers comments, and the CHECK lines are in comments,
it can create recursive behaviors. This introduces AUTOUPDATE-SPLIT as a
way of formally designating a split to exclusively be used for
autoupdate output.
This extracts out the SourceBuffer handling of `-` in order to trivially
share it.
Note this still has a number of TODOs, it's just setting up the
essential subcommand infrastructure, with some tests demonstrating that
it at least does something.
This should substantially reduce the total build size of `-c dbg`
builds, and especially improve cache hits during incremental
development. I'm seeing over 50% reduction total on Linux in just the
raw size of a complete debug build. Even on macOS where we can't use
split DWARF there are substantial reductions.
Note that LLDB and GDB want slightly different flags to have the best
experience with split debug information, and so the build and
documentation have been updated to enable LLDB's flags by default but
provide clear instructions for switching to GDB's flags, and they are
structured so that this can be done persistently for an individual
developer.
This adds deduction in all the cases where we can match the instruction
fields of the parameter against the corresponding instruction fields of
the argument. This handles all current type constants except for struct
types, for which we would want to match by field name.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
Refactor the current function call deduction logic to make it reusable.
Call into it from `impl` deduction. Also build a generic region for the
definition portion of a generic `impl` and substitute into it before
accessing the witness in a specific `impl`.
This is enough to get simple uses of generic `impl`s to work. The main
blocker for more complex cases is that we have very little support for
non-trivial deduction, so while we can deduce `forall [T:! type] T as
I`, we can't deduce `forall [T:! type] C as I(T)` yet.
Instead of stringifying types in the caller in some cases, add new types
to represent:
- `InstIdAsType`: an `InstId` diagnostic argument that represents a type
expression that should be included in the diagnostic
- `InstIdAsTypeOfExpr`: an `InstId` diagnostic argument that represents
an expression whose type should be included in the diagnostic
For these cases, we can produce more user-friendly descriptions of a
type than we can with a canonicalized `TypeId`. Add comments to
discourage using `TypeId` diagnostic arguments when one of the above can
be used, and move over existing uses where it's straightforward to do
so.
Move type stringification code to its own files and out of `SemIR::File`
to make `File` smaller and to further discourage the direct use of the
stringification logic.
Also update type printing to include the `` ` `` delimiters surrounding
the type. The intent is that we will eventually want to include other
information when formatting a type, like Clang does when printing a
typedef (`'string' (aka 'std::basic_string<char>')`), and such
formatting requires that the diagnostic machinery produces the `` ` ``s
itself.
There are a couple of cases where we really want to format valid Carbon
type syntax directly into a diagnostic, rather than an `aka` or similar,
because the diagnostic text includes part of the type itself, for
example: ``"consider using `partial {0}`"``. For such cases, a `Raw`
form of the diagnostic argument types is added: `TypeIdAsRawType` and
`InstIdAsRawType`. In principle we could instead use ``"consider using
`partial {0:raw}`"``, but our diagnostic machinery isn't set up for
that.
---------
Co-authored-by: josh11b <15258583+josh11b@users.noreply.github.com>
Causes the default development (`fastbuild`) Bazel build and test to
both use ASan, minimal optimizations, and produce good backtraces with
source locations.
This also fixes all of the non-host configs that were deeply broken for
the latest releases of Bazel going back quite some time.
The intent of our our Bazel toolchain config was for a minimally
optimized, minimal debug info, and ASan + UBSan configuration to be the
`fastbuild`, or the default development build of the project. This
matches its inclusion of asserts, etc.
At some point quite some time ago, all of this stopped working. Bazel no
longer has a `nonhost` feature. This was disabled a long time ago,
briefly argued to be re-enabled, but has persistently been removed.
However, since then the host and non-host features have been separated
including the compilation mode, and so none of that is needed now.
Instead, we can use a much simpler and more principled approach to all
of the feature configuration now which this PR implements.
However, we added TCMalloc _after_ all of the ASan stuff became broken,
and so we never saw that it is fundamentally incompatible with ASan. So
this PR also reworks how TCMalloc is used to only apply to `-c opt`
builds on Linux, and it also explicitly disables it when using
`--config=asan`. I've not found a convenient way to tie the malloc
library choice to a toolchain feature in Bazel, so this relies on the
config being used rather than the feature in isolation.
Last but not least, with this change `fastbuild` creates substantially
larger output and so this change also passes several new flags to reduce
the size costs. One is a general improvement from outlining ASan
instrumentation. The others are in a special feature as they reduce the
error message quality for two of the more expensive UBSan checks in
favor of small generated code size. Keeping these last two separate
allows disabling this locally if needed to debug a failure.
When profiling, these jumped out as good inline candidates that happened
to be out-of-line, this just moves them inline so that they're
available. I think as much as 10% improvement in check-phase from this,
but I haven't run detailed before/after measurements as these changes
seemed minimally disruptive.
Also switched from `CHECK` to `DCHECK` in one place that seems
especially hot and where the check itself seems reasonable to only do in
debug builds. Left a comment since we rarely need to remove these any
more.
llvm::function_ref (like std::unique_ptr, for instance) already has a
null/empty state, so use that to avoid confusion/duplication of empty
states between optional and the nested function_refs.
This should free up quite a bit of space and even make our builds faster
by avoiding duplicating every output. The syntax for this flag is
counter intuitive, so I've left a comment explaining it.
With this, we now check:
* The left argument to `where` is a facet type
* The right argument of a rewrite (`=`) requirement converts to the type
of the left argument.
* The left argument of an `impls` requirement is a type and the right
argument is a facet type.
No checking is done for `==` constraints yet.
In addition, make the "is facet type" query into its own function and
fix some comments noticed as part of this change.
This change reveals that accessing the members of a facet, like `.Self`,
isn't doing the right thing, and will have to be fixed in a follow-on
PR. Some tests have been adjusted or disabled as a result.
---------
Co-authored-by: Josh L <josh11b@users.noreply.github.com>
Updates `SemIR::Function::GetParamFromParamRefId` to return more
information in the form of a new `ParamInfo` struct. This struct has a
method for getting the `NameId` from the name binding instruction. The
callers previously got it from the `Param` instruction, but the plan is
for that instruction to no longer be associated with a name.
---------
Co-authored-by: Josh L <josh11b@users.noreply.github.com>
Co-authored-by: Geoff Romer <gromer@google.com>
Not sure about error recovery options - can/should we drop the
definition as a means of recovery when building the SemIR? I guess
probably not, so I guess this change is about right.
Phrasing of the error message I'm certainly open to.
This makes the code more resilient to changes in the structure of
parameter insts, and could help avoid bugs by making the
ImportRefResolver's data structures the single source of truth.
According to
https://docs.carbon-lang.dev/docs/design/generics/details.html#adapting-types:
> You can add any declaration that you could add to a class except for
declarations that would change the representation of the type. This
means you can add methods, functions, interface implementations, and
aliases, but not fields, base classes, or virtual functions. The
specific implementations of virtual functions are part of the type
representation, and so no virtual functions may be overridden in an
adapter either.
So, let's check/reject that.
Checking at the end of the class ensures that no matter the order of
methods and adapt statements, the issue will still be correctly
diagnosed.
Richard Smith
Jon Ross-Perkins
David BlaikieandRichard Smith
Geoff RomerandJon Ross-Perkins
josh11bandJosh L
Céline Dedaj