Skip to content

Make DefaultViewInstance safe via compiler-checked covariance#71

Merged
iainmcgin merged 3 commits intomainfrom
refactor/safe-default-view-instance
Apr 27, 2026
Merged

Make DefaultViewInstance safe via compiler-checked covariance#71
iainmcgin merged 3 commits intomainfrom
refactor/safe-default-view-instance

Conversation

@iainmcgin
Copy link
Copy Markdown
Collaborator

Stacked on #70.

Summary

Removes the last unsafe from the default-instance machinery by collapsing the two view-default traits into one safe trait whose covariance contract is checked by the compiler.

Previously, DefaultViewInstance was implemented only for FooView<'static>, and a separate unsafe trait HasDefaultViewInstance linked FooView<'a> to its 'static instantiation so that Deref for MessageFieldView<V> could serve the static default at any lifetime via a raw pointer cast:

None => unsafe { &*(V::default_view_ptr() as *const V) },

That cast is sound only if FooView is covariant in 'a — a promise the compiler can't verify through a *const u8, hence the unsafe trait.

This PR moves the lifetime coercion into the per-type impl body, where the compiler checks covariance via ordinary subtyping:

pub trait DefaultViewInstance {
    fn default_view_instance<'a>() -> &'a Self
    where
        Self: 'a;
}

impl<'v> DefaultViewInstance for FooView<'v> {
    fn default_view_instance<'a>() -> &'a Self
    where
        Self: 'a,
    {
        static VALUE: OnceBox<FooView<'static>> = OnceBox::new();
        VALUE.get_or_init(|| Box::new(<FooView<'static>>::default()))
    }
}

The return expression has type &'static FooView<'static>; the function must return &'a FooView<'v>. Rust accepts that coercion iff FooView is covariant in 'v — and rejects it with a lifetime error otherwise. So a non-covariant view type now fails to compile at the impl site instead of being a silently-unsound unsafe impl.

Deref for MessageFieldView<V> becomes plain safe code:

fn deref(&self) -> &V {
    self.inner.as_deref().unwrap_or_else(V::default_view_instance)
}

Why this is sound

A safe implementation cannot cause UB: there is no associated type to point at the wrong layout, and any &'a Self produced in safe Rust is valid by construction. The only contract not expressible in the old type signature — covariance — is now verified per-impl by the borrow checker.

Covariance verified end-to-end

All generated view field types are covariant in 'a:

  • &'a str, &'a [u8] — covariant
  • MessageFieldView<V> is Option<Box<V>> — covariant in V
  • RepeatedView<'a, T> is Vec<T> + PhantomData<&'a ()> — covariant
  • MapView<'a, K, V> is Vec<(K, V)> + PhantomData<&'a ()> — covariant (notably not HashMap, which would be the one risk)
  • View oneof enums hold the above — covariant

The workspace (including buffa-test which exercises every field shape, and StructView which has a MapView field) compiles cleanly under the new scheme, confirming this in practice.

Changes

  • buffa/src/view.rs: new DefaultViewInstance signature; HasDefaultViewInstance and default_view_ptr deleted; Deref/PartialEq/Eq for MessageFieldView now bound on DefaultViewInstance with no unsafe block; TinyView test impl updated.
  • buffa/src/lib.rs: HasDefaultViewInstance removed from public exports.
  • buffa-codegen/src/view.rs: emit the single lifetime-generic impl<'v> DefaultViewInstance for #view_ident<'v>; old two-impl emission removed.
  • CHANGELOG.md: extended the DefaultInstance trait is not actually unsafe #68/DefaultViewInstance is not actually unsafe #69 entry.
  • Regenerated buffa-types/src/generated/*.__view.rs.

Migration

Hand-written view types replace their two impls with one:

// Before (#70):
impl DefaultViewInstance for MyView<'static> { ... }
unsafe impl<'a> HasDefaultViewInstance for MyView<'a> {
    type Static = MyView<'static>;
}

// After:
impl<'v> DefaultViewInstance for MyView<'v> {
    fn default_view_instance<'a>() -> &'a Self where Self: 'a {
        static VALUE: OnceBox<MyView<'static>> = OnceBox::new();
        VALUE.get_or_init(|| Box::new(<MyView<'static>>::default()))
    }
}

Breaking change

Yes — targeted for v0.4.0 alongside #70.

Base automatically changed from fix/default-instance-not-unsafe to main April 27, 2026 18:53
Replace the unsafe HasDefaultViewInstance + raw pointer cast in
MessageFieldView::deref with a single safe DefaultViewInstance trait
implemented for FooView<'v> at every lifetime. The impl body returns a
&'static FooView<'static> at the caller-chosen &'a FooView<'v>; Rust
accepts that coercion iff FooView is covariant in 'v, so the compiler
now verifies what was previously an unsafe-trait promise.

Removes the last unsafe from the default-instance machinery.
Mirrors the DefaultInstance doc pattern so hand-implementers see the
canonical OnceBox shape and the compiler-checked covariance note in
rustdoc, not just the changelog. Also align the TinyView test impl with
the codegen-emitted form.
@iainmcgin iainmcgin force-pushed the refactor/safe-default-view-instance branch from 3d4aeec to 4726f62 Compare April 27, 2026 18:54
@iainmcgin iainmcgin marked this pull request as ready for review April 27, 2026 19:03
@iainmcgin iainmcgin requested a review from rpb-ant April 27, 2026 19:04
@rpb-ant
Copy link
Copy Markdown
Contributor

rpb-ant commented Apr 27, 2026

LGTM, I suggest adding a compile_fail doctest on the trait that tries to impl it for an invariant type (e.g. one holding Cell<&'v str>), to make the covariance a CI-enforced fact and guards against someone "simplifying" the signature later.

rpb-ant
rpb-ant previously approved these changes Apr 27, 2026
CI-enforces the covariance check at the heart of the safe
DefaultViewInstance design: a type invariant in its lifetime parameter
cannot return its 'static default at a shorter lifetime, so the
recommended impl pattern fails to compile. Uses
PhantomData<fn(&'v ()) -> &'v ()> for invariance (rather than Cell) so
the only error is the lifetime coercion, not a Sync bound on the static.
@iainmcgin
Copy link
Copy Markdown
Collaborator Author

[claude code]

Good call — added in the latest push. One tweak: Cell<&'v str> would make the test type !Sync, so a static-based impl would fail on the Sync bound rather than the covariance coercion. Used PhantomData<fn(&'v ()) -> &'v ()> instead (invariant, but Send + Sync) so the only compile error is:

error: lifetime may not live long enough
  = note: the struct `Invariant<'v>` is invariant over the parameter `'v`

which is the property we actually want pinned.

@iainmcgin iainmcgin merged commit d65280b into main Apr 27, 2026
7 checks passed
@iainmcgin iainmcgin deleted the refactor/safe-default-view-instance branch April 27, 2026 19:20
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 27, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants