Skip to content

Conversation

@cpu
Copy link
Member

@cpu cpu commented May 30, 2024

It might be best to review this commit-by-commit. The total overall changeset should have no functional effect - it's exclusively stylistic changes.

  • Removes explicit typing throughout, whenever the compiler can successfully infer the types on its own
  • Fixes redundant prefixing throughout, whenever an item was already in-scope without the prefix
  • Fixes a couple clippy findings that fell out of ^
  • Applies a project-wide formatting pass, including within the ffi_panic_boundary! macro bodies (using the approach ctz used in cargo fmt inside of ffi_panic_boundary! invocations #383 and that we're using in rustls-openssl-compat).
  • Updates make format and make format-check to enforce rust formatting within the ffi_panic_boundary! macros

Resolves #31

@cpu cpu self-assigned this May 30, 2024
@ctz
Copy link
Member

ctz commented May 31, 2024

Good stuff!

cpu added 21 commits June 4, 2024 14:19
Using:

```
sed -i -e 's/ffi_panic_boundary! {/if true {/g' src/*.rs
cargo fmt
sed -i -e 's/if true {/ffi_panic_boundary! {/g' src/*.rs
```
After removing the type hints, this is now a redundant binding:

```
error: redundant redefinition of a binding `cs`
   --> src/connection.rs:412:17
    |
412 |                 let cs = cs;
    |                 ^^^^^^^^^^^^
    |
help: `cs` is initially defined here
   --> src/connection.rs:408:17
    |
408 |             for cs in ALL_CIPHER_SUITES {
    |                 ^^
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_locals
    = note: `#[deny(clippy::redundant_locals)]` on by default
```
After refactoring out explicit type hints these are now `useless_vec`
findings of the form:

```
error: useless use of `vec!`
   --> src/client.rs:599:20
    |
599 |         let alpn = vec![h1.into(), h2.into()];
    |                    ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: you can use an array directly: `[h1.into(), h2.into()]`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#useless_vec
    = note: `-D clippy::useless-vec` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::useless_vec)]`
```
This papers around a `cargo fmt` limitation that leaves code within the
`ffi_panic_boundary!` macros unformatted.

By also adding this to `format-check`, we'll get CI enforcement to
prevent backsliding.
@cpu
Copy link
Member Author

cpu commented Jun 4, 2024

@jsha Would you like me to hold merging this until you've had a chance to review?

@jsha
Copy link
Collaborator

jsha commented Jun 5, 2024

Nope, go ahead and merge, thanks!

@cpu cpu merged commit 5d445f9 into rustls:main Jun 5, 2024
@cpu cpu deleted the cpu-tidying branch June 5, 2024 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants