Skip to content
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

Add #[allow(clippy::*)] to the top-level items in codegen.rs #1207

Merged
merged 8 commits into from
Feb 10, 2019
Merged

Add #[allow(clippy::*)] to the top-level items in codegen.rs #1207

Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
e19306c
add #[allow(clippy::*)] in the top-level items
T5uku5hi Jan 25, 2019
2230548
fixed conflict
T5uku5hi Feb 3, 2019
d3a5234
comment out the all #[allow(clippy::*)] lines
T5uku5hi Feb 3, 2019
51f7756
add #[allow(clippy::*)] to 901 line
T5uku5hi Feb 3, 2019
fb8e158
remove unnecessary changes
T5uku5hi Feb 3, 2019
1b76410
Merge remote-tracking branch 'upstream/master' into add-allow-clippy
T5uku5hi Feb 8, 2019
9167893
add #[allow(clippy::drop_ref)]
T5uku5hi Feb 8, 2019
5c0da88
add #[allow(clippy::all)] in top-level items.
T5uku5hi Feb 9, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions crates/backend/src/codegen.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -899,6 +899,7 @@ impl TryToTokens for ast::ImportFunction {
abi_arguments.push(quote! { #exn_data_ptr: *mut u32 });
convert_ret = quote! { Ok(#convert_ret) };
exceptional_ret = quote! {
#[allow(clippy::*)]
Copy link
Member

Choose a reason for hiding this comment

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

Ah I see -- we should only add #[allow(clippy::*)] to things that are top-level items in the expanded code, not anything that is at the top of a quote! macro invocation, since it might take multiple quotes to build up a top-level item that we are emitting.

A good rule of thumb is that we should add it on each fn we define.

Does that make sense?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes!! I understand you said.
I tried to add #[allow(clippy::*)] to things that are top-level items at first,
but this code occurred another error like this.

error: expected one of `(`, `)`, `,`, or `=`, found `::`
  --> crates/backend/src/codegen.rs:15:15
   |
15 | #[allow(clippy::*)]
   |               ^^ expected one of `(`, `)`, `,`, or `=` here

error: unexpected token: `)`
  --> crates/backend/src/codegen.rs:15:18
   |
15 | #[allow(clippy::*)]
   |                  ^ unexpected token after this

error: aborting due to 2 previous errors

　
I found that I should use #[allow(clippy::drop_ref)] instead of #[allow(clippy::*)] when I read codes written by others 🧐💡
Finally, I fixed it.

Copy link
Contributor

Choose a reason for hiding this comment

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

@T5uku5hi, did you try #![allow(clippy::all)]?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@limira
Oh! I didn't try all instead of *.
I tried it just now and I well did it!

I'll add #[allow(clippy::all)] in all the top-level items.
Thank you for your kindness 😊

Copy link
Member

Choose a reason for hiding this comment

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

This is my bad, sorry!

For some reason I thought the syntax was allow(clippy::*) and it is actually allow(clippy::all) as @limira points out. Sorry about that!

if #exn_data[0] == 1 {
return Err(
<
Expand Down