-
-
Notifications
You must be signed in to change notification settings - Fork 14.3k
Update provider API docs #150074
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
Update provider API docs #150074
Conversation
|
rustbot has assigned @Mark-Simulacrum. Use |
library/core/src/error.rs
Outdated
| /// } | ||
| /// ``` | ||
| /// | ||
| /// # Implementation Conventions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I put this at the bottom of the existing doc comment but I wasn't sure if I should feature this more prominently, the only issue is I felt like it might be confusing to provide an example of what not to do before an example of how to use it properly, but the pre-existing example is quite long so I also worry that readers will entirely miss the new section.
c2fcd93 to
39760d5
Compare
This comment has been minimized.
This comment has been minimized.
39760d5 to
b30716c
Compare
This comment has been minimized.
This comment has been minimized.
This comment was marked as resolved.
This comment was marked as resolved.
b30716c to
dd8a64a
Compare
This comment has been minimized.
This comment has been minimized.
dd8a64a to
e6f8696
Compare
This comment has been minimized.
This comment has been minimized.
e6f8696 to
cab124b
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
cab124b to
61fd451
Compare
This comment has been minimized.
This comment has been minimized.
61fd451 to
beee074
Compare
beee074 to
b007397
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r=me either way depending on how you feel about it
| /// unintended duplication of information in error reports or require heuristics to deduplicate. | ||
| /// | ||
| /// In other words, the following implementation pattern for `provide` is discouraged and should | ||
| /// not be used for [`Error`] types exposed in public APIs to third parties. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the use case for internal errors?
It seems like we could just say "don't do this", it seems clear that in fully private code people can always do whatever they want?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It feels to me like the way to say "don't do this, but it will not cause 'weird behavior'"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I don't mind erasing the nuance here but I added it to point out where this form of misuse will have a significant impact.
If you have a codebase that is a large workspace of unpublished crates and your codebase is the only one that ever interacts with and reports your errors if you duplicate this information in the chain but never observe it because you only ever call provide on the outermost error in the chain and don't care about identifying exactly which error it came from then its like a tree falling in the forest with nobody around to hear it. If in the future you change your mind and want to associate context with specific errors in the chain of your reports and your implementation becomes a problem you as the author can trivially fix it because you control the definition and all invocations of your errors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happy to defer to you either way on it :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'd prefer to leave it as is. If someone complains about it in the future I would not object to changing or removing the language but I'm hoping it'll be fine.
|
@bors r=Mark-Simulacrum |
…r=Mark-Simulacrum Update provider API docs NVPTX does not support cycles in static initializers (see rust-lang#146787). LLVM produces an error when attempting to generate code for such constructs, like self-referential structs. To avoid LLVM UB, we emit a post-monomorphization error on the Rust side before reaching codegen. This is achieved by analyzing a subgraph of the "mono item graph" that only contains statics. 1. Calculate the strongly connected components (SCCs) of the graph. 2. Check for cycles (more than one node in an SCC or one node that references itself).
…imulacrum Update provider API docs related: rust-lang#145899 (comment) print error from EnzymeWrapper::get_or_init(sysroot) as a note r? @ZuseZ4 e.g. 1. when libEnzyme not found ```shell $ rustc +stage1 -Z autodiff=Enable -C lto=fat src/main.rs error: autodiff backend not found in the sysroot: failed to find a `libEnzyme-21` folder in the sysroot candidates: * /Volumes/WD_BLACK_SN850X_HS_1TB/rust-lang/rust/build/aarch64-apple-darwin/stage1/lib | = note: it will be distributed via rustup in the future ``` 2. when could not load libEnzyme successfully ```shell rustc +stage1 -Z autodiff=Enable -C lto=fat src/main.rs error: failed to load our autodiff backend: DlOpen { source: "dlopen(/Volumes/WD_BLACK_SN850X_HS_1TB/rust-lang/rust/build/aarch64-apple-darwin/stage1/lib/rustlib/aarch64-apple-darwin/lib/libEnzyme-21.dylib, 0x0005): tried: \'/Volumes/WD_BLACK_SN850X_HS_1TB/rust-lang/rust/build/aarch64-apple-darwin/stage1/lib/rustlib/aarch64-apple-darwin/lib/libEnzyme-21.dylib\' (slice is not valid mach-o file), \'/System/Volumes/Preboot/Cryptexes/OS/Volumes/WD_BLACK_SN850X_HS_1TB/rust-lang/rust/build/aarch64-apple-darwin/stage1/lib/rustlib/aarch64-apple-darwin/lib/libEnzyme-21.dylib\' (no such file), \'/Volumes/WD_BLACK_SN850X_HS_1TB/rust-lang/rust/build/aarch64-apple-darwin/stage1/lib/rustlib/aarch64-apple-darwin/lib/libEnzyme-21.dylib\' (slice is not valid mach-o file)" } ```
Update provider API docs tracking issue: rust-lang#44930 Provide our own implementations in order to guarantee the behavior of `va_arg`. We will only be able to stabilize `c_variadic` on targets where we know and guarantee the properties of `va_arg`. r? workingjubilee
Update provider API docs Tracking issue: rust-lang#132980 Fixes rust-lang#133965 Fixes rust-lang#150613 r? @BoxyUwU
…crum Update provider API docs Adds guidance on a specific provider API implementation anti pattern that implementers of the error trait should avoid. resolves rust-lang#99301 (comment)
…crum Update provider API docs Adds guidance on a specific provider API implementation anti pattern that implementers of the error trait should avoid. resolves rust-lang#99301 (comment)
…crum Update provider API docs Adds guidance on a specific provider API implementation anti pattern that implementers of the error trait should avoid. resolves rust-lang#99301 (comment)
Rollup of 11 pull requests Successful merges: - #149976 (Add waker_fn and local_waker_fn to std::task) - #150074 (Update provider API docs) - #150094 (`c_variadic`: provide our own `va_arg` implementation for more targets) - #150164 (rustc: Fix `-Zexport-executable-symbols` on wasm) - #150569 (Ensure that static initializers are acyclic for NVPTX) - #150607 (Add amdgpu_dispatch_ptr intrinsic) - #150694 (./x check miri: enable check_only feature) - #150717 (Thread `--jobs` from `bootstrap` -> `compiletest` -> `run-make-support`) - #150736 (Add AtomicPtr::null) - #150787 (Add myself as co-maintainer for s390x-unknown-linux-musl) - #150789 (Fix copy-n-paste error in `vtable_for` docs) r? @ghost
Rollup of 10 pull requests Successful merges: - #149976 (Add waker_fn and local_waker_fn to std::task) - #150074 (Update provider API docs) - #150094 (`c_variadic`: provide our own `va_arg` implementation for more targets) - #150164 (rustc: Fix `-Zexport-executable-symbols` on wasm) - #150569 (Ensure that static initializers are acyclic for NVPTX) - #150694 (./x check miri: enable check_only feature) - #150717 (Thread `--jobs` from `bootstrap` -> `compiletest` -> `run-make-support`) - #150736 (Add AtomicPtr::null) - #150787 (Add myself as co-maintainer for s390x-unknown-linux-musl) - #150789 (Fix copy-n-paste error in `vtable_for` docs) r? @ghost
Rollup merge of #150074 - provider-doc-update, r=Mark-Simulacrum Update provider API docs Adds guidance on a specific provider API implementation anti pattern that implementers of the error trait should avoid. resolves #99301 (comment)
Adds guidance on a specific provider API implementation anti pattern that implementers of the error trait should avoid.
resolves #99301 (comment)