Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(compiler): minimal capturing from inflight methods #1682

Merged
merged 45 commits into from
Mar 6, 2023
Merged

Conversation

eladb
Copy link
Contributor

@eladb eladb commented Feb 28, 2023

Analyze inflight methods to identify exactly which fields are being referenced and which operations are performed on them. Previously, we assumed all inflight methods access all fields across all their inflight operations.

Implementation notes

This is currently implemented during jsification. The function find_inflight_references() iterates over all methods in a resource. For each method it uses a FieldReferenceVisitor to analyze the method and identify all references to this.xxx.

In some cases we will only fail at runtime. For example see inflight_ref_resource_collection.w and inflight_ref_unknown_op.w are both supposed to fail and require that explicit annotations are added. This will be handled as part of #1610.

The JSifier now maintains a diagnostics list so it will be possible to report errors (this turned it into a mutable creature).

We still want to bail out before jsification if there are any errors in previous phases because otherwise, the jsifier cannot infer certain types, etc.

Tests are prefixed with inflight_ref_* (both valid and invalid).

Co-author: @yoav-steinberg

By submitting this pull request, I confirm that my contribution is made under the terms of the Monada Contribution License.

Analyze inflight methods to identify exactly which fields are being referenced and which operations are performed on them.

TODO:
- [ ] Check that referenced fields are capturable
@eladb eladb requested a review from yoav-steinberg February 28, 2023 12:39
@eladb eladb marked this pull request as ready for review March 1, 2023 15:28
@eladb eladb requested a review from a team as a code owner March 1, 2023 15:28
@eladb eladb changed the title feat(compiler): infer inflight field references feat(compiler): identify operations when capturing fields from inflight methods Mar 1, 2023
@eladb eladb changed the title feat(compiler): identify operations when capturing fields from inflight methods feat(compiler): minimal capturing from inflight methods Mar 1, 2023

inflight test() {
let res = this.another.func();
this.another.my_queue.push("message");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add comment to the code? (It seems like this should be valid, no?)

examples/tests/valid/inflight_ref_resource.w Show resolved Hide resolved
libs/wingc/src/jsify.rs Outdated Show resolved Hide resolved
libs/wingc/src/jsify.rs Outdated Show resolved Hide resolved
libs/wingc/src/jsify.rs Outdated Show resolved Hide resolved
libs/wingc/src/jsify.rs Outdated Show resolved Hide resolved
libs/wingc/src/jsify.rs Outdated Show resolved Hide resolved
libs/wingc/src/jsify.rs Outdated Show resolved Hide resolved
libs/wingc/src/jsify.rs Outdated Show resolved Hide resolved
@eladb eladb requested a review from yoav-steinberg March 3, 2023 09:12
mutator added 2 commits March 3, 2023 09:20
Signed-off-by: mutator <[email protected]>
Signed-off-by: mutator <[email protected]>
libs/wingc/src/jsify.rs Outdated Show resolved Hide resolved
libs/wingc/src/jsify.rs Outdated Show resolved Hide resolved
@eladb eladb requested a review from yoav-steinberg March 3, 2023 16:40
@eladb
Copy link
Contributor Author

eladb commented Mar 5, 2023

@yoav-steinberg this is ready for another round

libs/wingc/src/jsify.rs Outdated Show resolved Hide resolved
libs/wingc/src/jsify.rs Outdated Show resolved Hide resolved
libs/wingc/src/jsify.rs Outdated Show resolved Hide resolved
@eladb eladb merged commit b886009 into main Mar 6, 2023
@eladb eladb deleted the eladb/infer-refs branch March 6, 2023 08:28
@monadabot
Copy link
Contributor

Congrats! 🚀 This was released in Wing 0.5.100.

mergify bot pushed a commit that referenced this pull request Mar 15, 2023
Following up on #1682 to identify cases where we cannot infer which operations are performed on a captured resource.

See positive/negative tests for examples.

Rewrite the algorithm which analyzes the expressions captured by inflight methods so that it is able to identify more cases and emit errors when captures cannot be qualified (i.e. a resource is captured but we cannot determine which operations are performed on it without static analysis).

For each method, we identify all expressions that start with `this.xxx` and break them down into parts (using nested references). Then, we traverse the list of parts and split the expression into *preflight* and *inflight*. The preflight part is what we are capturing and the first inflight component qualifies which operations are performed on the captured object.

Reorganized capture tests into `resource_captures` (both under valid and invalid).

This does not address #76 but it explicitly identifies these cases. We will follow up at some point with a way to allow users to explicitly qualify the reference.

*By submitting this pull request, I confirm that my contribution is made under the terms of the [Monada Contribution License](https://docs.winglang.io/terms-and-policies/contribution-license.html)*.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants