At present, `var *;` is a crash because it has errors that weren't being properly stored/handled. This changes the handling to give more to track invalid parses in patterns.
I'm suspicious this is responsible for some OOMs in fuzzing... Specifically, it dumps a really big parse tree, then the auto fuzzing system OOMs trying to cache the entire output in memory.
This handles the basics of type and value for structs. Structurally, these look like parameters and arguments (respectively) because expressions/generics may result in multiple IR nodes being generated.
Because `{}` needs to be cast to a type for storage, I'm also adding some validation that's not specific to `{}`, e.g. that `1` shouldn't be valid as a type for storage (previously, nothing errored for that).
This adds more stringification of types, particularly literals, because they come up in value errors now.
ImplicitAs is the result of me mulling whether I'm taking the right approach on type conversions. I think it needs to return a value so that if the implicit cast rewrites the value, the result is accessible to the caller. I may reorient the current TryTypeConversion logic to be more based on the ImplicitAs logic.
This PR is making two main changes to the Explorer and Toolchain:
- Replace the `is` keyword in `where SomeType is SomeInterface` with `impls`, so it is `where SomeType impls SomeInterface`
- Rewrite uses of the "impls" to something else to avoid, frequently "`impl` declarations" or "implementations", to avoid confusion with the `impls` keyword.
---------
Co-authored-by: Geoff Romer <gromer@google.com>
I'm trying to reduce the amount of per-SemanticsNodeKind boilerplate, and make mistakes (e.g., SemanticsNodeKind not matching the Make name, misplacing the type, or Get/Make type mismatches) easier to see.
I could've done this with (more) macros, but felt that the template approach was reasonable enough and likely easier to understand/debug. I'm not sure whether there's more that I could be doing with variadics to reduce the amount of factory code, but this feels good right now.
These changes should make output more stable when builtins are added to semantics. By omitting them from nodes and printing nodes as "relative to the last builtin", I should be able to add and remove builtins without automatically affecting every test. Also by printing builtin nodes as `nodeNameOfBuiltin`, it's a little easier to understand what's going on (for me, at least).
`self` wasn't being handled in expression logic, only parameter logic. This is needed for `self.x`.
Also renames SelfDeducedParameter to SelfIdentifier because it's parsed fairly consistently with Identifier; this better represents the situations where `self` will need to be handled, it's where `Identifier` is allowed (although `Identifier` could also refer to a type... but I still think `SelfDeducedParameter` is a more difficult to understand name).
This was bugging me after I saw all the strings; it feels like this is why we have EnumBase on the toolchain side.
I've included the move of EnumBase to //common because I figured it's reasonable to evaluate together; if we don't want EnumBase in this case, it doesn't make sense to move.
I've been heading this route with other parts of the parser because the overhead of adding enums and then switching on them felt tedious, and odd from a performance perspective to make calls when the caller knew the value to use. My leaning is towards this approach that makes it clearer what's actually different between the modes, and allows removing BraceExpressionKindToParserState. It's a mild code size decrease.
Also cleans up some comments about related parse nodes. Currently basic and not heavily validated.
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
This makes it possible to specify both deduced and regular parameters on types. It reorganizes the handling of parameter lists in order to allow more reuse of code in this approach. Both functions and types use the new DeclarationNameAndParams handling. Overall the goal here is to take advantage of commonality in structure.
Regarding destructors, the likely approach would be to use ParameterListAsDeduced directly because `destructor` is a keyword with no declaration name and no regular parameters.
Unused diagnostics were incorrectly always returning "false" regardless of whether there was an issue. It was still looking for registry file changes, not kind file changes. Also, this should only be run once per pre-commit run (pass_filenames=false).
Remove the one unused diagnostic.
This is just a mild simplification to address repeat macro use. I'm hoping it makes it clearer and easier in parser_state.def to write down the multiple variants.
I'm writing these macros in a simple form that I think is easy to read, versus some complex macro recursion which I think is _possible_ but would be harder to reason. And, we don't really need arbitrary arg counts -- this is probably going to stay fairly limited long-term, although I could easily see something like a half dozen in some cases so maybe I'll be wrong and it'll go higher. But, I feel like these macros still make it easier to focus on the _intent_ of cases, rather than visually comparing each line for differences.
Co-authored-by: Chandler Carruth <chandlerc@gmail.com>
We could similarly add others -- this is intended to make it easy to add more that parse essentially the same.
The functionality expected is that types will use GetDeclarationContext in order to error on certain functionality in the declaration scope loop. e.g., with how constraints and interfaces currently don't allow definitions.
I've only moved out `package` because it's only valid on the top line. It might still be good to parse it later, but with slightly different logic because it would always be an error, and the declaration context isn't quite the right framing for that.
Also unifies some errors with `fn`.
This is something we'd discussed. I added a TODO that we may want to eventually make this a map, but I remain uncertain and think it's not something that's going to really cost us if we end up switching back. In the meantime, I think this approach does offer simplicity. As discussed too, the performance overhead of a map may ultimately not be worthwhile here versus the relative memory costs.
Depends on #2646
Right now, deduced parameter handling is very narrow to `self` support. This folds it into pattern handling, which should eventually be a superset of deduced parameter support, so this will avoid more duplication of logic.
Note, this subtly adds handling of multiple deduced parameters, but not generic parameters (`:!`) so it's still not quite right.
I've been thinking about how to restructure block recursion in order to handle params better, but this PR doesn't encounter that issue, and I may be able to adjust SemanticsIR to avoid the issue (but I'm still not sure how to form complex types in LLVM IR).
Co-authored-by: Chandler Carruth <chandlerc@gmail.com>
The comments in parse_node_kind.def capture the change being made here.
Before:
```
// _external_: DeclaredName
// InterfaceBodyStart
// _external_: statements
// InterfaceBodyEnd
// InterfaceDefinition
```
After:
```
// InterfaceIntroducer
// DeclaredName
// InterfaceDefinitionStart
// _external_: declarations
// InterfaceDefinition
```
Really I just want to treat introduced things consistently. `var` defines my philosophy here: it doesn't always have a `DeclaredName`, so the `VarIntroducer` _must_ be the bounding node. By being consistent with that, I believe that overall the structure becomes easier to understand (that is, there are fewer inconsistencies to understand).
This also adds InterfaceDeclaration, since I think it can be predicted we'll have that, and it's helpful for making recover consistent with HandleDeclarationError.
Similarly, I'm also trying to standardize the loop processing a little with HandleDeclarationLoop. In the current approach, InterfaceDefinitionFinish isn't a necessary state, so I'm removing it.
Per [discussion](https://discord.com/channels/655572317891461132/655578254970716160/1078427629427904563), there's a preference for having the support this enables in the parser. For example, that the parser should detect and error on a non-default interface function's definition.
However, we do need to handle nesting of declarations. We could do that by making this a stack. I think though that it'll be more efficient to keep using state_stack_, since it'll be called in places which are a limited number of steps from the actual state. That may already be in cache since we frequently look at state_stack_, so I'm uncertain that maintaining an additional stack would be a net benefit.
This is starting to build out actual lowering logic, for a really simple `fn Main() -> i32 { return 0; }`
Notes for achieving this:
- In semantics, currently function names are bound separate from the signature. When emitting IR, this turns out to be inconvenient because we want to know the name when we process the declaration and the definition. This change addresses that by merging the name into the FunctionDeclaration node, which is also accessible from the definition. It removes the separate BindName. This should be the cause of all the test changes in semantics, because the IR generated changes.
- Add a "Lowering" class which I'm using to hold the llvm builder state. This class now has minimal support for the SemanticsIR generated by the above example.
- In the "Lowering", values from expressions are stored in a DenseMap. I'll keep thinking about whether there's a cleaner way to achieve this, and I'd call it a temporary solution for now. However, this is how the `0` in `return 0` gets properly associated across SemanticsIR instructions, and it'll frequently be an issue in less trivial cases.
The toolchain misinterprets escaped whitespace characters as unescaped when trimming trailing whitespace on a line. This PR adds a variable to track the length of the string after escaping the most recent escape, making sure we don't backup past that character, whatever it is.
I did try the approach mentioned in #2132 of not backing up `end_of_regular_text` number of characters, but this caused problems on lines like <kbd>tab</kbd> <kbd>space</kbd> <kbd>tab</kbd> (those characters literally, not escaped) where it would leave the first tab since that is processed in a different iteration of that loop.
I added a test case for this bug. I kept getting mixed up in the test output which one was the expected value, so I also changed a variable name there for clarity.
Fixes#2132.
Adjusts handling of class constants (`static const`) to use CamelCase. This probably better reflects how we use it in C++ code, treating as appropriate for CamelCase instead of under_score.
Fixes adding_children to be preorder in caller (not sure why this wasn't automated).
No automated changes.
Just opening files in vscode and seeing what clangd flags.
Some edits to ostream.h to stop it from getting flagged (the usage pattern means it's not always obviously used).
This starts handling return types on functions, and comparing types with `return` statements.
Note, errors remain poor because the type literal is currently associated with a builtin, losing the parse_node that specified it. This means we don't have the original source location to associate with, even though it may be helpful to point at the type in source. We could point at the signature overall, but my leaning is that we wouldn't want that long-term, so TODOs for now and may want to change a little about how the parse node is tracked once things are a little further along.
The intent here is that changes to prelude.carbon shouldn't break every test that expects some error from prelude.carbon; that would be too fragile. As a consequence, this effectively ignores the line number in prelude.carbon.
This is a little complex because we don't know which line in the original source file is actually causing the error, just that there is an error. Also, the previous look-behind approach required a fixed-with prefix, whereas we want a little more than that in order to capture the filename for comparison.
This would be hard to do with extra_check_replacement because the path to bazel.runfiles is complex to calculate. As a consequence, this is basically all new code.
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
This adds tracking of call information plus basic type checking. It adds a builtin for the empty tuple, mainly so that I have the basis for a default function return type.
As an aside, it also unifies printing within SemanticsIR, fixing a missing comma after callables.
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Reals were mostly handled, but this PR adds storage of them. It also switches a little towards the FloatingPointType semantic from TokenizedBuffer.
While real literals like `1.0` were handled, the type literals were not. This just adds `f64`, similar to how I also only support `i32`.
The String type literal wasn't used, so I've added support in lexer and parser. Per discussion with @zygoloid String might be renamed based on the newer type literal plan, but it's still String in explorer and the design, so this is just consistent.
The builtin_types.carbon tests the three basic types that are there right now. The test is added to both parser and semantics so that it's clear what the state is in both stages.
There were some notes about default constructors being required in parse_tree.h, but they don't seem to be. Removing the default constructor forces the explicit `::Invalid` where there's no value immediately being assigned, which works fine for existing code. I think this actually reduces the chance of accidents more than the prior default-construct-as-invalid approach.
It feels like every time I add a builtin, I forget to add it to the builtin IR. This approach enforces the addition of builtin nodes, and also takes advantage of the normalized structure to set types appropriately (I think the new approach is simpler on this aspect).
Using the same const/constexpr done in EnumBase, adds Invalid and Builtin* values to replace Make functions that produced the same. This should make it clearer at call sites what the cost actually is, and reduces the syntactic overhead for MakeBuiltinReference in particular.
Really, this is that MakeBuiltinReference has been feeling pretty verbose, so I did that, and then one MakeInvalid is right next to it, and then obviously I should replace the other MakeInvalid for consistency...
The toolchain link just needed a resourcekey.
The open discussion link may just not work because it was pre-go-public, so it points at a doc that we probably copied.
Co-authored-by: josh11b <josh11b@users.noreply.github.com>
Jon Ross-Perkins
josh11bandGeoff Romer
Saeed Ahmed
Calvin