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

Avoid missing docs warning on pub extern crate #647

Merged
merged 1 commit into from
Jul 11, 2023

Conversation

jannic
Copy link
Member

@jannic jannic commented Jul 11, 2023

Due to rust-lang/rust#112308, the public re-export of the pac will trigger a missing docs warning in rust 1.71.0.

This is bogus, as the doc string would not be shown in the rendered docs anyway. However, the warning would be annoying, so add a short doc string to the item.

Due to rust-lang/rust#112308, the public
re-export of the pac will trigger a missing docs warning in rust
1.71.0.

This is bogus, as the doc string would not be shown in the rendered
docs anyway. However, the warning would be annoying, so add a short
doc string to the item.
@jannic jannic merged commit df8e614 into rp-rs:main Jul 11, 2023
@ithinuel
Copy link
Member

Does the issue arise if we use edition 2021's pub use rp2040_pac as pac; ?

@jannic
Copy link
Member Author

jannic commented Jul 11, 2023

Does the issue arise if we use edition 2021's pub use rp2040_pac as pac; ?

It doesn't seem so, but it leads to lots of other errors:

    Checking rp2040-hal v0.9.0-alpha.1 (/home/jan/rp2040/rp-rs/rp-hal/rp2040-hal)
error[E0433]: failed to resolve: use of undeclared crate or module `pac`
 --> rp2040-hal/src/i2c/controller.rs:6:5
  |
6 | use pac::{i2c0::RegisterBlock as Block, RESETS};
  |     ^^^ use of undeclared crate or module `pac`

error[E0433]: failed to resolve: use of undeclared crate or module `pac`
 --> rp2040-hal/src/pwm/reg.rs:2:5
  |
2 | use pac::pwm::CH;
  |     ^^^ use of undeclared crate or module `pac`

[...]
error: could not compile `rp2040-hal` (lib) due to 255 previous errors; 4 warnings emitted

Probably all fixable by adjusting those use statements, but is it worth the effort?

@ithinuel
Copy link
Member

Since our MSRV has edition 2021 support anyway, do we have any benefits from sticking with 2018?

@jannic
Copy link
Member Author

jannic commented Jul 12, 2023

I don't think so. It's probably just that nobody did the conversion, because it doesn't give us significant benefits either.

Maybe https://doc.rust-lang.org/stable/edition-guide/rust-2021/panic-macro-consistency.html could add some additional overhead because panic! now always involves string formatting. But my gut feeling is that, as we already have panic statements which use string formatting, the overhead will be very small, if any.

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.

3 participants