jobq: drop JobBuilder's Default impl so it is really unconstructible outside
The builder's module doc claimed a builder "cannot be constructed, held or inserted from outside this crate". Two of those three were false: `new()` is `pub(crate)`, but a hand-written `impl Default for JobBuilder` is a trait impl on a `pub` type, so it is public regardless — `JobBuilder::default()` compiled downstream. Nothing was unsound (`insert_with` stayed `pub(crate)`, so an outside-built builder could not reach a graph), but the sentence claimed more than the visibility enforced, which is the bug this crate's docs have hit before. Delete the impl; `new()` constructs directly. The doc now says only what is enforced, and records why there is no `Default` — so the next person reaching for one finds the reason instead of adding it back.
This commit is contained in:
parent
947d45a854
commit
2454a1ea6a
1 changed files with 14 additions and 13 deletions
|
|
@ -8,8 +8,10 @@
|
|||
//! **An insertion API, not a spec factory.** A builder is only ever handed to a
|
||||
//! closure by the single insertion entry point
|
||||
//! ([`crate::scheduler::Scheduler::insert_job`]), which inserts the declared
|
||||
//! nodes and returns the ids the job asked for. It cannot be constructed, held
|
||||
//! or inserted from outside this crate, and there is no intermediate
|
||||
//! nodes and returns the ids the job asked for. It cannot be constructed or
|
||||
//! inserted from outside this crate — `new()` and `insert_with` are both
|
||||
//! `pub(crate)`, and there is deliberately no `Default` impl, since a trait impl
|
||||
//! on a `pub` type is public regardless. There is no intermediate
|
||||
//! node-description type to keep in sync with [`crate::Graph::insert`]'s signature —
|
||||
//! so a job has no representation that can be passed around instead of being
|
||||
//! inserted.
|
||||
|
|
@ -242,16 +244,6 @@ pub struct JobBuilder<N, R> {
|
|||
nodes: RefCell<Vec<Pending<N, R>>>,
|
||||
}
|
||||
|
||||
// Hand-written rather than derived: `#[derive(Default)]` would demand
|
||||
// `N: Default, R: Default`, which has nothing to do with an empty builder.
|
||||
impl<N, R> Default for JobBuilder<N, R> {
|
||||
fn default() -> Self {
|
||||
Self {
|
||||
nodes: RefCell::new(Vec::new()),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl<N, R> JobBuilder<N, R> {
|
||||
/// A fresh, empty builder.
|
||||
///
|
||||
|
|
@ -261,8 +253,17 @@ impl<N, R> JobBuilder<N, R> {
|
|||
/// nodes and returns the ids. Nothing job-shaped is constructible or
|
||||
/// carryable outside this crate — otherwise it is a spec factory again,
|
||||
/// just with a builder's name on it.
|
||||
///
|
||||
/// Deliberately **not** a `Default` impl. A trait impl on a `pub` type is
|
||||
/// public no matter how private its inherent constructors are, so
|
||||
/// `JobBuilder::default()` would hand every downstream crate the builder
|
||||
/// this fn is `pub(crate)` to withhold. The body is what `#[derive(Default)]`
|
||||
/// could not be anyway — deriving would demand `N: Default, R: Default`,
|
||||
/// which has nothing to do with an empty builder.
|
||||
pub(crate) fn new() -> Self {
|
||||
Self::default()
|
||||
Self {
|
||||
nodes: RefCell::new(Vec::new()),
|
||||
}
|
||||
}
|
||||
|
||||
/// Whether nothing has been declared yet — for a caller deciding whether an
|
||||
|
|
|
|||
Loading…
Reference in a new issue