Skip to content

Update lint declaration example #713

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 1 commit into from
May 29, 2020
Merged

Conversation

JohnTitor
Copy link
Member

Fixes #712

@tshepang
Copy link
Member

huh, such much speeds 😮

impl_lint_pass!(
WhileTrue => [WHILE_TRUE],
);
declare_lint_pass!(WhileTrue => [WHILE_TRUE]);
Copy link
Member Author

Choose a reason for hiding this comment

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

We now use declare_lint_pass macro instead of defining manually. Should we do manually for clarify?

@JohnTitor
Copy link
Member Author

Hm, the link error is unrelated to this, I'll submit another PR not to block the process.

@JohnTitor
Copy link
Member Author

So, r? @nikomatsakis or @Mark-Simulacrum as per git blame

@Mark-Simulacrum
Copy link
Member

Seems fine to me though without the context as to what upstream changes caused this I can't be sure the new code is good. Someone from clippy can likely give better guidance than I.

But r=me on the changes presuming they compile.

@JohnTitor
Copy link
Member Author

I think the issue here is that while_true lint example is entirely outdated and now on EarlyLintPass.

But r=me on the changes presuming they compile.

@Mark-Simulacrum It'd be great if you could approve this so that "Review required" is satisfied :)

@JohnTitor
Copy link
Member Author

cc @rust-lang/wg-rustc-dev-guide it'd be great if someone could also review here :)

Copy link
Member

@mark-i-m mark-i-m left a comment

Choose a reason for hiding this comment

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

It's been a long time since I looked at this code, and it keeps changing a bit over time.

I'm personally fine with merging this and fixing later, if you think that would be the best path forward.

@Mark-Simulacrum
Copy link
Member

Oh sorry, missed the notification. Feel free to ping me on Zulip in the future, that's much more likely to get through.

@JohnTitor
Copy link
Member Author

Thanks both!

Feel free to ping me on Zulip in the future, that's much more likely to get through.

Gotcha!

@JohnTitor JohnTitor merged commit b381598 into rust-lang:master May 29, 2020
@JohnTitor JohnTitor deleted the lint-declare branch May 29, 2020 23:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

emitting errors & diagnostics: example lint appears to be outdated
4 participants