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
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.
Add caching of parsed documents, and testing of the textDocument
handlers. This is based on #4896, which splits out some of the
boilerplate to calls.
Note, this caches the entire parse state because we'll want to try to
emit diagnostics when we see the update, without waiting. It may be
helpful to do that asynchronously, but we don't want to wait for another
call (such as documentSymbol). Really, we'll probably want to also add
check for diagnostics, at least.
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.
Similar to #3705, we actually have a mix of `Make` and `Create` in
factory functions too, so this PR is normalizing on `Make`. It's
intended to be consistent with the naming choice for Carbon factory
functions.
Note, MakeSyntheticBlock is the only one I feel a little weird about
because llvm's own APIs use Create, and this is essentially wrapping
LLVM calls. But the flipside is it also feels like a vague line to draw,
when we also differ from LLVM coding style in other ways.
Per [#toolchain
discussion](https://discord.com/channels/655572317891461132/655578254970716160/1176632520834560211)
We'd at one point been trying to put `[[nodiscard]]` everywhere, but
then we stopped because it had felt verbose without finding many issues
(plus, people plain forgot to add it). Some history in #888.
Since newer code gets added without it, we now have code like:
```
auto GetLineInfo(Line line) -> LineInfo&;
[[nodiscard]] auto GetLineInfo(Line line) const -> const LineInfo&;
auto AddLine(LineInfo info) -> Line;
auto GetTokenInfo(Token token) -> TokenInfo&;
[[nodiscard]] auto GetTokenInfo(Token token) const -> const TokenInfo&;
auto AddToken(TokenInfo info) -> Token;
[[nodiscard]] auto GetTokenPrintWidths(Token token) const -> PrintWidths;
```
Here, the lack of `[[nodiscard]]` doesn't mean anything: for example,
`GetLineInfo` should not have its result discarded if it's called. But
the mix could be confusing for readers.
As a resolution, remove the attribute. `[[nodiscard]]` should be treated
like other attributes going forward, which essentially means "avoid in
general, add a comment to explain why the attribute is needed" rather
than use-as-default.
- Treat an input of `-` as meaning stdin.
- Fix building of an llvm::MemoryBuffer from a non-regular file.
- Do not enforce filename restrictions on non-regular files.
- Do not invent an output file name based on the name of a non-regular
file.
---------
Co-authored-by: Chandler Carruth <chandlerc@gmail.com>
This updates SourceBuffer to diagnostics. Some additional edits to
diagnostics were necessary due to issues moving arguments around, which
seems to stem from a compile error with clang 14 (fixed in later
versions).
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Rearranges driver logic into CompilationUnits in order to associate
artifacts from the various stages of compilation.
Note, I'm not totally sure what the right thing to do is for
lower/codegen, so I'm just doing a rote change there for now that
mirrors prior phases (this is all the code supports anyways, so is
probably right for now regardless).
SourceBuffer error output is moved local for consistency with other
steps, and so that it's less ambiguous whether the error should be
expected to already include a filename.
This adds vfs support to the toolchain, allowing Driver to take in-memory inputs in tests. As a consequence, I'm simplifying SourceBuffer: rather than allowing tests to pass in their own memory buffer, I'm using InMemoryFileSystem to push for greater consistency with production code. This does hit a quirk where I need to be careful about null terminator handling because fuzzer imports don't always have one, but that's probably more robust anyways.
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
My intent is to add CARBON_ bas a prefix, and this makes that easier by creating a tool for auto-fixing guards in general.
string_literal is a manual fix -- it had no guard and I didn't automate that (technically I think I could, especially by enforcing the file header/footer, but it didn't feel quite worth it to me).
There's currently a bug with empty files, in that it initializes SourceBuffer with an invalid StringRef that results in a crash. That got me looking at the std::optional TODO, but the issue is that there are really three states:
- Buffered
- mmapped (not buffered)
- Moved out of (no longer initialized)
Technically an optional could work if we initialize the buffer on move out, indicating the mmap is gone. But the mode setup felt better to me.
And then this also adds the size check. Which is really how I started looking at this.
There are some declaration order changes, and a few test classes switched from `struct` to `class`. However, this PR is mostly adopting `_` naming of private member variables due to the shift in naming style. None of what's here should have behavior impacts, it should just be style.
Note, there are a lot of things that *look* like they could be accessor-named, but I'm not doing that in this change. Happy to do it separately if you want me to do another PR focused on it.
Co-authored-by: Chandler Carruth <chandlerc@gmail.com>
This should clean up our top level directory and the build patterns.
No non-mechanical edits here. Just injecting `toolchain/` and
`TOOLCHAIN_` and then running formatting tools.
Jon Ross-Perkins
Richard SmithandChandler Carruth
Jon Meow