Skip to content

Remove in_macro_or_desugar #4417

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Aug 19, 2019
Merged

Remove in_macro_or_desugar #4417

merged 2 commits into from
Aug 19, 2019

Conversation

kraai
Copy link
Contributor

@kraai kraai commented Aug 19, 2019

in_macro_or_desugar is just a wrapper around Span::from_expansion, so remove the former and call the latter instead.

changelog: Remove in_macro_or_desugar.

@kraai
Copy link
Contributor Author

kraai commented Aug 19, 2019

I left one occurrence of in_macro_or_desugar in tests/ui/used_underscore_binding.rs because I wasn't sure whether it should be updated or not. Please let me know if I should rename that one as well.

@flip1995 flip1995 added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Aug 19, 2019
@phansch phansch self-requested a review August 19, 2019 19:01
phansch
phansch reviewed Aug 19, 2019
View reviewed changes
Copy link
Member

@phansch phansch left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Just one question regarding the comment, otherwise LGTM. Thanks for also updating the lint writing docs!

clippy_lints/src/misc.rs Outdated
/// **Why is this bad?** NaN does not compare meaningfully to anything – not
/// even itself – so those comparisons are simply wrong.
/// **Why is this bad?** NaN does not compare meaningfully to anything not
/// even itself so those comparisons are simply wrong.
Copy link
Member

Choose a reason for hiding this comment

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

It looks like the - was replaced with a space here, was this on purpose?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, that wasn't intentional. When I save that file with my editor configured to run rustfmt, it replaces the en dashes with spaces. :( I've pushed a commit to restore them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was a victim of rust-lang/rust-mode#279.

@phansch phansch added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Aug 19, 2019
@phansch
Copy link
Member

phansch commented Aug 19, 2019

Thanks!

@bors r+

@bors
Copy link
Contributor

bors commented Aug 19, 2019

📌 Commit eaf8f08 has been approved by phansch

@bors
Copy link
Contributor

bors commented Aug 19, 2019

⌛ Testing commit eaf8f08 with merge 7810652...

bors added a commit that referenced this pull request Aug 19, 2019
@bors
Auto merge of #4417 - kraai:remove-in_macro_or_desugar, r=phansch
7810652 
Remove in_macro_or_desugar

`in_macro_or_desugar` is just a wrapper around `Span::from_expansion`, so remove the former and call the latter instead.

changelog: Remove `in_macro_or_desugar`.
@bors
Copy link
Contributor

bors commented Aug 19, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: phansch
Pushing 7810652 to master...

@bors bors merged commit eaf8f08 into rust-lang:master Aug 19, 2019
@kraai kraai deleted the remove-in_macro_or_desugar branch August 19, 2019 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@phansch phansch phansch left review comments

Assignees
No one assigned
Labels
S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@kraai @phansch @bors @flip1995