Add local and global variable support #13199
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Instead of storing all state inside the Interpreter itself, store it
inside a separate class, and then compose those classes. This has some
advantages:
- We can pass this state to type convertors and validators, allowing
more validation code to be done in shared validators instead of in
the bodies of functions
- We can share this state with modules, removing the need for
repeating ourselves in the ModuleState object
- It simplifies the Interpreter class, which is very complicated
- It simplifies creating subprojects, as we can re-use a single
interpreter, simply replacing it's local state while maintaining
the global state
Unfortunately we need to keep the `.subproject` attribute to continue to implement an interface. I have implemented this as a method, which simplifies this, as it simply looks up the state value. I have left the uses of .subproject as they are numerous and we can't remove the attribute anyway.
Also initialize it to a non-null value, as we don't expect it to be null, and it's easy to fix. This also requires a property for interfaces reasons. It is, however, used much less and as such I have replaced all uses that are not part of that interface.
This was only used in a couple of places, and requires extra tracking to ensure it is correct, while we already have `state.local.current_node.lineno`, which is always accurate and up to date.
Summary is shared across all subprojects. It's also mutable state that might need to be turned back if a subproject fails (unlike some other attributes like the holder maps and modules), so it needs to be stored outside the Interpreter itself.
These are renamed to `args_frozen` since `global_args_frozen` will move to the `GlobalState` so they wont conflict, and to reduce some amount of typing.
Which is built, but never used
Since both the AST Interpreter and the main Interpreter both use this in exactly the same way I put it in the base state.
For the main Interpreter, this is actually a useless second tracking of
information in the Build class, but for ASTInterpreter, which doesn't
have a Build, it needs to be an attribute. To ensure API compatibility,
I've used a `@property` in Interpreter.state.global. I've not used this
as properties (methods) have more overhead than an attribute lookup
This also uncovered an interesting bug, when a function is called inside
of `project()`, like `project(version : files('VERSION'))`, we would use
the default subproject_dir for validation, which provides a mechanism
for sandbox escape. I'm not entirely happy with my patch changes to make
that more robust, but they do make it more robust (and work).
This is, unfortunately, still tracked in two place. One in `Build.dep_manifests` and one in `Interpreter.state.local`. I don't see a good way to get rid of this, unfortunately
And rename to `default_subproject_options`, which help clarify the difference between that and `project_default_options`
Since it's only used to check membership, a set makes more sense. Also add some documentation to the attribute
Since we'll soon be using a single Interpreter with changing State objects instead of whole new Interpreters, we can't pass a static reference to build here. Instead, access it through the Interpreter reference, so that each subproject will use the right state
Otherwise as the build changes the backend will be incorrect. Mypy gets really confused about `@property`s, and module imports, so I've had to change `import build` to `import build as _build`.
The former performs a linear search, which is slower than the set based search.
A second object is used, do the need to attach a few extra attributes. This is partially a nice cleanup, and partially to get to the point that we can get rid of multiple Interpreter instances.
It's not actually useful, we can just add the root meson.build file to the depfiles when we translate the meson.build into ast. If there's a provided ast then we don't go down that path anyway.
Since we're about to stop replacing the interpreter and use a single interpreter for the entire run.
Instead, replace the state with new state. This means we don't have to
re-calculate any of the methods or tell the sub-interpreter about the
state of the super-interpreter, it's already there. Since all of the
state that is not an implementation detail of the Interpreter itself is
no longer bound to the interpreter this is all very trivial.
This fixes a number of potential bugs created by side-effects from a
failing, optional, subproject (Assume A => B => C, where C configures
successfully but B fails after that)
1. If B called `meson_override_dependency()` or
`meson.override_find_program()` those overrides would remain, even
though the targets would not exist.
2. Targets from C would be built, even though B will not be, and A does
not use them.
3. If C called `meson.override_dependency()` or
`meson.override_find_program()` these overrides would remain, and
could affect A
4. If B is optional, but a subproject D is called if B fails, and D
calls C with different `default_options` than B, D will get a copy
of C using those different options
5. Summary options for C will be printed, even though it is unused
This is currently unused
I'm not in love with this, because I feel like it really should be part of the AssignmentNode.
This reverts commit cb7e8d212e8177692b28374f0217b5cbf0a4299e.
For what scope a variable is assigned to
TODO: - the operator is incorrect if scoping is used - += shouldn't inherit this operation - the formatter/rewritter needs to be taught about this
It's unused, and with the addition of local variables it becomes a footgun, since there are both local and global variables, and this only returns global variables.
Which is less code, and will properly handle local vs global variables
This allows the writing of code like: ```meson var: global = 'bar' var: local = 'foo' message(var) ``` which will print `foo`, as local variables always shadow global ones. There are a few things here I'm not sure how to handle. 1. Should we allow a variable to be declared global and local in the same file? 2. Should scoping only be allowed on the initial declaration? Ie, should we have a DeclarationNode that is different than an AssignmentNode?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is still very WIP, and it's got a lot of other work it's based on.
Closes: #2607