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.
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).
There was a lot of repetition and unnecessary cruft in our Bazel
toolchain support. Switch to generating all of it with a single macro
that handles everything. This should make no real difference but
dramatically simplifies adding a new CPU.
Use this simplified system and add `aarch64` which is how Arm 64-bit CPU
support shows up on a Linux host.
Also teach the basic scripts to map `aarch64` to `arm64` which is used
in the released artifact strings.
- Require namespace members be declared in the same name scope as the
namespace is declared.
- Allow binding patterns to directly declare names in namespaces.
- Disallow using different namespaces in the same binding pattern.
---------
Co-authored-by: Chandler Carruth <chandlerc@gmail.com>
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
- 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>
As a follow up to #2825 this pr implements size deduction for nested
arrays.
E.g. `var x: [[i32;];] = ((1,2), (3,4));`.
Also updated the pattern matching logic for arrays, now it also checks
element types of the tuple size being deduced from. As a result code
like `var x: [i32;] = ("foo", "bar");`(note, that it tries to init an
array of i32 with a tuple of strings) fails to compile with a pattern
match error instead of an implicit cast failed error.
Moved some common type-related logic used in type_checker.cpp and
interpreter.cpp to the separate file.
When there is no `package` directive, default to `Main//default api`
instead of
`Main//default impl`. This means:
- The extension will be `.carbon`, not `.impl.carbon`.
- There can only be one such file when compiling.
---------
Co-authored-by: Chandler Carruth <chandlerc@gmail.com>
When a `namespace` keyword has no `;` following it, we recover by
building a parse tree `Namespace` node from the `Namespace` token (as
there isn't a `;` token). Allow this correspondence on errors.
Also teach the diagnostics in this case to avoid the end-of-file token
as that's almost always going to be a less meaningful location. Instead,
we can point at the introducer which should at least be in the code that
led to the error.
This should cover:
```
library "lib" api;
import Foo library default;
import library default;
import library "lib";
```
This splits out `PackageName` and `LibraryName` to their own parse nodes
so that checking can ignore them and still get a balanced parse tree
(otherwise, we essentially need to implement handling of the parse nodes
only to remove the identifiers/string literals -- the optional names
mean we can't blindly do that as before). For reference, these nodes
don't need to be handled because CheckParseTree will need to directly
funnel import information along with checked IRs.
This captures the exit code of Bazel and checks for success or permanent
errors on each attempt. It also sleeps a small amount between attempts.
We should be able to increase the retries and sleeps as needed to
minimize flakiness here, and Bazel should even persist incremental
progress efficiently. Hopefully this helps reduce the failure rate of
our CI.
It also changes how we build on a `push` to use a single Bazel clause to
hold this logic.
Managed to get one of the download failures when testing this, and the
retry logic worked but there was a bug in the success logic.
Otherwise seems to work:
- Synthetic failure:
https://github.com/carbon-language/carbon-lang/actions/runs/6872431707/job/18690894069
- Success:
https://github.com/carbon-language/carbon-lang/actions/runs/6872461061
---------
Co-authored-by: josh11b <josh11b@users.noreply.github.com>
A struct with the same member name twice can cause a `CARBON_CHECK`
failure later when it is used (problem found by fuzzing).
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
As I was working on this, I noticed `import` and `library` syntax needs
to be fixed for how it imports the current package, and for `Main`
libraries. This mostly reflects the current state in its testing.
Otherwise, this should handle most of the errors I could think of:
dependency cycles, redundant imports, etc.
It does not actually deal with the nuances of cross-IR references.
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
For now, we require the same introducer to be used each time a class is
declared, but see #3384.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
Jon Ross-PerkinsandRichard Smith
Geoff Romer
Sree Dananjay S
josh11b
Jacob Schneider
kshokhin