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.

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

Clippy fixes #90

Merged
merged 6 commits into from Oct 25, 2020
Merged

Clippy fixes #90

merged 6 commits into from Oct 25, 2020

Conversation

filnet
Copy link
Contributor

@filnet filnet commented Oct 25, 2020

No description provided.

@filnet filnet mentioned this pull request Oct 25, 2020
@filnet
Copy link
Contributor Author

filnet commented Oct 25, 2020

There are only two clippy warnings left:

warning: very complex type used. Consider factoring parts into `type` definitions
   --> C:\msys64\home\Philippe\rust\shaderc-rs\shaderc-rs\src/lib.rs:645:8
    |
645 |       f: Option<
    |  ________^
646 | |         Box<dyn Fn(&str, IncludeType, &str, usize) -> result::Result<ResolvedInclude, String> + 'a>,
647 | |     >,
    | |_____^
    |
    = note: `#[warn(clippy::type_complexity)]` on by default
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#type_complexity
warning: defining a method called `clone` on this type; consider implementing the `std::clone::Clone` trait or choosing a less ambiguous name
   --> C:\msys64\home\Philippe\rust\shaderc-rs\shaderc-rs\src/lib.rs:698:5
    |
698 | /     pub fn clone(&self) -> Option<CompileOptions> {
699 | |         let p = unsafe { scs::shaderc_compile_options_clone(self.raw) };
700 | |         if p.is_null() {
701 | |             None
...   |
704 | |         }
705 | |     }
    | |_____^
    |
    = note: `#[warn(clippy::should_implement_trait)]` on by default
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#should_implement_trait

I won't address them as they are trickier to fix.

Edit:

I fixed the type_complexity warning.

Fixing the should_implement_trait would require a breaking change because the current clone function signature has to be changed from

pub fn clone(&self) -> Option<CompileOptions>

to

pub fn clone(&self) -> Self

in order to implement Clone.

@filnet filnet force-pushed the clippy_fixes branch 4 times, most recently from 7e7f9f0 to c8f2e7f Compare October 25, 2020 12:32
@filnet filnet mentioned this pull request Oct 25, 2020
@filnet
Copy link
Contributor Author

filnet commented Oct 25, 2020

Just noticed that clone is broken: the value of include_callback_fn is not cloned and left at None.

@antiagainst
Copy link
Collaborator

Awesome! Buildbots should be fixed now. Could you rebase first?

@antiagainst
Copy link
Collaborator

Awesome! Buildbots should be fixed now. Could you rebase first?

Actually nvm, I did that. :)

@antiagainst antiagainst merged commit 41f59da into google:master Oct 25, 2020
@filnet filnet deleted the clippy_fixes branch February 5, 2023 21:07
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.

None yet

2 participants