Skip to content

return RewriteResult for rewrite_path & rewrite_struct_*** #6236

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

Merged
merged 2 commits into from
Jul 18, 2024

Conversation

ding-young
Copy link
Contributor

Tracked by #6206

Description

Modify following functions to return RewriteResult

  • rewrite_path and its caller (rewrite_struct_lit / pat)
  • method rewrite_aligned_item of AlignedItem trait

Impl rewrite_result for PreciseCapturingArg, AssocItemConstraint, PolyTraitRef, TraitRef in types.rs
(These are the nodes that were waiting for certain rewrite_*** to return RewriteResult. see #6220)

Side notes

  • rewrite_prefix, another method of the AlignedItem trait remains Option<String> since it is used for the possible range of width for struct prefix (see struct_field_prefix_max_min_width)
  • Additionally, there are some messy unknown_error() calls since I haven't modified write_list, itemize_list, and similar functions. There's ongoing concern about modifying these functions and the iterator implementation for ListItems.

@ytmimi ytmimi added the GSoC Google Summer of Code label Jul 10, 2024
@ytmimi
Copy link
Contributor

ytmimi commented Jul 10, 2024

Thanks for another PR. This one is next on my todo list

Copy link
Contributor

@ytmimi ytmimi left a comment

Choose a reason for hiding this comment

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

I think we're good to go on this one. We just need to address the merge conflict

- refactor rewrite_*** functions that call rewrite_path to return RewriteResult
- modify rewrite_aligned_item method of AlignedItem
@ytmimi
Copy link
Contributor

ytmimi commented Jul 18, 2024

Running the Diff-Check Job now

Edit: The job passed ✅

@ytmimi ytmimi merged commit babc2f9 into rust-lang:master Jul 18, 2024
26 checks passed
@calebcartwright
Copy link
Member

Just noting that this broader change (not specifically this PR, but the overall mission of #6206, has a higher chance of causing merge conflicts on the subtree syncs so if possible it would be good to try to get some more frequent syncs through

@ding-young
Copy link
Contributor Author

@calebcartwright Thanks for pointing that out. I'll try to follow your advice :)

@calebcartwright
Copy link
Member

@calebcartwright Thanks for pointing that out. I'll try to follow your advice :)

nothing for you to worry about @ding-young, it was mostly a note to myself & Yacin to try to get these in more frequently

@ytmimi
Copy link
Contributor

ytmimi commented Jul 18, 2024

@calebcartwright good call out. I don't think there's been many changes in r-l/rust recently, but I can certainly look to get subtree-push PRs out weekly or bi-weekly to keep up with the changes here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GSoC Google Summer of Code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants