Note this makes some structural choices that I'm not sure how popular they'll be... Most notably, I could remove `DeclarationKind` because I'm using `std::visit` and `Declaration::Visitor` in reality. I could go to `switch(tag())` instead, but I'm wondering if this approach will be well received for the separation of ownership. Alternatively, I could move `Declaration` around so that it actually implements the things like `TopLevel` -- I did feel weird with the old structure of typecheck.cpp implementing members of declaration.h, though.
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.
We will eventually need the ability to materialize temporaries eventually, but we don't really seem to need it yet, and when we do, I think we should do it in a way that gives uniform treatment to temporaries and local variables.
Also:
- Switch data members, and associated parameters and return types to be values/references rather than pointers, where applicable.
- Move Expression's TagVisitor to anonymous namespace, to avoid name collision.
- Switch to trailing return syntax (per style guide).
Propose the decision from #542, noting implementation from #563
Also integrates some of #339 into `variables.md` because that's actually how this started, looking for a proposal reference for #542
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Co-authored-by: josh11b <josh11b@users.noreply.github.com>
Pretty much any names will be awkward, since these types represent cases where an `Expression` is not an expression, and a `Value` is not a value, but we can at least be more explicit about the fact that they represent bindings rather than variable usages.
- `Kind` enumerator names always match the corresponding factory function, accessor, and type names (if any).
- All abbreviations in those names are expanded.
- Those names always have a suffix to disambiguate expressions from values.
`VarTV` is excluded from these changes because there's a pending PR to remove it.
This ensures that the factory functions correspond 1:1 with expression kinds, and MakeOp covers their use cases with minimal syntactic overhead now that it can take initializer list arguments.
* moved most of logic from HandleValue into the Step functions
* removed the HandleValue function
* added some comments
* responses to Geoffrey
* move increment of act->pos out of the step logic for value actions
* changed act->pos to start at 0 and not have any gaps
* Update executable_semantics/interpreter/action.h
Co-authored-by: Geoff Romer <gromer@google.com>
* update comment based on Geoffrey's suggestion
Co-authored-by: Geoff Romer <gromer@google.com>
The built-in GitHub action cache is too broken for us to use currently.
Pull request runs will evict trunk runs from the cache easily, which
will remove a useful baseline and make builds generally uncached and
extremely slow.
Instead, move to using Bazel's remote caching functionality with a GCP
storage bucket. This gives us nearly limitless storage and is actually
vastly simpler than the prior arrangement. It should also allow cache
sharing between different build configurations and other benefits.
This cache isn't usable by anyone else sadly, but it should at least
provide a better scaling technique for our CI.
This does require us to move from `pull_request` workflow to
a `pull_request_target` workflow and run the CI within the domain of the
project. This is only really safe for us to do while the project remains
private. When we go public we'll want to make the cache publicly
readable and change this code so that when not running on a branch in
the main repository we use the cache in a read-only mode and only write
back updates for branch runs. This should still be effective but
requires a bit more complexity here and in the remote cache including
setting up public access. For now, going with the simple if slightly
less secure model.
Geoff RomerandJon Meow
josh11b
Jeremy G. Siek
Richard SmithandChandler Carruth
