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

Add loop and other control flow expressions #119

Merged
merged 8 commits into from
Mar 8, 2023

Conversation

xFrednet
Copy link
Member

@xFrednet xFrednet commented Feb 27, 2023

This PR adds loop expressions and control flow expressions to Marker's API. With this cargo dogfood only complains about the path resolution of two generic parameters (which I'm planning to fix soon) and closures. The rest seems to be working 🎉.

For and While loops again have some interesting desugar, I tried to document them in the code. It seems to be working well, at least according to the test output.

Most of the changes should be test output and documentation as usual. Sorry, that it involves so many changes at once. (I'm totally not using Marker to procrastinate) 😅


r? @Niki4tap If you have the time, I'd appreciate your feedback :)

cc: #52

Not much more to say. For everyone reading this, have a wonderful day :)

@xFrednet xFrednet added A-api Area: Stable API D-rustc-driver Driver: Rustc Driver S-waiting-on-review Status: Awaiting review A-driver Area: Driver or something related to the internal working of a driver. labels Feb 27, 2023
@xFrednet xFrednet added this to the v0.0.1 milestone Feb 27, 2023
Copy link
Member

@Niki4tap Niki4tap left a comment

Choose a reason for hiding this comment

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

Sorry for the delay ^^

Commenting mostly on the docs, as I haven't checked the implementation very thoroughly, but the tests seem to be working, so I think it should be fine (famous last words) 😅

marker_api/src/ast/expr/control_flow_expr.rs Outdated Show resolved Hide resolved
marker_api/src/ast/expr/control_flow_expr.rs Outdated Show resolved Hide resolved
marker_api/src/ast/expr/control_flow_expr.rs Outdated Show resolved Hide resolved
marker_api/src/ast/expr/block_expr.rs Outdated Show resolved Hide resolved
marker_api/src/ast/expr/control_flow_expr.rs Outdated Show resolved Hide resolved
marker_api/src/ast/expr/control_flow_expr.rs Outdated Show resolved Hide resolved
marker_api/src/ast/expr/control_flow_expr.rs Outdated Show resolved Hide resolved
marker_api/src/ast/expr/block_expr.rs Outdated Show resolved Hide resolved
@xFrednet
Copy link
Member Author

xFrednet commented Mar 5, 2023

Sorry for the delay ^^

No problem at all, thank you for providing feedback!! I have a busy week ahead. So, it might take some time until I address the comments :)

Commenting mostly on the docs, as I haven't checked the implementation very thoroughly, but the tests seem to be working, so I think it should be fine (famous last words) 😅

That's totally fine. The docs and API are the important parts. I find the rustc backend a bit chaotic. While working on it, I also focus on the tests. That's what they're for. So feel free to continue handling it like that :).

Co-authored-by: Niki4tap <rombiklol2@gmail.com>
@xFrednet xFrednet force-pushed the 052-more-control-more-control branch from 514254f to 8d6f2cb Compare March 7, 2023 21:21
@xFrednet
Copy link
Member Author

xFrednet commented Mar 7, 2023

I hope the last commit covers all suggestions. Thank you for the review :)

@Niki4tap
Copy link
Member

Niki4tap commented Mar 8, 2023

Thanks! Looks great now!

bors r+

@bors
Copy link
Contributor

bors bot commented Mar 8, 2023

@bors bors bot merged commit 374befb into rust-marker:master Mar 8, 2023
@xFrednet xFrednet deleted the 052-more-control-more-control branch March 8, 2023 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-api Area: Stable API A-driver Area: Driver or something related to the internal working of a driver. D-rustc-driver Driver: Rustc Driver S-waiting-on-review Status: Awaiting review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants