Rationale: Based on the status of #478 and #505, Carbon won't have this feature for a while, and it will be simpler not to support it on spec in the meantime.
`// NOLINT` is added on type aliases in stack.h and dictionary.h to allow lower_snake_case naming -- this didn't feel like a check worth disabling in spite of false positives.
Co-authored-by: Geoff Romer <gromer@google.com>
Prior to this change, `__await` would make a deep copy of the continuation stack, but shallow-copy the individual stack frames within it. As a result, continuations appeared to have shallow semantics so long as the continuation stack had only a single frame.
This change also removes an obsolete test from the brief period when we intended continuations to have deep-copy semantics, which has been passing basically by accident.
This does a mass rename of:
- `SourceLoc()` -> `source_loc()` for property naming
- `loc` -> `source_loc_` for underscore+consistency
- Generally changing function args to `source_loc` for consistency
- `Tag()` -> `kind()` for property naming and `Kind` parity
- `tag` -> `kind_` for underscore
Also renames `Pos` and `Results` on `Action`. These are a bit of an exception in that most base classes only have `Tag` and maybe `SourceLoc`, whereas `Action` has a little more. I felt okay having `source_loc()` and `kind()` on the base class where children do `Exp()` and the like, but it felt weird to me to mix it on the same class.
The reason for doing this cross-class in one PR is so that I can do it efficiently with a global replace in the codebase, rather than e.g. changing `Expression` but having to read through compiler errors to determine where it's calling `Expression`'s `Tag` versus a different `Tag`. The end result should be equivalent.
The code is pretty intertwined: having the AST be truly mutable means (to me) changing parser.ypp to return non-const values, but then the way things are passed around between objects should be non-const (particularly an issue with lists), which then creates issues with construction of lists in the TypeChecker, which then TypeChecker needs to mostly be non-const.
Due to the difficulties in breaking this apart, whereas I'd previously considering refactoring accessor naming in the same PR, I've largely avoided doing so. The intent is then that this PR focuses mainly on const -> non-const AST behavior.
call_main moves out of interpreter.cpp so that interpreter.cpp can receive a fully const AST.
Revives BisonWrap because this seems a reasonable use of it (avoiding the need to have an std::optional or pointer for Alternative, both of which I thought could be unclear about the intent).
The advantage is a C++ pointer is special, and this approach eliminates the Ptr class type that was causing problems in conversions. Attribute suggestion was courtesy of chandlerc. We're sticking with the Ptr name because it's shorter than Nonnull, and we're likely to keep this in lots of places.
With this, only main.cpp instantiates an arena. Maybe we'll want to split that up more later (e.g., so that the runtime interpreter uses its own arena), but given the intent to have type-checking update the AST, I thought this was a reasonable approach for now in order to avoid ownership complexities.
Fixes#769
* Debugging quality-of-life improvements
- Use std::abort for `CHECK`/`FATAL` failures, which acts as a debugger breakpoint as well as automatically printing a stack trace.
- Re-enable printing continuations in `--trace` mode.
- Log the source location of each step in `--trace` mode.
Co-authored-by: Jon Meow <46229924+jonmeow@users.noreply.github.com>
Along with #789 this addresses most of #769 although global_arena is still a TODO (that's widespread and overlaps with other changes so I wanted to do it after these are in).
Note this makes a few cases where the Statement was optional explicit (Block, If, Sequence). I do add a few CHECKs around where statements were optional and assumed but unverified.
I switch TypeCheckStmt to not take an optional Statement because I think it makes the call sites clearer in behavior. It's also a smaller change than the converse, because taking an optional Statement means the returned statement would also need to be optional. Arguably a wrapper for optional statements could be added, but this still seems cleaner to me, and there aren't that many cases of an optional statement.
Co-authored-by: Geoff Romer <gromer@google.com>
Sorry about the big change, this is hard to split. ParenContents is used by both, templated, and expects the same pointer type. While I could duplicate ParenContents with some ExpressionParenContents or PatternParenContents, that seems a little kludgy versus a single large change handling both. The worst of it is that Expression is already pretty sweeping, Pattern is really just incrementally adding.
That said, I believe this includes a couple fixes I found with incorrect use of dyn_cast in typecheck.cpp (checked nullptr at the wrong step in 2 code locations). There's also a missing `*` in member.cpp this caught. I adjust passing of expressions for Return due to nullness (I felt adding another constructor was the best solution).
I add a `.Release()` to BisonWrap due to things like `$3.first` needing some way to work through BIsonWrap. I felt this was better than `operator->`, but feel free to comment if you prefer the other path (`.Release()` conveniently lets me do pair unwrapping, so it felt a better solution).
I do add a TODO to think about better Ptr-to-Ptr cast<> support too, though, as that doesn't work cleanly with LLVM's infra. But so far it seems to only come up in one spot, so I'm not prioritizing it.
This enables the interpreter logic to express its intent more directly, especially in the common cases, and enables us to get rid of ValAction. It's also a step toward simplifying and encapsulating `state->stack`.
This also eliminates the ctad wrapper for Stack: I think the leaning is to remove it. It felt worth keeping the constructor because constructing with a single element is a common use-case.
Adds a single-argument constructor for Scope because the `std::list<std::string>()` is common, and eliding it is consistent with what we've done for things like tuples.
I was considering a vector constructor due to the double-Push on line 1139, but thought the Push() semantics may mean that it's better not to provide.
It was in my mind to add String in order to support libraries in `package`. `print` is added in order to have a String go to stdout. I've tried to do `print` in a way that won't be too hard to add other printable types, but it's probably also somewhat optional here -- that is, if desired, I could remove it. But it was a lot easier to doublecheck `\n` behavior with it, and I suspect it'll be helpful in other tests if it supports more value types.
On the side, this also fixes dereferencing in Pattern/Expression Print() calls, which I was noticing printing pointers instead of values. This may be another argument for moving away from passing pointers, since this seems to be a difficult-to-catch error.
Co-authored-by: Geoff Romer <gromer@google.com>
This creates a ReturnExpression. A separate change should enforce that an implicit return of `()` is only allowed in functions that have an implicit return type of `()`, and I think the structure taken here should ease that.
`Pattern` is intended to pilot some changes I would like to apply to all our sum types:
- The alternatives are expressed as derived classes rather than members of a `std::variant`.
- The alternatives are classes in the [style guide sense](https://google.github.io/styleguide/cppguide.html#Structs_vs._Classes), meaning they can have invariants, but can't have public data members.
- Creating an object is expressed using a constructor rather than a factory function.
- Accessing an alternative is expressed as a cast (using LLVM's RTTI system) rather than `std::get` or a `Get` method.
Co-authored-by: Jon Meow <46229924+jonmeow@users.noreply.github.com>
Jon Meow
Geoff Romer