-
Notifications
You must be signed in to change notification settings - Fork 16
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
Additional tree-morph selection strategies #466
Conversation
Code and Documentation Coverage ReportDocumentation CoverageClick to view documentation coverage for this PR
Click to view documentation coverage for main
Code Coverage SummaryThis PR: Detailed Report
Main: Detailed Report
Coverage Main & PR Coverage ChangeLines coverage changed by 0.10% and covered lines changed by 16
Functions coverage changed by -0.20% and covered lines changed by 1
Branches... coverage: No comparison data available |
I like everything on the list! |
Continuing my tradition of writing these in random places... For tree-morph, I suggest we take an existing test case and see how far uniplate can go. For example: Take this test: https://github.com/conjure-cp/conjure-oxide/blob/ff378f2a13013be119a3cd085122e714855e8034/crates/tree_morph/tests/add_mul.rs
Very happy to expand on this. I think tree-morph needs a good design document. |
i havent had much chance to test the limits of biplate yet so it would be cool to see how far we can push it! feel free to dump any problems you run into in issues in the uniplate repo :) |
The pull request is marked as stale because it has been inactive for 30 days. If there is no new activity it will be automatically closed in 5 days. |
2575e81
to
5160e94
Compare
add rule selection via user input selection function which panics on multiple options add random and smallest subtree selection strategies doc tweak
5160e94
to
56155b6
Compare
@ozgurakgun Any idea why the hygiene checks are failing here? Seems to be caused by Conjure, not the crate. Also happy new year! |
Fixed on |
I'd like to test these selection strategies, although it may be better to do it as part of a larger test. @niklasdewally mentioned an idea for making an example expression available to the user and letting them try the different selection strategies to see the effects. Perhaps as a conditionally compiled module? If everyone is happy with the current state of the We could also do some benchmark testing, perhaps using I'll now be working on the rule groups idea Oz mentioned, and try to integrate clean/dirty. |
Sounds great @lixitrixi. Make it a command line option and that defaults to what we have now (the naive rewriter)? Regarding clean/dirty, not sure if you were around but we had a discussion about how it needs to be more than a boolean. Essentially it needs to be an int, recording up to what rule level we are "clean". Happy to discuss your plans if you can write them down / through a meeting. |
Regarding testing, you can (and should) add tests to the tree_morph crate itself, maybe that's what you were thinking of? What I said above was about using tree_morph inside oxide. |
Absolutely agree; in fact I made a One example I thought of for |
Maybe make them standalone examples instead of having it all in main? https://doc.rust-lang.org/cargo/reference/cargo-targets.html#examples |
WIP adding useful out-of-the-box selection strategies for
tree-morph
.I encourage people to comment ideas that might be useful! See
helpers.rs
for a current TODO list.