Build docs for target-specific code independent of platform #4122
Conversation
| #[cfg(all(target_arch = "arm", target_os = "none"))] | ||
| use core::arch::global_asm; |
There was a problem hiding this comment.
I removed these conditional imports while I was going over the code. Conditional imports are a great recipe for producing warnings or errors that can easily be avoided by just referring to items using their fully qualified name, or moving imports into the respective functions themselves.
There was a problem hiding this comment.
I think this is the most reasonable path forward for now, until https://doc.rust-lang.org/cargo/reference/unstable.html#per-package-target is stabilized (if it is stabilized).
While I think this is very undesirable (having to further modify the OS source to tell the tooling to fix a mistake/bad asummption it is making), the benefit of actually documenting our code, while not having to manually implement some sort of manual cargo doc for the workspace where we would manually combine different crate docs, seems worth it. The correct documentations is already being generated, it is just only generated per-board, and not for the workspace as a whole.
Theoretically we would want to somehow avoid this being out of date, but, I don't think that is worth it. If a any(doc) is missed, it's no worse than just not having docs, which we are missing in many places already.
I also think this PR inserts any(doc) in many places which don't do anything, but it's not a big deal and it makes copy-paste easier in the future. I choose to believe someday cargo will catch up and we can delete all of these hacks anyway :).
|
@bradjc Agree with all of the above, although one minor comment:
A nice side effect of this is that we now document the actual target language items, instead of the mock variants. Even if those don't have a doc comment attached, things like the |
| #[cfg(all(target_arch = "arm", target_os = "none"))] | ||
| global_asm!( | ||
| #[cfg(any(doc, all(target_arch = "arm", target_os = "none")))] | ||
| core::arch::global_asm!( |
There was a problem hiding this comment.
I don't think so. My intention here was to make every target selector also set doc properly so when someone copies this expression it would be correct.
Pull Request Overview
This pull request adds
any(doc, ...to target-specific conditional compilation expressions, to ensure that such functions and expressions are always documented.This makes their documentation comment be rendered in Rustdoc, and also allows us to jump to the correct (non-mock) function definitions from within the documentation.
Testing Strategy
This pull request was tested by
cargo docandcargo check.TODO or Help Wanted
N/A
Documentation Updated
/docs, or no updates are required.Formatting
make prepush.