The `//@include-in-dumps` and `//@dump-sem-ir-begin`/`-end` tooling
directives were consumed for their side effects without a comment
record, so the tokens and comments together no longer reconstructed the
source: tooling that re-emits a file from them, such as `carbon format`,
silently dropped the directive lines. Now each recognized directive line
is also recorded as an ordinary full-line comment alongside its side
effect.
Adjacent full-line comments coalesce into one comment record only within
a category, determined by the byte after the `//` introducer: ordinary
comments (whitespace, or the end of the line or file), `//@...`
directives, and invalid introducers. A transition between categories
starts a new record, so a directive next to a comment block is its own
comment, while all the invalid spellings lump together to keep the
diagnostic noise at one per run.
The category boundary also fixes a lost directive: the invalid-comment
bulk skip compared only the `//` prefix, so `//!x` directly above
`//@dump-sem-ir-begin` absorbed the directive line and its side effect
was never recorded. Invalid comment runs now skip line by line (they
start from a diagnosed error, so they are not hot) and stop at a
whitespace or `@` introducer; the SIMD bulk skip handles only ordinary
comment blocks, whose prefix comparison already includes the whitespace
byte.
Nothing outside the lexer and the formatter reads comment records, and
lex dumps do not include comments, so no other behavior changes.
Assisted-by: Claude Code
Carbon currently requires a comment to be the only non-whitespace on its
line. A `//` comment that follows other content on a line, called a
_trailing comment_, is a lexer error. This proposal removes that
restriction, allowing a comment to follow other content on a line.
Everything else about comments is unchanged: a comment still begins with
`//`, still requires whitespace after the `//`, and still runs to the
end of the line. Carbon continues to provide only line comments; no
block or intra-line comments are added.
Three observations motivate the change. First, trailing comments are
well suited to short _annotations_ attached to a specific entity or
value on a line. Second, the lexer design now makes it trivial to lex
trailing comments, and in fact requires extra logic and potentially cost
to reject them. Third, C++ code routinely uses trailing comments, so
allowing them lets Carbon carry the layout of migrated code over
directly, rather than reworking each comment to read well in a different
structure.
Implementation notes (beyond the proposal's design):
Keeping trailing comments cheap to lex required a few supporting
changes, all of which keep the cost off the lexer's hot path:
- The lexer already dispatches `//` to comment lexing wherever it
appears, so classifying a comment as trailing is a single O(1) check of
whether the `//` is the line's first non-whitespace (`start + indent`).
The hot comment path is otherwise unchanged.
- That check relies on each line's recorded indentation being its real
leading whitespace. Multi-line string literals previously recorded the
column where the literal opened for the lines they span; they now record
the true (closing-delimiter) indentation instead.
- Parser error recovery (`SkipPastLikelyEnd`) had relied on that
opening-column indentation to keep tokens following a multi-line string
literal attached to the same construct. It now reconstructs that
relationship directly by consulting the line on which the literal
opened, including when other tokens follow the closing delimiter (such
as `''' + "more"`). This is on the cold recovery path.
- `CommentData` records the trailing bit in the high bit of its length
field, keeping it at 8 bytes.
Assisted-by: Claude Code
`IsRawIdentifier` checked `token_text.starts_with("r#")` and then read
`token_text[2]`, but `starts_with` only guarantees a length of two. An
`r` identifier immediately followed by `#` at the end of the source --
so the token text is exactly `r#` -- made the `token_text[2]` read run
off the end. Guard on the length first.
Found by fuzzing. The read is reached only via `GetTokenText`, so the
parser fuzzer, which does not request token text, never hit it.
Assisted-by: Claude Code
If a keyword or a sized type literal (eg, `f2`) is used in a context
where we are confident that we are expecting an identifier -- either
before a `:` in a binding pattern or after a `.` in a member access or
designator -- then recover as if a raw identifier was used.
This appears to be a particular stumbling block for coding agents, so
seems worth paying special attention to.
Add a mechanism to the tokenized buffer to track additional tokens
synthesized for error recovery so that we can keep the lexed token
sequence immutable and still satisfy the invariants throughout the rest
of the toolchain for recovery tokens. Thanks to chandlerc for suggesting
this approach!
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
Mainly because "sorting_diagnostic_consumer" is legacy, since
`SortingDiagnosticConsumer` became `SortingConsumer`. Also better
reflecting contents of these files.
Where I'm not renaming, I'm less positive about dropping "diagnostics"
from "file_diagnostics" and "null_diagnostics" (which contain both a
consumer and emitter, and "null.h" seems like poor naming), so not doing
that here. Also "diagnostic.h" contains `struct Diagnostic`, so is a
decent fit.
Assisted-by: Google Antigravity with Gemini 3 Flash
Trying to figure out an easy way to debug semir in the prelude, #5703
removed an option to set `--exclude-dump-file-prefix` to empty. But,
this is probably an improvement over that flow... With this change, it's
possible to add `//@dump-sem-ir-file` to a specific prelude file, and
its full IR will be printed. Additionally, it becomes an option with the
default `--dump-sem-ir-ranges=only` to add `//@dump-sem-ir-file` and get
the full file's IR.
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>
Split out `TokenInfo` to be able to easily write `using ValueType =
TokenInfo;` on `TokenIndex`. Also fixes a small type issue on
`ValueStore` that affected `mapped_iterator` behavior when writing
`old_tokens_it->first < next_offset`.
Moves LineInfo and CommentData out so that they can easily be set as
`ValueType` on the Index types. I've also been thinking about letting
`ValueStore` take `ValueType` as a parameter instead of requiring it to
be inferred this way, but for these it feels more consistent with the
rest of the toolchain to do it this way.
I'm not doing similar with `TokenInfo` just because the recovery token
splicing makes it more difficult to use `ValueStore`.
This consolidates Lex::TokenizedBuffer::DumpSemIRRange and
Parse::TreeAndSubtrees::TokenRange into a single InclusiveTokenRange,
also making the OverlapsWithDumpSemIRRange function take the new struct.
I considered switching to `llvm::iterator_range<Lex::TokenIterator>`,
but we often want to see if the range is size one. Using `TokenIterator`
just looked like it'd add a bunch of offsetting to make it work; I view
that as low-value overhead.
For example:
```
Lex::InclusiveTokenRange token_range = GetSubtreeTokenRange(node_id);
auto begin_loc = tree_->tokens().TokenToDiagnosticLoc(token_range.begin);
if (token_range.begin == token_range.end) {
return begin_loc;
}
auto end_loc = tree_->tokens().TokenToDiagnosticLoc(token_range.end);
```
would become:
```
llvm::iterator_range<Lex::TokenIterator> token_range = GetSubtreeTokenRange(node_id);
auto begin_loc = tree_->tokens().TokenToDiagnosticLoc(*token_range.begin());
if (token_range.begin() + 1 == token_range.end()) {
return begin_loc;
}
auto end_loc = tree_->tokens().TokenToDiagnosticLoc(*(token_range.end() - 1));
```
So I'm keeping the bespoke struct.
This prints instructions that are inside the range, and entities that
overlap with the range. Note this can lead to incomplete printing of
entity contents.
Syntax rationale is on `DumpSemIRRange` to try and record this, since
I'm not sure this belongs in the language design. The intent of this is
to be able to subset SemIR, which will be done separately in the
formatter.
---------
Co-authored-by: David Blaikie <dblaikie@gmail.com>
What this really does is avoids shadowing names, so that we can
comfortable have things like `Check::DiagnosticEmitter` or
`Check::DiagnosticLoc` without shadowing being a concern.
Note, down this path I'm also thinking about:
- Renaming misc DiagnosticConsumer/DiagnosticEmitter classes, possibly
just to DiagnosticConsumer/DiagnosticEmitter (so
`Check::DiagnosticEmitter` instead of `SemIRLocDiagnosticEmitter`).
- Dropping `Diagnostic` from `Emitter::DiagnosticBuilder`.
- But not for `Check::DiagnosticBuilder`, because `Check::Builder` would
be ambiguous.
- Renaming diagnostics/diagnostic_* to drop "diagnostic".
[Discussion about SemIRLoc ->
DiagnosticLoc](https://discord.com/channels/655572317891461132/655578254970716160/1353771570463768698)
reminded me of this (in particular the older [Check::DiagnosticBuilder
discussion](https://discord.com/channels/655572317891461132/655578254970716160/1344363562608627763)),
but I'd only do that rename if there's matching consensus about a path
forward where we keep SemIRLoc, and in a way that it's only ever used
for diagnostics (the divergence from which is at the root of current
LocId discussion).
I'm trying to keep that separate from a namespace addition for clarity.
At present, we typically define a DiagnosticConverter, then store an
instance of it and a DiagnosticEmitter that wraps it. This is relatively
minor in general, but I've been trying to create more self-contained
DiagnosticEmitter classes (which hold their own DiagnosticConverter,
similar to NullDiagnosticEmitter), and there it just gets in the way.
Since we don't reuse DiagnosticConverter instances, this combines the
definition into DiagnosticEmitter. Mainly this means we don't have a
separate object in play, and less to carry around.
The most impact is probably to SemIRDiagnosticConverter, which was also
the most complex. Now `SemIRLocDiagnosticEmitter`, this gets some
different construction flow. Note in the PR I've split the file rename
to its own commit, to try to help delta views. However, the most
substantial parts of the refactoring are split into #4876, which this
depends upon.
I'm looking at eliminating `DiagnosticConverter`. This change removes
`NodeLocConverter` (albeit adding `UnitAndImportsDiagnosticConverter`),
and in doing so, refactors lex conversion functions to extract them out
from the `DiagnosticConverter` functions.
I'll be following up with changes that collapse `DiagnosticConverter`
logic into `DiagnosticEmitter` locations. The intent is that we
shouldn't need separate ownership of both types.
- Escape dumped token strings (what got me here)
- Change the quoting from backticks to quotes
- Also add a `FormatEscaped` helper function for this, updating other
`.write_escaped` uses
This change deliberately breaks away from the line/column ordering, and
instead focuses on a last byte offset corresponding to the final token
processed as part of producing the message. Where that's equal, this
maintains stable ordering in order to reflect the order that diagnostics
were produced.
The intent of this approach is that lex, parse, and check diagnostics
are interleaved based on where they are produced, but that
subexpressions still have diagnostics emitted prior to containing
expressions. In particular, the prior line/column sort essentially
sorted on the _start_ of where a diagnostic was associated, and this is
closer to sorting based on the _end_. As a consequence, something like
`F(1 2)` will have the error for `1 2` emitted _before_ a diagnostic for
`F(1 2)` not matching parameters, instead of _after_.
In check, we track the last handled node. This provides a
last_byte_offset _separate_ from where a diagnostic is associated. The
intent is that this creates an ordering of diagnostics which may be
associated with earlier code, to cause the diagnostics to be emitted
later. An example consequence of this is the change in ordering of
modifier diagnostics: we are diagnosing those from the same place, but
they have the same last_byte_offset, so we print them out in the order
produced.
I've added similar tracking to parse, but cannot identify any test which
is affected by it (note the separate commit, I thought about this late).
I'm not sure whether we have good out-of-order errors we could produce
for this.
A significant number of tests have reordered diagnostics as a
consequence of this change, so this change does not add further testing.
- Provide `Check::Dump(context, arg)` and similar.
- gdb and lldb should do contextual lookup, and `call Dump(*this,
Lex::TokenIndex::Invalid)` has been tested with gdb.
- Since this is only for debug, keeps the functions fully separated from
code.
- Uses alwayslink to ensure objects are correctly linked, even though
there are no calls.
- `-Wno-missing-prototypes` is needed when we don't have forward
declarations.
- Code is not linked in opt builds, using `#ifndef NDEBUG`.
- This probably could be doing something in BUILD files with a
`select()`, but the `#ifndef` seemed easier.
This is based on #4620, but uses free functions instead of member
functions.
Co-authored-by: Dana Jansens <danakj@orodu.net>
---------
Co-authored-by: danakj <danakj@orodu.net>
This is essentially the result of looking at `.begin()` uses. We also
frequently do `std::shuffle`, but unfortunately STLExtras doesn't
provide a wrapper for that.
Previously Collect() was used for types that implemented
CollectMemUsage() but otherwise Add() was used. This required the caller
to think about the type of the field and know/decide which method to
use.
Now, the caller always uses Collect() unless they are adding specific
byte values, in which case Add is used. Typically then, Add will only be
used to implement the CollectMemUsage() function.
To do this we require all Collect() methods to be templates so that they
all be a single overload set. The Collect on BumpPtrAllocator is
converted to a template that checks
`std::same_as<llvm::BumpPtrAllocator, T>`.
This is in anticipation of making the integer value store be customized
heavily. I'd like to extract it from the common code when doing that, so
first disentangling them here without any intended change in
functionality or behavior to enable that.
I've tried to update `#include`s to be as minimal as I can and added a
few missing includes spotted in the process.
I've split the test for value store to include what was easy focused on
just the value store templates rather than the unified shared value
stores.
This might surface some opportunities for adding more tests, but for
this PR, just doing the minimal restructuring.
Trying to make split file tests of lex functionality shorter and easier
to read. numeric_literals.carbon in particular has an example of why I'm
interested in this (at the bottom). This also switches from `[]` list
format to `-` list format so that the trailing `]` is removed.
Trimming comments in tokenized_buffer.h because (1) it feels like it's
giving too much detail about what's printed, which has drifted slightly
and (2) it also feels like it's trying to justify YAML output, when
that's just what we're doing in general.
---------
Co-authored-by: Geoff Romer <gromer@google.com>
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>
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.
This switches `DCHECK` and `FATAL` as well.
The goal is to reduce the code size impact of these assertions so that
we can keep more of them enabled. Currently, the largest cost I see from
`CHECK` is not the actual check or the cold code itself, but actually
the failure to inline trivial functions due to the presence of the cold
code. This means that our goal isn't to reduce apparent code size in the
final binary but the LLVM IR cost assessed for these routines in the
inliner, which closely correlates with code size but is a bit different.
As discussed in #4283, experimentation shows that a single function call
with a minimal number of arguments is the lowest cost model for these.
This is easily achieved with a format-string API that internally uses
`llvm::formatv`. This PR is essentially the `CHECK` version of #4283.
However, the check macros are substantially harder to make work with
both format strings and streaming because they also take a condition.
Also, unexpectedly, I was very successful at devising a regular
expression based automated rewrite from the streaming to the format
string form with only low 10s of manual fixes. This includes compacting
strings broken up across lines, etc. Given how well that went, I've
prepared this PR which just directly switches to the format string API
and migrate everything to use it.
One nice side-effect is that the format string approach ends up greatly
simplifying the implementation here as well.
This is ... *shockingly* effective. Parsing speeds up by more than 3%
with just this change. And checking speeds up by **8%** with this change
alone:
```
BM_CompileAPIFileDenseDecls<Phase::Parse>/256 86.3µs ± 1% 82.9µs ± 1% -3.94% (p=0.000 n=17+19)
BM_CompileAPIFileDenseDecls<Phase::Parse>/1024 431µs ± 1% 415µs ± 1% -3.76% (p=0.000 n=18+19)
BM_CompileAPIFileDenseDecls<Phase::Parse>/4096 1.77ms ± 1% 1.71ms ± 1% -3.18% (p=0.000 n=18+19)
BM_CompileAPIFileDenseDecls<Phase::Parse>/16384 7.44ms ± 1% 7.17ms ± 2% -3.56% (p=0.000 n=18+20)
BM_CompileAPIFileDenseDecls<Phase::Parse>/65536 30.7ms ± 1% 29.7ms ± 1% -3.15% (p=0.000 n=18+20)
BM_CompileAPIFileDenseDecls<Phase::Parse>/262144 131ms ± 1% 127ms ± 1% -2.81% (p=0.000 n=18+18)
BM_CompileAPIFileDenseDecls<Phase::Check>/256 878µs ± 2% 800µs ± 1% -8.91% (p=0.000 n=19+20)
BM_CompileAPIFileDenseDecls<Phase::Check>/1024 1.88ms ± 2% 1.72ms ± 1% -8.56% (p=0.000 n=19+20)
BM_CompileAPIFileDenseDecls<Phase::Check>/4096 5.78ms ± 2% 5.28ms ± 1% -8.70% (p=0.000 n=20+18)
BM_CompileAPIFileDenseDecls<Phase::Check>/16384 21.9ms ± 1% 20.1ms ± 1% -8.02% (p=0.000 n=18+20)
BM_CompileAPIFileDenseDecls<Phase::Check>/65536 90.4ms ± 2% 83.1ms ± 1% -8.04% (p=0.000 n=19+20)
BM_CompileAPIFileDenseDecls<Phase::Check>/262144 381ms ± 2% 352ms ± 1% -7.79% (p=0.000 n=19+19)
```
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Co-authored-by: josh11b <15258583+josh11b@users.noreply.github.com>
This makes each token info consist of 8 bytes of data:
- 1 byte of the kind
- 1 bit for whitespace tracking
- 23 bits of payload
- 32 bits for byte offset in the file
This builds directly on representing the location of the token as
a single 32-bit offset, now compressing the rest of the data into
a single 32-bit bitfield.
This adds some implementation limits: we can no longer lex more than
2^23 tokens in a single source file. Nor can we have more than 2^23
string literals, integer literals, real literals, or identifiers. Only
the first of these is even close to an issue, and even then seems
unlikely to ever be a problem in practice.
The memory efficiency here is great and the motivating goal. But to make
this work well, we also need to streamline how we create the tokens.
Otherwise, all the bit fiddling can end up erasing our gains. This PR
adds a number of APIs to manage creating and accessing the now
significantly more complex storage of token infos to try and help with
this.
One big change required to simplify the writes here is to switch from
computing whether a token has trailing space after-the-fact to
pre-computing whether a token will have leading space. That lets us have
the leading space information available immediately when forming the
token, and avoids doing a single bit flip afterward.
Another change that helps with this representation is to minimize the
updating of groups after-the-fact. The code now tries to set the opening
index directly when creating the closing token and only updates the
opening group afterward. Because of the bit packing, this is a reduction
of 0.5% of dynamic instructions in the compile benchmark, and has
dramatic improvements for the grouping symbol focused benchmarks.
All combined, this is a significant improvement on the lexer-focused
benchmarks despite the added complexity, and a significant win on our
compile time benchmarks due to both the lexer improvements and
downstream memory density improvements: 5-12% reduction in lex time,
growing larger as files get larger. About a 4.5% reduction in parse
time, and even a 1-2% reduction in total check time. =D
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
Co-authored-by: Geoff Romer <gromer@google.com>
First, this replaces the separate line index and column index in the
token information with a single 32-bit byte offset of the token. This is
then used to compute line and column numbers with a binary search of the
line structure and then using that to compute the column within the
line. In practice, this is _much_ more efficient:
- Smaller token data structure. This will hopefully combine with a
subsequent optimization PR that shrinks the token data structure still
further.
- Fewer stores to form each token's information in the tight hot loop of
the lexer.
- Less state to maintain while lexing, fewer computations while lexing.
We only have to search to build the line and column information off the
hot lexing path, and so this ends up being a significant win and shrinks
some of the more significant data structures.
Second, this shrinks the line start to a 32-bit integer and removes the
line length. Our source buffer already ensures we only have 2 GiB of
source with a nice diagnostic. I've just added a check to help document
this in the lexer. The line length can be avoided in all of the cases it
was being used, largely by looking at the next line's start and working
from there. This also precipitated cleaning up some code that dated from
when lines were only built during lexing rather than being pre-built,
which resulted in nice simplifications.
With this PR, I think it makes sense to re-name a bunch of methods on
`TokenizedBuffer`, but to an extent that was already needed as these
methods somewhat predate the more pervasive style conventions. I avoided
that here to keep this PR focused on the implementation change, I'll
create a subsequent PR to update the API to both better nomenclature and
remove deviations from our conventions.
There may also be a way to de-duplicate the binary search in the
diagnostic location conversion and the main line accessor binary search,
but it wasn't obvious to me that it would be a net savings, so left it
alone for now.
The performance impact of this varies quite a bit...
The lexer's benchmark improves pretty consistent across the board on
both x86 and Arm. For x86, where I have nice comparison tools, it
appears 3% to 20% faster depending on the specific pattern. For Arm
server CPUs at least it seems a much smaller but still an improvement.
The overall compilation benchmarks however don't improve much with these
changes alone on x86. Significant reduction in instruction count
required for lexing, but the overall performance is bottlenecked
elsewhere in the overall compilation it seems. However, on Arm, despite
the more modest gains in special cases of lexing, this shows fairly
consistent 1-2% improvements in overall lexing performance on our
compilation benchmark. And the expectaiton is these improvements will
compound with subsequent work to further compact our representation.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
`i32` is retained as a special case for now, for bootstrapping purposes,
and maps to `BuiltinIntType`, which is distinct from `Core.Int(32)`.
This will be removed later once we support `Core.BigInt`.
For now this provides both the `iN` types and also the builtins to
support `Core.Int(N)`. The intent is that we'll change the `iN` support
to rewrite to calls here when we do that for the other type literals and
type keywords.
No conversions between integer types are supported yet, and all literals
are of type `i32`, so we can't actually form values of any of these new
types.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
Co-authored-by: Carbon Infra Bot <carbon-external-infra@google.com>
As discussed around #3792, identify the import a diagnostic message came
from prior to the diagnostic message itself. This occurs during location
translation so that the logic can be central.
I'd considered associating the parse node with ImportRef instructions,
but I realized about halfway through that because I need to store the
ImportDirectiveId on the ImportIR for cross-package imports, it's there
for use in location translation without extra work. That saves a fair
amount of stringing it through declarations, as well as an oddity where
ImportRef instructions would have a node that didn't really represent
them.
Since the addition of TranslateArg, I don't think this type is going to
go away (cutting a TODO). Refactoring names slightly to fit the current
role, and adding const to ConvertLocation.
I wanted to choose one or the other. I think some code has been using
each from early on. We're predominately using `filename`, so
consolidating on that. This conveniently matches the [Google dev doc
style guide](https://developers.google.com/style/word-list#filename)
(which we use for docs) which says "filename: Not file name".
In toolchain:
```
╚╡git grep file_name . | wc -l
35
╚╡git grep filename . | wc -l
489
```
Move handling of mismatched brackets out of the main lexing loop into a
separate pass that is only run if there are mismatched brackets This is
done in preparation for using both lookahead and lookbehind to work out
how to match brackets, and to get this code far away from the hot lexing
loop.
Fix bracket insertion location to be immediately after the token that
we're inserting the bracket after, rather than potentially at the end of
a comment. When there are open brackets at the end of the file, say that
there are open brackets, not that there's a closing bracket without a
matching opening bracket.
We have `StringLiteral`s in multiple other `Carbon` sub-namespaces.
Rename to a more specific name to avoid collisions.
We should likely also rename `Carbon::IntId` -> `Carbon::IntValueId` and
`Carbon::RealId` -> `Carbon::RealValueId`, but this collision is
prioritized because it was blocking work on typed parse nodes which
introduces a `Carbon::Parse::StringLiteralId`.
I was suggesting this because `FloatingPoint` is pretty long. `int` and
`float` should be familiar abbreviations. `unsigned` should be familiar
to developers too, but `UnsignedInt` still feels usefully clearer for
the additional chars.
This is an incremental improvement on our diagnostic messages that
simply underlines an entire token if the token is larger than 1 char
(else it points to the single char with a caret like it used to).
Following up on discussion yesterday regarding this split.
Note, I'm expecting #3341 to do IdentifierId -> NameId in SemIR. It
might be worth adding NameId creation directly to StringStore if you're
content with this setup though.
Chandler Carruth
Richard Smith
Jon Ross-Perkins
Thomas Köppe
Dana Jansens
cui fliter
Jacob Schneider