Building on #3463. The PushState+PopState to construct a state feels
worth cleanup. The rest is just kind of making it easier to do without
adding another PushState overload.
Building on #3462, trying to make the flow easier to see, also making a
little more use of macros for boilerplate modifier handling.
---------
Co-authored-by: josh11b <josh11b@users.noreply.github.com>
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
BinaryOperatorAdd had been added early on as a proof-of-concept for
toolchain design for parsing -> checking flow. However, it doesn't
reflect the interface direction for operators, and now other portions of
the toolchain demonstrate the relevant logic. Instead, it's just a bit
of an outlier versus other instructions which have evolved over time.
Choosing to make the constructor explicit in the test, rather than
NOLINT, because it seems to better reflect how our code is usually
written (and may be more likely to trip an issue).
Per their documentation, the `llvm::Initialize*` functions are only
supposed to be called by the main program, not by a library like
toolchain/codegen. Fixes a hang due to a data race in multithreaded
autoupdate.
Add a utility class `Carbon::InitLLVM` to do the common LLVM
initialization shared by all Carbon tools, optionally including
initializing the LLVM targets. Because the LLVM targets add a lot of
binary size, only initialize them for binaries that opt in by depending
on a new target `//common:all_llvm_targets`.
Also fix `//explorer:file_test` and `//explorer:file_test.trace` to
share a binary rather than linking an identical binary twice.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
In theory because none are allowed. This is to improve consistency in
handle_decl_name_scope's modifier handling, removing the namespace
special-case.
I noticed there's a crash bug on `impl <declaration>` which I'll address
separately.
This builds on #3461.
This is supporting a direction that all parse nodes should correspond to
a single token, allowing for reduced tokenized buffer access during
checking (it's still necessary for diagnostics, and some literals).
One of the justifications for a unified parse node was implementation
LOC: note this is slightly smaller, using macros to reduce some
duplication. While this does add more switching in HandleDeclScopeLoop,
that's offset by less explicit switching in the check handlers. Also, I
think the duplication in HandleDeclScopeLoop can be reduced by shifting
the flow there, which I'll do in a separate PR.
Most of the calls to `StringifyType` already passed `true` for
`in_type_context`. Checking the rest, I found that every one of them was
already sufficiently clear that they were printing a type, or could be
made so with a very small change to the diagnostic text.
This creates a namespace for `package` scope.
It looks like names of class_decls in namespaces lead to an unexpected
instref. This is already true, as best as I can tell. I'm not sure if
there's a preferred approach to address that, so I've left a TODO for
now.
Also switch how we ensure that stdin is closed for tests, so that `bazel
run` doesn't hang if invoked manually.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
This now puts file content into a string, allowing conflict markers to
be elided from file content. When code executes, this means it executes
without seeing conflict markers, without a temporary update to the file
that would only remove conflict markers.
Also refactors the main process flow, because it was getting a little
too lengthy. This means passing a bunch of parameters passed around
(partly because TestContext is private on the test class, and I don't
want to change that). I'm hoping that overall it's easier to read the
core loop now.
Mostly tested with some manually added conflict markers, and that
current tests don't change.
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
There's a trade-off here of explicitness in the use versus repetition,
but I'm hoping the Id offers sufficient info (also, some Ids already
relied on this, so this builds consistency). The forward declarations
I'm mixed on, but they are difficult to avoid if heading down this route
due to interdependencies between ids and types which contain ids.
This removes the filename from the file-scoped block, and places it
above to make it clear where the full SemIR begins (with multifile,
providing a barrier between).
This Pull Request added support for Bazel installation and setup in the
contribution_tools.md file un the docs directory. It added a link to the
releases page of the bazel repository for debian/ubuntu users to
download the binary and execute the commands provided in the
documentation.
Closes#3439
This reflects how we're naming classes that derive from these classes,
and matches usage for each existing `Id` and `Index` type, except:
- `Parse::NodeId` previously inherited from `ComparableIndexBase`, and
is no longer comparable.
- `SemIR::MemberIndex` previously inherited from `IndexBase`, and is now
comparable.
Making `Parse::NodeId` non-comparable reflects that it's intended to be
an opaque identifier for a node and that the ordering is an
implementation detail rather than part of the intended public interface.
`PostorderIterator` and `SiblingIterator` still rely on the numerical
meaning of `NodeId`s, but that's OK since they're part of the node
implementation.
The main difference I'm aiming for is that clangd doesn't complain about
the struct being unused, but it does miss the function's use. But really
these are specific to formatv for diagnostics, so this is more clearly
marking such, and probably makes for a better pattern for the future.
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.
These are manual fixes; mostly from clang-tidy, some from clangd (which
notes unused includes).
In typed_insts, adding inlline due to misc-definitions-in-headers. Per
discussion, clang-tidy is wrong, but inline silences it.
For parameter name skew in definition versus declaration, I'm just using
the name from the definition.
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
This gets to a lifetime subtlety, particularly with things like the
sorting diagnostic consumer that delay output. In order to reduce the
chance of accidental references, disallow StringRef in the diagnostics.
For example:
```
./toolchain/diagnostics/diagnostic_emitter.h:162:5: error: static_assert failed due to requirement '!std::is_same_v<llvm::StringRef, llvm::StringRef>' "Use std::string or llvm::StringLiteral for diagnostic lifetimes."
static_assert(
^
toolchain/check/convert.cpp:477:11: note: in instantiation of member function 'Carbon::Internal::DiagnosticBase<std::string, std::string, llvm::StringRef>::DiagnosticBase' requested here
CARBON_DIAGNOSTIC(StructInitMissingFieldInConversion, Error,
^
./toolchain/diagnostics/diagnostic_emitter.h:47:7: note: expanded from macro 'CARBON_DIAGNOSTIC'
::Carbon::Internal::DiagnosticBase<__VA_ARGS__>( \
^
```
---------
Co-authored-by: Chandler Carruth <chandlerc@gmail.com>
This adds instructions so that we get printing. I may adjust the
instruction format a little further to add a type, but I think the basic
setup will remain.
Note this builds on #3414
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.
The version of `flake8` was too old to support with Python 3.12 -- there
is new F-string support that caused false positives sadly. The updated
version has fixes for all of these.
This in turn updates codespell which has picked up several new fixes
that actually fire in our code, so also fix everything. While we don't
do more in-depth updates to old proposals, similar to simply fixing
broken links, fixing automatically detected typos seems scalable and
fine.
All edits were automatically generated here.
Also requires switching to using `pip_parse` and providing a fully
resolved requirements lock file. This moves the input requirements to
the `requirements.in` file, and processes it with:
```console
$ bazel run //github_tools:requirements.update
```
This will regenerate the `requirements.txt` file that is checked into
the repository. The advice in the documentation is specifically to keep
this file checked into the repository for hermetic builds with stable
Python dependency versions.
This should fix builds on systems where the Python version is 3.12 and
newer and the older version of `rules_python` stops working with errors
due to removal of long-deprecated interfaces.
Jon Ross-Perkins
josh11b
Geoff RomerandRichard Smith
Sree Dananjay S
