Stream: git-cranelift

Topic: cranelift / PR #1340 Fix remaining clippy warnings

GitHub (Jan 13 2020 at 17:34):

abrown opened PR #1340 from clippy to master:

• [x] This has been discussed in issue #1170
• [x] A short description of what this does, why it is needed: this fixes the remaining clippy warnings that I could see from running cargo clean && cargo clippy (is there a better way to do this?)
• [x] This PR contains no test cases; no functionality should change
• [x] A reviewer from the core maintainer team has been assigned for this PR

GitHub (Jan 13 2020 at 17:34):

abrown requested bnjbvr for a review on PR #1340.

GitHub (Jan 13 2020 at 17:44):

abrown updated PR #1340 from clippy to master:

• [x] This has been discussed in issue #1170
• [x] A short description of what this does, why it is needed: this fixes the remaining clippy warnings that I could see from running cargo clean && cargo clippy (is there a better way to do this?)
• [x] This PR contains no test cases; no functionality should change
• [x] A reviewer from the core maintainer team has been assigned for this PR

GitHub (Jan 14 2020 at 09:07):

bnjbvr submitted PR Review.

GitHub (Jan 14 2020 at 19:56):

abrown updated PR #1340 from clippy to master:

• [x] This has been discussed in issue #1170
• [x] A short description of what this does, why it is needed: this fixes the remaining clippy warnings that I could see from running cargo clean && cargo clippy (is there a better way to do this?)
• [x] This PR contains no test cases; no functionality should change
• [x] A reviewer from the core maintainer team has been assigned for this PR

GitHub (Jan 16 2020 at 17:05):

abrown updated PR #1340 from clippy to master:

• [x] This has been discussed in issue #1170
• [x] A short description of what this does, why it is needed: this fixes the remaining clippy warnings that I could see from running cargo clean && cargo clippy (is there a better way to do this?)
• [x] This PR contains no test cases; no functionality should change
• [x] A reviewer from the core maintainer team has been assigned for this PR

GitHub (Jan 16 2020 at 17:39):

abrown updated PR #1340 from clippy to master:

• [x] This has been discussed in issue #1170
• [x] A short description of what this does, why it is needed: this fixes the remaining clippy warnings that I could see from running cargo clean && cargo clippy (is there a better way to do this?)
• [x] This PR contains no test cases; no functionality should change
• [x] A reviewer from the core maintainer team has been assigned for this PR

GitHub (Jan 16 2020 at 17:44):

abrown submitted PR Review.

GitHub (Jan 16 2020 at 17:44):

abrown created PR Review Comment:

@iximeow, @yurydelendik: here it is! (With an extra variable assignment)

GitHub (Jan 16 2020 at 17:46):

iximeow submitted PR Review.

GitHub (Jan 16 2020 at 17:46):

iximeow created PR Review Comment:

@yurydelendik Do you recall what this u32 specifically is? It's a count of FDEs or something, right? It'd be nice if we could use that name instead of fde_first_u32.

GitHub (Jan 16 2020 at 18:14):

yurydelendik submitted PR Review.

GitHub (Jan 16 2020 at 18:14):

yurydelendik created PR Review Comment:

As discussed in previous PR, gimli does not provide right API to get all data needed. It is a field (u32) that show length of the CIE, so the len and 4 u32 bytes is offset of the FDE structure that follows CIE.

GitHub (Jan 16 2020 at 18:27):

abrown submitted PR Review.

GitHub (Jan 16 2020 at 18:27):

abrown created PR Review Comment:

Should I rename fde_first_u32 as cie_len?

GitHub (Jan 16 2020 at 21:11):

philipc submitted PR Review.

GitHub (Jan 16 2020 at 21:11):

philipc created PR Review Comment:

Nit: the expect message is wrong. That try_into isn't checking there were 4 bytes at the beginning of bytes. It's checking that the slice passed to try_into is 4 bytes, which will always be true because we indexed with a range of [..4]. If there were less than 4 bytes, then there would be a panic on the slice indexing, not on the try_into().expect(). IMO using unwrap is fine in this case.

GitHub (Jan 16 2020 at 22:04):

abrown updated PR #1340 from clippy to master:

• [x] This has been discussed in issue #1170
• [x] A short description of what this does, why it is needed: this fixes the remaining clippy warnings that I could see from running cargo clean && cargo clippy (is there a better way to do this?)
• [x] This PR contains no test cases; no functionality should change
• [x] A reviewer from the core maintainer team has been assigned for this PR

GitHub (Jan 17 2020 at 19:59):

yurydelendik submitted PR Review.

GitHub (Jan 17 2020 at 20:00):

yurydelendik submitted PR Review.

GitHub (Jan 17 2020 at 20:03):

yurydelendik merged PR #1340.

Last updated: Nov 22 2024 at 17:03 UTC