Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Fix Lifetime Warnings #11603

Closed
wants to merge 7 commits into from
Closed

Fix Lifetime Warnings #11603

wants to merge 7 commits into from

Conversation

shawntabrizi
Copy link
Member

@shawntabrizi shawntabrizi commented Jun 6, 2022

Fix these warnings:

warning: unnecessary lifetime parameter `'a`
  --> primitives/runtime/src/curve.rs:28:28
   |
28 | pub struct PiecewiseLinear<'a> {
   |                            ^^
   |
   = help: you can use the `'static` lifetime directly, in place of `'a`

warning: `sp-runtime` (lib) generated 1 warning
   Compiling frame-support v4.0.0-dev (/Users/shawntabrizi/Documents/GitHub/substrate2/frame/support)
warning: unnecessary lifetime parameter `'a`
   --> frame/support/src/storage/bounded_vec.rs:112:25
    |
112 | pub struct BoundedSlice<'a, T, S>(&'a [T], PhantomData<S>);
    |                         ^^
    |
    = help: you can use the `'static` lifetime directly, in place of `'a`

   Compiling frame-system v4.0.0-dev (/Users/shawntabrizi/Documents/GitHub/substrate2/frame/system)
warning: `frame-support` (lib) generated 1 warning

The second warning about BoundedVec seems we should ignore.

Needs: rust-lang/rust#96956

@github-actions github-actions bot added the A0-please_review Pull request needs code review. label Jun 6, 2022
@shawntabrizi shawntabrizi added B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit labels Jun 6, 2022
@@ -109,16 +109,13 @@ where
/// Similar to a `BoundedVec`, but not owned and cannot be decoded.
#[derive(Encode, scale_info::TypeInfo)]
#[scale_info(skip_type_params(S))]
pub struct BoundedSlice<'a, T, S>(&'a [T], PhantomData<S>);
pub struct BoundedSlice<T: 'static, S>(&'static [T], PhantomData<S>);
Copy link
Contributor

Choose a reason for hiding this comment

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

I really don't see why this can be 'static, we should be parameterizing over the lifetime of the reference to the slice, but I don't know why rustc or clippy is telling us to just use 'static.

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, thinking a little more, I think you are right. So then just add a clippy silencer here?

Copy link
Member Author

Choose a reason for hiding this comment

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

rust-lang/rust#96956

Seems it is a bug atm (either to silence it or the warning itself)

Copy link
Member

Choose a reason for hiding this comment

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

No idea why it started annoying us with this. Does not make sense IMO.
Otherwise it would not work outside of static context anymore

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah perhaps we silence this lint globally.

Copy link
Member

Choose a reason for hiding this comment

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

You can not silence it. However, I also don't think this here is the correct solution. This is a bug in rustc and I also already looked into it. It is coming from the scale_info::TypeInfo derive macro. We need to wait for rustc to fix this.

@shawntabrizi shawntabrizi deleted the shawntabrizi-lifetime branch June 7, 2022 14:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants