-
Notifications
You must be signed in to change notification settings - Fork 1.8k
clean-up match_same_arms
a bit
#15727
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
Conversation
.map(|(_, arm)| snippet_opt(cx, arm.pat.span)) | ||
.collect::<Option<Vec<_>>>() | ||
{ | ||
let mut suggs = src | ||
let suggs = src | ||
.iter() | ||
.map(|(_, arm)| (adjusted_arm_span(cx, arm.span), String::new())) | ||
.chain([(dest.pat.span, pat_snippets.iter().join(" | "))]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
snippet_opt
is probably going to be removed at some point. Note that get_source_text
does not allocate a temporary string.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, I remember hearing something like that. Would it make sense to mark snippet_opt
as #[deprecated]
then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's waiting on a cleanup PR to be merged.
That isn't really doable without an annoyingly large number of allows.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why allows? Can't one basically always replace snippet_opt
with get_source_text
? Well unless you do need a String
, but that's just an added .to_owned()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if all the uses are going to be replaced then it can just be removed instead of deprecated.
eb29c32
to
f76664c
Compare
changelog: none