This is just a cleanup. Since we now have a testing directory, I think this is a better home for testonly libraries than //common. (I was thinking about this when I was considering adding more test_raw_ostream deps)
Also make minor updates to the skeletal design in
docs/design/name_lookup.md following #2113, as there are no longer any prelude names that are made available to unqualified name lookup by default.
Add `type` to the keyword list in
docs/design/lexical_conventions/words.md, following #2360.
I'm thinking about how to handle multiple files, and I think the current IRFactory is useful as a file-focused thing. So shifting/renaming accordingly. (doing this in its own PR to make the history a little cleaner for git's move detection)
Note, not trying to address every last error, just some obvious/easy ones.
```
/usr/local/google/home/jperkins/dev/carbon-lang/common/string_helpers.cpp:200:13: warning: prefer transparent functors 'less_equal<>' [modernize-use-transparent-functors]
auto le = std::less_equal<const char*>();
^
/usr/local/google/home/jperkins/dev/carbon-lang/toolchain/semantics/nodes/function.h:22:45: warning: pass by value and use std::move [modernize-pass-by-value]
Function(ParseTree::Node node, NodeId id, llvm::SmallVector<NodeRef> body)
^
/usr/local/google/home/jperkins/dev/carbon-lang/explorer/ast/declaration.cpp:230:14: warning: static member accessed through instance [readability-static-accessed-through-instance]
return cast<SelfDeclaration>(declaration).name();
^
/usr/local/google/home/jperkins/dev/carbon-lang/explorer/interpreter/type_checker.cpp:1104:33: warning: std::move of the variable 'impl' of the trivially-copyable type 'ConstraintType::ImplConstraint' has no effect [performance-move-const-arg]
impl_constraints_.push_back(std::move(impl));
^
/usr/local/google/home/jperkins/dev/carbon-lang/explorer/interpreter/type_checker.cpp:1145:36: warning: std::move of the variable 'rewrite' of the trivially-copyable type 'ConstraintType::RewriteConstraint' has no effect [performance-move-const-arg]
rewrite_constraints_.push_back(std::move(rewrite));
^
/usr/local/google/home/jperkins/dev/carbon-lang/explorer/interpreter/type_checker.cpp:1156:32: warning: std::move of the variable 'context' of the trivially-copyable type 'ConstraintType::LookupContext' has no effect [performance-move-const-arg]
lookup_contexts_.push_back(std::move(context));
^
/usr/local/google/home/jperkins/dev/carbon-lang/explorer/fuzzing/fuzzverter.cpp:74:42: warning: argument name 'trace' in comment does not match parameter name 'parser_debug' [bugprone-argument-comment]
/*trace=*/false);
^
./explorer/syntax/parse.h:19:17: note: 'parser_debug' declared here
bool parser_debug) -> ErrorOr<Carbon::AST>;
^
/usr/local/google/home/jperkins/dev/carbon-lang/explorer/fuzzing/proto_to_carbon_test.cpp:34:55: warning: argument name 'trace' in comment does not match parameter name 'parser_debug' [bugprone-argument-comment]
const ErrorOr<AST> ast = Carbon::Parse(&arena, f, /*trace=*/false);
^
./explorer/syntax/parse.h:19:17: note: 'parser_debug' declared here
bool parser_debug) -> ErrorOr<Carbon::AST>;
^
/usr/local/google/home/jperkins/dev/carbon-lang/explorer/fuzzing/proto_to_carbon_test.cpp:42:41: warning: argument name 'trace' in comment does not match parameter name 'parser_debug' [bugprone-argument-comment]
&arena, f, source_from_proto, /*trace=*/false);
^
./explorer/syntax/parse.h:25:59: note: 'parser_debug' declared here
std::string_view file_contents, bool parser_debug)
^
/usr/local/google/home/jperkins/dev/carbon-lang/explorer/syntax/parse_test.cpp:27:60: warning: argument name 'trace' in comment does not match parameter name 'parser_debug' [bugprone-argument-comment]
ParseFromString(&arena, "file.carbon", FileContents, /*trace=*/false);
^
./explorer/syntax/parse.h:25:59: note: 'parser_debug' declared here
std::string_view file_contents, bool parser_debug)
^
/usr/local/google/home/jperkins/dev/carbon-lang/migrate_cpp/cpp_refactoring/var_decl.cpp:57:58: warning: string concatenation results in allocation of unnecessary temporary strings; consider using 'operator+=' or 'string::append()' instead [performance-inefficient-string-concatenation]
segments.push_back({type_loc_class, qual_str + " " + range_str});
^
/usr/local/google/home/jperkins/dev/carbon-lang/explorer/fuzzing/ast_to_proto_test.cpp:105:55: warning: argument name 'trace' in comment does not match parameter name 'parser_debug' [bugprone-argument-comment]
const ErrorOr<AST> ast = Carbon::Parse(&arena, f, /*trace=*/false);
^
./explorer/syntax/parse.h:19:17: note: 'parser_debug' declared here
bool parser_debug) -> ErrorOr<Carbon::AST>;
^
```
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.
The approach uses a RecursiveASTVisitor rather than matchers. Matchers
and callbacks do not compose neatly and introduce significant runtime overhead
over RecursiveASTVisitor when an action needs to be performed on most nodes.
In Python 3.x it is not necessary to have `object` inheritance in classes, it may be useful to have `object` inheritance in classes if the code has support for Python 3.x, 2.x, but currently the present code does not contain support for python 2.x.
Co-authored-by: martimartins <martim13artins13@gmail.com>
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.
This moves over to the vanilla upstream GoogleTest pulled in the more
expected manner with Bazel. It also adds Abseil and Google Benchmark
libraries in the same fashion (there are cross dependencies here).
As part of this, also introduce a dependency check test that can enforce
basic layering of dependencies. For example, this lets us ensure that
non-test Carbon code only depends on LLVM and Clang despite having other
libraries available. There remains some cleanup to improve the way these
dependency tests work, but this at least ensures we don't regress.
I've also provided workarounds to allow both Carbon code and LLVM code
to freely be used with GoogleTest (and other `std::ostream` based
output code). This is done by extending the code in
`//common/ostream.h`. One downside is that it requires opening the
`llvm` namespace and adding an ADL_found overload there. I think on
balance this is still a win and doesn't make me too nervous.
The new version of GoogleTest requires printing more often from matchers
and so I've also added several printing routines to types that
previously didn't require them. Otherwise, most of the updates are just
using the more conventional upstream style of including the headers and
adding `ostream.h` where it is needed.
I did consider moving code over to use `std::ostream` instead of LLVM's
`raw_ostream`, but the advantages of not doing virtual dispatch still
seem significant, and it also seems good to retain access to LLVM's
formatting utilities built around `raw_ostream` given that we can't pull
arbitrary dependencies into Carbon code outside of test code.
All of this was slightly motivated by requests for newer features in
GoogleTest, but much more-so by my desire to have access to Google
Benchmark and Abseil when writing benchmarks. For example, using
Abseil's random number generator seems extremely helpful when generating
inputs for benchmarks. The growing dependencies between these packages
further motivated me to just pull them all in and ensure they worked
well.
I'm seeing if I can upstream thundergolfer/bazel-mypy-integration#43, but we can also point at my fork for the time being.
This should resolve conflicts with mypy treating imports as non-hermetic, creating inconsistent behavior if packages are/aren't installed locally.
Per https://github.com/bazelbuild/rules_cc this still isn't necessary. There's no build-time enforcement, so usage is inconsistent/incorrect. Rather than letting this linger, remove it pending Bazel tooling enforcing it.
This replaces GetAstMatcher with AddMatcher because cxxForRangeStmt is a StatementMatcher. addMatcher has multiple definitions (https://clang.llvm.org/doxygen/classclang_1_1ast__matchers_1_1MatchFinder.html) and so this approach allows using the right addMatcher without writing per-call overloads.
To handle the `var`, I'm considering something like moving VarDecl logic into a VarMatcherBase so that I can just use CXXForRangeStmt's getLoopVariable. The problem is a for-range statement has multiple VarDecls, and getLoopVariable may be the easiest way to identify the real one.
Range-based for loops remain messed up -- I'm just disabling the test here and turning a blind eye.
Co-authored-by: Chandler Carruth <chandlerc@gmail.com>
TODOs in the test for known issues.
I may switch the approach to getting the variable type (on examination, this isn't working quite as well as I'd thought) but for now I think it's okay. I had an earlier approach though that may work better overall -- I'd been thinking this would work better, but as you can see in the null check for type information, I think I missed a key point.
Anyways, what'd really been vexing me was `int i, j` which I think I handle passably well now. There's obviously room for improvement, but given I've been going at this for a couple days now, I thought it best to checkpoint where I was.
This also includes some related framework changes to fix bumps I was running into. Overall the tool should operate a bit more smoothly with these changes. There are still issues with overlapping replacements, but I think it's primarily with range-based for loops which I just need to take some time to fix.
Now that LLVM 12 has been released we no longer have any need to
bootstrap LLVM to get the desired featureset. LLVM 12 is available
widely, including in Homebrew across multiple platforms and in the
GitHub action runners.
Sadly, the Linux distribution builds of LLVM-12 are largely broken and
not as useful for us. The Homebrew Linux install was also broken
originally, but I've worked extensively with the Homebrew folks to get
the Linux install into a really good shape. It should now work reliably.
There are two primary bugs in Linux LLVM packages that need to be fixed
before we can just use them:
- https://bugs.llvm.org/show_bug.cgi?id=43604
- https://bugs.llvm.org/show_bug.cgi?id=46321
Once those are addressed and point releases with the fixes widely
available we can further simplify things.
Even with the need to use Homebrew installs, using the released LLVM has
the extra advantage of making it easy to properly support Darwin ARM and
I've added that configuration so that I can test things there.
Last but not least, this will significantly shrink our build outputs
which should allow building much more in continuous integration on
GitHub actions without exceeding the action cache size limits. I've even
added several tweaks and adjustments to the compile and build flags to
improve the build performance and reduce the build output size.
Once this is landed and stable, we can consider adding the refactoring
tooling back to our CI.
One of the biggest downsides of this path is that our CI has to download
and install the LLVM toolchain from Homebrew on each run. This is pretty
slow (takes a couple of minutes). But it is a fixed overhead -- it won't
get worse over time. Eventually, we can either look at a much fancier
action configuration to avoid this or hopefully the Debian packages will
get updated and we can move back to those.
The bootstrapping has served us long enough at this point. We can
resurrect it if we ever find a compelling reason for breaking off of the
latest LLVM release as our host toolchain.
Co-authored-by: Jon Meow <46229924+jonmeow@users.noreply.github.com>
This replaces the use of `isExpansionInMainFile` with instead taking the list of input files, and only touching them. This doesn't keep `isExpansionInMainFile` because it should be redundant, and as such it'd be easy to forget. Unless it starts being a performance issue, it's probably better to omit.
This might need some customization, e.g. `woff2_decompress.impl.carbon` (as a file with a `main`) might be better as `woff2_decompress.carbon`, but I thought at least putting on `.carbon` extensions would help perspectives on files.
This restructures the script to make it easier to access standard info (e.g. cpp_files) without passing everywhere.
This picks up a newer version of LLVM and the LLVM Bazel integration.
The big change here is that we can configure the LLVM targets that are
built, which allows us to dramatically reduce the build costs by
focusing on a couple of CPUs for the time being.
There are a few API updates needed as well.
This also rotates the cache version so we start with a clean Bazel cache
from here. Otherwise we'd potentially pay the cost of carrying around
stale bits of LLVM endlessly.
What this does:
- Sets up a `migrate_cpp` tool which currently only runs `clang-tidy`.
- This is intended to have more transformations in the future.
- Sets up a `migrate_cpp.sh` script.
- This copies the original woff2 code into a `carbon` directory and runs the `migrate_cpp` tool on it there.
- Adds the initial `carbon` directory of woff2
- To be clear, this is currently only updated via `clang-tidy`.
- More transformations should be expected in the future.
- Minor related edits. For example:
- Adjust pre-commit to skip the `carbon` directory, because it's third-party code and shouldn't be edited in the same way.
- Adds `@brotli_carbon` as a local repository so that we can "build" outputs.
- Makes clang-tidy from the bootstrap toolchain accessible for BUILD dependencies, as it's then used for `migrate_cpp`.
What this does not do:
- Any actual transformation of C++ code to Carbon
Co-authored-by: Matthew Riley <mdriley@gmail.com>
Jon Ross-Perkins
Richard Smith
SADIK KUZUandJon Ross-Perkins
Chandler Carruth
Andy Soffer
Martim Martinsandmartimartins
Josh Soref
pk19604014
Jon Meow
