Disables three new warnings because they lean more towards style
conflicts than fixes. I've brought these up on #style.
Other than that, mostly fixing basic issues, and things that
clang-tidy-20 seems to fire where clang-tiday-16 didn't. One particular
curious case is `llvm::StringLiteral::data()` uses, which are flagged as
not strictly null-terminated; I'm switching to `const char*` in those
spots which matches `llvm::formatv`'s format argument, but feels worse.
I'm removing `run_clang_tidy.py` here because I'm observing it give
fewer warnings than `bazel build --config=clang-tidy -k
//toolchain/...`. The latter matches how we enforce in GitHub actions
(and also caches results, and suppresses output for files that have no
issues), so I'm dropping the bespoke script.
Commenting on danakj's clang-tidy PRs, it would've been helpful to just
have a quick reference for older checks. So while I've been getting
comments for new things, go back and comment the ones that predate
adding per-check comments.
I did this mainly by running clang-tidy and seeing whether we could
re-enable them, thus also fixing the clang-tidy wrapper script.
Adding backticks to try to make it easier to scan.
I think `paths.parents[2]` is easier to read than
`path.parent.parent.parent`, just applying uniformly. I'd noticed this
while looking at #4227
Also remove a couple `resolve()` calls that shouldn't be necessary since
`__file__` is absolute (elsewhere in the same files, `resolve()` is used
to resolve potentially relative paths)
I was considering making a "/bazel" alias for this, but I'm really on
the fence about whether that's a good choice. However, I do think this
is good to have to simplify installs.
Fixes#3071 and #3896
Adds scripts/calculate_release_shas.py to print the versions, and
updates tool versions. Consolidates target-determinator logic with the
other tool logic.
This is also intended to make it easier to add more tools.
This is awkward to track... Probably it would be best done by tracking
the ratio of unique IDs to lines as a floating point and plot them and
see what a best fit distribution curve looks like. But none of the
histogram printing or stats tracking stuff already in use here makes it
easy to do any of that...
So this does what I hope is a reasonable rough approximation by counting
the ceiling of unique identifiers per 10 lines of code, and plotting
that discreet histogram. Shape of the histogram is exactly what I would
expect: one centered distribution, vaguely normal looking. And the
center for a bunch of different codebases, including our toolchain, is
exactly at 5, which would mean 0.5 unique IDs per line. And the
distribution is pretty reliably bounded above by 10 or 1 unique ID per
line. Which almost seems to clean to be true? Slightly worried about
confirmation bias making me think this code is working because the
results look so pretty.
Here is the output for the toolchain:
```
## Unique IDs per 10 lines ## (median: 6)
2 ids [ 2] █▎
3 ids [19] ████████████▎
4 ids [32] ████████████████████▋
5 ids [55] ███████████████████████████████████▌
6 ids [62] ████████████████████████████████████████
7 ids [44] ████████████████████████████▍
8 ids [22] ██████████████▎
9 ids [11] ███████▏
10 ids [ 7] ████▌
11 ids [ 2] █▎
```
And here is the output for llvm-project/*/{lib,include} (to avoid
tests):
```
# Unique IDs per 10 lines ## (median: 5)
1 ids [ 29] ▍
2 ids [ 282] ███▊
3 ids [1492] ███████████████████▉
4 ids [2674] ███████████████████████████████████▌
5 ids [3011] ████████████████████████████████████████
6 ids [2267] ██████████████████████████████▏
7 ids [1549] ████████████████████▋
8 ids [ 817] ██████████▉
9 ids [ 301] ████
10 ids [ 98] █▎
11 ids [ 61] ▊
12 ids [ 50] ▋
13 ids [ 25] ▍
14 ids [ 33] ▌
15 ids [ 14] ▏
16 ids [ 15] ▎
17 ids [ 9] ▏
18 ids [ 8] ▏
19 ids [ 12] ▏
20 ids [ 15] ▎
21 ids [ 3]
22 ids [ 8] ▏
23 ids [ 3]
24 ids [ 3]
25 ids [ 6] ▏
26 ids [ 0]
27 ids [ 2]
28 ids [ 0]
29 ids [ 0]
30 ids [ 3]
31 ids [ 1]
32 ids [ 1]
```
Python [PEP-585](https://peps.python.org/pep-0585/) replaces a number of
`typing` module types with built-in equivalents and `collections.abc`
versions as of Python 3.9, with the aim of eventually removing the
`typing` module versions of these classes altogether. Since the minimum
required version of Python listed in the [Contribution Tools
document](https://github.com/carbon-language/carbon-lang/blob/trunk/docs/project/contribution_tools.md#main-tools)
is 3.9, the type hints in the various python files in the repo can be
updated to this style of type hint without a need for backwards
compatibility.
Feel free to close if this isn't a desired change at this time!
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
This adds a defined Carbon version to the Bazel build and codebase that
can be used both to implement features like version checks and to report
a meaningful version on the command line. This replaces a hard-coded
string and a TODO in the driver.
As part of this, it adds support for defining the version in Bazel, and
special build flags for overriding relevant parts such as the
pre-release marker used. The exact structure and meaning of our version
string, including the pre-release parts, is implemented here in line
with the draft proposal:
https://docs.google.com/document/d/11S5VAPe5Pm_BZPlajWrqDDVr9qc7-7tS2VshqO0wWkk/edit?resourcekey=0-2YFC9Uvl4puuDnWlr2MmYw
This also introduces a workspace status command to the repository to
extract the git commit SHA and other information when building, and the
logic to stamp that into binaries as part of the version string when
useful. The technique used leverages weak symbols with whole archive
linking to allow a link-time override of unstamped data with stamped
data in the leaf executable. This makes building with `--stamp` a
reasonable default, especially for development builds. The CI system is
explicitly opted out of this as there it has no benefit.
Last but not least, all of these are wired into the install rules so
that we build installable packages with the version number in a
conventional place in the directory and filename.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
This uses a header-only extraction of the Boost unordered hashtable
project to allow a trivial Bazel build and for us to benchmark against
it effectively.
Note this builds on #3772 (I was partly cleaning up because I was
looking at this again).
This stops printing "Ignore missing" for ignored includes because it was
feeling noisy. This has been bugging me for a bit, and now I'm here. To
get an idea of what I mean, here it is masking a fix:
```
Querying bazel for Carbon targets...
Querying bazel for external targets...
Building header map...
Building generated file list...
Parsing headers from source files...
Ignored missing '#include "testing/fuzzing/carbon.pb.h"' in 'explorer/fuzzing/ast_to_proto_main.cpp'
Ignored missing '#include "testing/fuzzing/carbon.pb.h"' in 'explorer/fuzzing/ast_to_proto.h'
Ignored missing '#include "testing/fuzzing/carbon.pb.h"' in 'explorer/fuzzing/fuzzer_util.h'
Fixing include format in 'testing/file_test/file_test_base.h': '#include "gtest/gtest.h"' to '#include <gtest/gtest.h>'
Ignored missing '#include "testing/fuzzing/carbon.pb.h"' in 'testing/fuzzing/proto_to_carbon.cpp'
Ignored missing '#include "testing/fuzzing/carbon.pb.h"' in 'testing/fuzzing/proto_to_carbon.h'
Ignored missing '#include "testing/fuzzing/carbon.pb.h"' in 'testing/fuzzing/proto_to_carbon_test.cpp'
Done!
```
To help make this work, I'm also adjusting the gtest handling to use the
same EXTERNAL_REPO logic as the rest. I don't recall if there'd been
some other reason for the special casing, but AFAICT it works fine
(auto-adds a missing gtest dependency) this way.
I'd kept these in to separate the bazel module update from the BUILD
file changes, then forgot about it. I think all of these can be cleanly
removed now. I think it's something we should clean up for consistency
with the bazel central repository names; I think it's best to reduce
that divergence.
llvm_zlib and llvm_zstd remain because of how llvm depends on the
particular names.
Directories in `external/` now include versions of dependencies, so
continuing to make that work would require version-locking the
compile_flags.txt. We've previously seen issues with directories being
forgotten; this switches to letting bazel generate everything.
Setup follows instructions at
https://github.com/hedronvision/bazel-compile-commands-extractor?tab=readme-ov-file#first-add-this-tool-to-your-bazel-setup
Maintains `./scripts/create_compdb.py`. Per discussion, new files seem
to work okay -- I'm getting #include completion that clearly detects
files.
Note this creates an `external` directory:
```
╚╡ls -ld external
lrwxrwxrwx 1 jperkins primarygroup 27 Dec 19 14:02 external -> bazel-out/../../../external
```
I felt sort of weird checking that in though, at least alongside
`bazel-execroot` it feels like it should have a `bazel-` prefix.
However, I can't rename it. So omitting it, and leaving it to the script
to generate, felt like a reasonable compromise.
Running takes a few minutes (about 3 for me), and should be faster on
reruns. It does print a couple warnings, just tree_sitter missing
parser.c and _GNUC_PREREQ errors from m4.
Some background information is at https://bazel.build/external/migration
Trying to handle the simple cases first. This adds a requirement for
bazel 7 due to differences in bzlmod handling between 6 and 7 (also
discussed on
[#infra](https://discord.com/channels/655572317891461132/707150492370862090/1184942191412510720)).
Bazel seems to be okay with a partial migration such as ths.
The python import behavior has subtly shifted, so `carbon.` is no longer
part of import paths. There's a version-incompatible change for `@@`.
bzlmod makes repos sometimes show as `name~version`.
`target-determinator` seems to be okay with `@@` after a version update.
Things not moved here are things that basically need more dep work:
- clang_register_toolchains because I need to dive into its format.
- llvm-project because we need something slightly atypical, I need to
make sure patching and the repo work carries over.
- com_google_libprotobuf_mutator is sufficiently atypical that it
doesn't have a module already, but should be one of the easier things to
fix.
- brotli/woff2: I think we should actually consider removing these. But
again, they're not trivial moves.
- treesitter due to toolchain registration, which has shifted a bit.
- rules_nodejs because treesitter depends on it in an awkward way to
migrate.
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.
Running `bazel test //...` reported:
```
Test execution time outside of range for MODERATE tests.
Consider setting timeout="short" or size="small".
```
This change adds size="small" to avoid such warnings being reported.
---------
Co-authored-by: Chandler Carruth <chandlerc@gmail.com>
This is pretty rough both in code-quality and what it tries to cover.
The coverage approximation is probably fine as it mostly just gives some
vague idea of the distributions we should aim at when evaluating
performance. We still want performance with unusual distributions to be
good.
Happy to improve the code quality if folks want, and especially with
suggestions on what would help. For this kind of quick scripting, it
seemed "fine" to me.
### Description:
This pull request introduces two significant changes:
**Improved Error Handling in `remap_file` Function:**
- The error handling in the `remap_file` function has been simplified
for improved clarity and user-friendliness. The original code included a
combination of `assert` and `exit`, which could be perplexing during
runtime.
**Changes Made:**
- Replaced the original `assert` and `exit` with a more straightforward
`if-else` block for handling errors.
- Utilized `sys.exit` for clear error messages and proper program
termination when an error occurs.
**Why I Did It:**
- Improved Clarity: The original code's `assert` primarily served
debugging purposes and might not behave as expected during runtime. The
new code offers clarity and predictability.
- User-Friendly Errors: The updated code provides users with easily
understandable error messages.
**Added `.DS_Store` to `.gitignore`:**
- `.DS_Store` files are commonly generated by macOS Finder to store
folder-specific metadata. They do not need to be tracked in version
control.
**Changes Made:**
- Added `.DS_Store` to the `.gitignore` file.
**Why I Did It:**
- To prevent `.DS_Store` files from being tracked in the Git repository,
ensuring a cleaner repository and avoiding unintended commits of
macOS-specific files.
### Improvements:
- **Better Error Handling:** The new code in the `remap_file` function
ensures the program exits gracefully with informative error messages
when errors occur.
- **Enhanced Code Clarity:** The code now communicates its intentions
more clearly with `sys.exit` for handling expected runtime errors.
- **Cleaner Git Repository:** The addition of `.DS_Store` to
`.gitignore` prevents the tracking of macOS-specific files in version
control.
This is a rough script that uses regexes to do a simple scan of source
code and extract some basic source code statistics. Things like column
width, comment line density, identifier lengths and densities.
After scanning, it prints out both raw stats and in a few cases renders
a quick histogram to the terminal to help visualize a relevant
distribution.
I threw this together pretty quickly, and this is an area of Python I
have very limited familiarity with, so happy to have any suggestions for
how to better approach this.
Add a language server for carbon as part of GSoC.
This currently does code outline using toolchain parser.
See development steps in utils/vscode/README.md for running and using
language server.
Where there was an object initialization immediately followed by
assignment I condensed it to one line, which seems to be the convention
looking at other files. I also deleted the repetition of 'of' in some
c++ comments because it was grammatically incorrect.
---------
Co-authored-by: Clayton Gearhart <claytongearhart240@gmail.com>
I've migrated the toolchain autoupdate scripts here, I just need a
little more time to do the explorer side (which I need to check
performance, that may require multithreading as we do in the current
script). However, this felt substantial enough to share and it means I
can handle autoupdate in more of the toolchain, including preparatory
work for autoupdate on multi-file tests.
Once explorer is done I'll remove the old script.
I'm integrating absl flag support with a few thoughts here...
1. It simplifies the handling of files in file_test.
- Removes the need for a separate subset target.
3. Looking forward, I'm planning to add a flag to allow for autoupdate
of golden files.
4. In explorer, there's been confusion about having file_test run tests
"twice" so hopefully it's clearer when it's a separate target with a
different flag on the target.
protobuf deps have become complexly structured, but AFAICT this is still the preferred dep. Also fixes some the fuzzer include checking, but that's a trivial misconfig.
LLVM's bazel build has changed a bit, so this updates the tree for that.
LLVM is also moving `llvm::Optional` to match the standard API, but it seemed simpler to just switch to `std::optional`.
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
This does some more work to the run_clang_tidy.py wrapper script, and runs an example pass.
"again" because it's really the proto fuzzer changes that broke it, it had been working before.
"mostly" because there's still an issue within the proto fuzzer that it can't find "port/protobuf.h", i.e. https://github.com/google/libprotobuf-mutator/tree/master/port, but I'm still hesitant to add an include path there.
run_clang_tidy.py (the clang-tools-extras version) is parallel so should be feasible to work with. I'm trying to codify this mainly because finding out the right invocation can be difficult.
This fixes some loading of re2 and proto that's started to become an issue.
It's hard to see actual issues because of the identifier length warning being turned off in #2244, and just general issue creep. There may be more fixes to do but this should still be an improvement.
This will allow us to automatically create and maintain specific repos
based on the main repository here, pulling key files like the license
and other infrastructure and pushing them systematically to a narrow
repo. Things like editor plugins that are best packaged and installed
from a separate repos can still be developed in a central place, even
potentially sharing common things like grammars where useful.
Currently this will maintain a Vim plugin repository out of the
`utils/vim` directory, but can be easily expanded for other systems.
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).
I'm treating my PR to mypy bazel integration as dead in the water. Abandoning that approach, this seems to work reasonably well as a replacement. It's actually a slightly newer version of mypy too -- the bazel integration was stuck on an old version.
Note I don't think we're losing much: we shouldn't be writing too much python.
The logic is using bazel Runfiles API per Jon's suggestion.
One of the scenarios used by the fuzzing framework is to place the binary and its runfiles into a folder, resulting in a directory layout like this:
[temp dir]
fuzzer
fuzzer.runfiles/
Tested:
bazel test -c opt explorer/fuzzing:explorer_fuzzer
bazel build -c opt explorer/fuzzing:explorer_fuzzer + run the binary from various locations
I went down this route because I figured the installer binary should be in `check_deps`. The result:
- Add thin wrapper scripts for `buildozer` and `bazel`, similar to the `buildifier` wrapper that's already there.
- These are mainly to avoid deduplicating logic, and dodging intricacies of Python imports that I don't understand well through instead subprocessing.
- Fix update_roots.py (it was missing a `.parent`, must've been relocated without a re-run at some point)
- Fix fuzzer proto code to be `testonly`
- Update the `check_deps` targets
* finished fuzzer and added fuzzverter util
* fixed typo
* renamed cmd line params
* fixed libproto_mutator download path
* small fixes
* small fixes
* small fixes
* renamed sample corpus proto
* small fixes
* try building on github with LIBCPP_DEBUG enabled
* temporarily marked proto fuzzer as a manual test
* code review
* use a dedicated proto-fuzzer feature to work around LIBCPP_DEBUG=1 crash in proto code
* code review comments, added README.md
* minor fixes to the text
* Update bazel/cc_toolchains/clang_cc_toolchain_config.bzl
Co-authored-by: Jon Meow <jperkins@google.com>
* use Carbon source representation for "empty Main()" instead of text format proto representation
* fixed typo
* made FuzzerUtil produce the full carbon source (proto converted + Main if needed) to decrease code duplication a bit
* typo
* switched to text proto format per code review
* Update executable_semantics/prelude.h
Co-authored-by: Jon Meow <jperkins@google.com>
* Update executable_semantics/fuzzing/README.md
Co-authored-by: Jon Meow <jperkins@google.com>
* Update executable_semantics/fuzzing/README.md
Co-authored-by: Jon Meow <jperkins@google.com>
* Update executable_semantics/fuzzing/README.md
Co-authored-by: Jon Meow <jperkins@google.com>
* Update executable_semantics/fuzzing/README.md
Co-authored-by: Jon Meow <jperkins@google.com>
* Update executable_semantics/fuzzing/README.md
Co-authored-by: Jon Meow <jperkins@google.com>
* review comments
* removed unnecessary file mode variables
* Update executable_semantics/fuzzing/fuzzverter.cpp
Co-authored-by: Jon Meow <jperkins@google.com>
* Update executable_semantics/fuzzing/README.md
Co-authored-by: Jon Meow <jperkins@google.com>
* Update executable_semantics/fuzzing/README.md
Co-authored-by: Jon Meow <jperkins@google.com>
* code review comments
* Update executable_semantics/syntax/BUILD
Co-authored-by: Jon Meow <jperkins@google.com>
* buildifier
Co-authored-by: Jon Meow <jperkins@google.com>
* initial fuzzer proto
* visibility change
* use newer protocol buffer version which has the defs.bzl bug fixed
* Adjusted fix_cc_deps to work with protobuf external repo
* explicitly load rules_cc to avoid a frozenset bug in the version loaded by protobuf
* use explit deps, use llvm's zlib
* restored cxx settings
* deps change
* Cleaned up WORKSPACE and changed the test to read carbon sources from testdata
* updated comment
* adapted to new ErrorOr return value
* proto buffer 3.19.2 -> 3.19.4
* changed comment
* Apply suggestions from code review
Co-authored-by: Jon Meow <jperkins@google.com>
* Apply suggestions from code review
Co-authored-by: Jon Meow <jperkins@google.com>
* Update executable_semantics/fuzzing/BUILD
Co-authored-by: Jon Meow <jperkins@google.com>
* addressed review comments
* updated Unimplemented error message
* Addressed review comments
* more review comments
* switched to loading protobuf via rules_proto()
* Ignore protobuf headers in fix_cc_deps.py until the script supports alias rules
* renamed repeated proto fields to be plural
* added @zlib to check_non_test_cc_deps
* Update common/fuzzing/BUILD
Co-authored-by: Jon Meow <jperkins@google.com>
* review comments
* set is_omitted_expression for return value
Co-authored-by: Jon Meow <jperkins@google.com>
I'm wondering if my issue all along was parallel script executions (I reproduced the error message locally with `pre-commit run -a`, and I think in my case that's what happened).
Adding the file size to errors for sanity checking, since part of what had me looking again was checksum mismatches, and actually the parallel execution offers a consistent story there (checksum mismatch due to file being modified by another download mid-stream).
Moves common script logic into utils.py (not a great name, but couldn't come up with better). This is in particular to make the buildifier.py script really trivial, allowing that pre-commit to be easily added. However, scripts have also been diverging on how we find bazel, so I'm trying to unify that.
The advantage of reimplementing buildifier's pre-commit is that (a) we can now run buildifier server-side, and (b) we can stop advising installing it manually. Then the only Linux-specific package manager is Cargo, which is only used for watchman, which is optional -- so stop highlighting Linux-specific package managers in the tool instructions.
`go install` is a bit troublesome: there's old versions floating around that are incompatible with the `@version` use, and the cache it creates isn't writable so temp reapers can create inconsistent, broken states.
This switches approaches to download bazel's releases directly. This should still have the necessary platform support, but without using the `go` tooling and with finer control over cache results.
Jon Ross-Perkins
Chandler Carruth
Jack McCluskeyandJon Ross-Perkins
Jonathan B. CoeandChandler Carruth
m3ndax
maan2003
Clayton GearhartandClayton Gearhart
Enzo Venturi
pk19604014
Jon Meow