Only the special nullability attributes (`_Nonnull`) work correctly
through type aliases like we're using with `Ptr`. But they aren't
strictly UB and so have to be specially enabled in our sanitizer config
in order to usefully catch nullness errors early. Turn on those
sanitizers as well.
Also, now that we are using fully remote build output caching for our
CI and not trying to squeeze under an arbitrary size limit, re-enable
the nice error messages for all the UBSan checks.
Note that this will have a (very) slow CI run as it will have to
recompile ~everything and upload fresh artifacts. But those should then
be effective cache hits going forward.
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.
There's a small update to update_checks.py to handle the recursive directories. Also, I'm only using one level of nesting in this PR but really no reason we can't do more. I'm just not sure what clustering is best right now.
As a pattern, I'm trying to name all failing tests `fail_*.carbon`.
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.
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>
Also apply it to the list of tokens. This way there's no need for "sort order" comments.
This was a side-effect of me trying to merge in api/impl/library/package, and thinking "why am I doing this manually?"
Co-authored-by: Geoff Romer <gromer@google.com>
Doesn't add much logic, only takes advantage of parser structure for the ordering enforcement.
Note import_nonexistent tests should probably fail, but writing import tests needs a chain of functionality, and I figured I'd just start adding some to validate the syntax (not adding existent imports because that'd require multi-file structure).
This is based on discussion on #732: that we should probably parse the invalid whitespace, then reject it as part of string validation, rather than having different parses. I worry the question of "how is this parsed" may lead to subtly unexpected results if we aren't consistent, so I'm switching the logic from the lexer to the unescape library (and also adjusting the list of rejected whitespace).
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).
Related to discussion on #738, but also trying to standardize handling of everything and make placement of token spellings (`AND "and"`) consistently in lexer.lpp.
I could have gone the other direction, removing the list of things in lexer.lpp, but this feels like it does more to use the compiler to detect skew between lexer.lpp and parser.ypp.
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.
- brew instead of pyenv to centralize package management
- can't recall why we used pyenv before, may not have been the best choice.
- pyenv has been a burden in updating versions, a single `brew upgrade` works better
- python3.9 because it's the latest and greatest, feels weird going to old versions if we're recommending installs.
- bazel 5.0.0 pre-release due to https://github.com/bazelbuild/bazel/commit/b9fc66d327debcfbdb2964afdba35a1cc8919b81
- earlier versions want `python` to be on the path, which pyenv did but brew does not
For most people this will mean: `rm -rf ~/.pyenv && brew uninstall pyenv && brew install python@3.9 && pip3 install -U pip && pip3 install pre-commit`, plus removing any `pyenv` invocations from the shell `rc` file.
Co-authored-by: Chandler Carruth <chandlerc@gmail.com>
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 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.
I think Stack used to use ListNode, but you'd previously removed that dependency. This finalizes the split, and merges ListNode so that we don't accidentally accrue more dependencies. In part I think we'll eventually want to change how Dictionary is set up.
This opens up a path for switching `int line_num` to a `Location loc`, which I want to do for tracking filenames of code. But I don't think we should have a default constructor on `Location` to avoid mistakes, and switching FunctionDefinition to an arena alloc seems more consistent anyways.
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>
Jon Meow
Chandler Carruth
Geoff Romer