Create Key trait

[declanvk]

Currently the tree only accepts Box<[u8]> as keys. I should implement support for different keys types, so long as they can be temporarily transformed to [u8] of some sort.

My initial idea has the general sketch traits:

trait Key {
    fn view_bytes<O, F: FnMut(&[u8]) -> O>(&self, f: F) -> O;
}

/// When the lexicographic ordering of the [`Key`] byte-string matches the ordering of the underlying type.
unsafe trait OrderedKey: Key {}

/// When the type is guaranteed to never produce a [`Key`] byte-string that is the prefix of another [`Key`] or vice-versa.
unsafe trait NoPrefixesKey: Key {}

Example of use for u64:

impl Key for NativeEndian<u64> {
    fn view_bytes<O, F: FnMut(&[u8]) -> O>(&self, mut f: F) -> O {
        let bytes = self.0.to_ne_bytes();

        (f)(bytes.as_ref())
    }
}

unsafe impl NoPrefixesKey for NativeEndian<u64> {}

impl Key for BigEndian<u64> {
    fn view_bytes<O, F: FnMut(&[u8]) -> O>(&self, mut f: F) -> O {
        let bytes = self.0.to_be_bytes();

        (f)(bytes.as_ref())
    }
}

unsafe impl OrderedKey for BigEndian<u64> {}
unsafe impl NoPrefixesKey for BigEndian<u64> {}

impl Key for LittleEndian<u64> {
    fn view_bytes<O, F: FnMut(&[u8]) -> O>(&self, mut f: F) -> O {
        let bytes = self.0.to_le_bytes();

        (f)(bytes.as_ref())
    }
}

unsafe impl NoPrefixesKey for LittleEndian<u64> {}

#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
#[repr(transparent)]
struct BigEndian<T>(T);

#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
#[repr(transparent)]
struct LittleEndian<T>(T);

#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
#[repr(transparent)]
struct NativeEndian<T>(T);

// hiding some `From` impls...

See full example at https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=0cfdebe7eee884c906ea0607c79e5c07

Ideally I'd like to be able to have the Key trait just directly return &[u8], not go through a closure, but the issue is that some types need to derive a new value for the bytes, not just expose a reference. For integers, the provided methods (to_le_bytes, etc) produce an owned array of bytes which doesn't quite work.

The uses for the traits:

• Key - this trait is the main one which will be implemented for all keys which can be used with the tree, it allows us to access the component bytes for the type.
• OrderedKey - this trait indicates that the lexicographic ordering of the byte-string matches the natural ordering of the type, based on what is used in the PartialOrd or Ord implementation.
  • This one doesn't yet have an API usage yet, so maybe not essential
• NoPrefixesKey - this one is the most important for delivering on a non-failing insert function. Currently there is only the try_insert function, which will fail if the given key is a prefix or makes an existing key a prefix of the new key. This trait would indicate that the set of possible keys will have no prefixes.
  • Examples include fixed length keys (arrays), keys that are terminated with a null value (CStr)

This change would also require changing the overall type variable situation from <V> to <K,V>, and the definition of LeafNode would change from

pub struct LeafNode<V> {
    pub value: V,
    pub key: Box<[u8]>,
}

to

pub struct LeafNode<K, V> {
    pub value: V,
    pub key: K,
}

[declanvk]

The issue with the OrderedKey trait is that I want to allow users of the tree to iterate over the values, even if it doesn't match the natural ordering of the type.

I could add redundant methods like range_ordered that are bounded on that trait, but just do the same thing as the normal range method. Or flip it with range_unordered and the default range requires OrderedKey

[declanvk]

Update to Key trait:

trait Key {
    fn view_bytes<O, F: FnMut(&[u8]) -> O>(&self, f: F) -> O;
    
    type Inner;
    fn as_inner_ref(&self) -> &Self::Inner;

    fn from_inner(src: Self::Inner) -> Self;
    
    fn into_inner(self) -> Self::Inner;
}

The as_inner_ref and from_inner as basically copies from the AsRef and From/Into traits. The purpose of this is to allow wrapping and unwrapping the inner-values from the wrapper types like BigEndian or LittleEndian. For the signed-integers, we'd even have a second wrapper OrderedSignedInt that does the transform described in Section 4.B of https://db.in.tum.de/~leis/papers/ART.pdf, under bullet point b.

Another important point is that we cannot implement the Key trait generically over all values for BigEndian (etc), we need to be able to unwrap/wrap through multiple layers. If there is a key type of BigEndian<OrderedSignedInt<i64>>, then it has to unwrap 2 layers.

Having these additional methods on the trait allows us to input/output values of Key::Inner or &Key::Inner at the API boundaries, instead of requiring the user to wrap and unwrap multiple layers themselves. The user of the API would only ever see the BigEndian<OrderedSignedInt<... bit in the type signature.

[declanvk]

Possibly values for keys:

• All integer values (big endian, little endian, native endian variants)
  • Also need transformations from signed integers to a byte string format that sorts lexicographically the same as the normal ordering
• All float values (needs transformation described in Section 4.B of https://db.in.tum.de/~leis/papers/ART.pdf, under bullet point c.
• Box<[u8]>, Vec<u8>, &[u8] (does this one have weird implications? dropck or something), [u8; N]
• str, CStr (with null and without null variants), OsStr
  • And all owned variants String, CString, OSString
• All variants through smart pointers, Arc<T> where T: Key, for Arc, Box, RefCell, basically anything that implements Deref
• Path? it can dereference to an OSStr
• char? It can be transformed to u32 and then endian-ed from there
• ???????
• Maybe bool for completeness?
• Also Ipv4Addr and Ipv6Addr both can become fixed sized arrays, maybe implement it for IpAddr as well

[declanvk]

More fleshed-out implementation: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=a60b31de91db14fae566069a605be943

The as_inner_ref/from_inner/into_inner leave something to be desired for composition, making [u8] a Key doesn't quite work since it is unsized. I'd want to be able to say Key for {Box,Arc,Rc}<K> where K: Key, so that it will compose for other dynamically sized types. Also extend it for Key for &K where K: Key and Key for &mut K where K: Key

[declanvk]

Going to hold off on endian types:

#[derive(Debug, Clone, Copy)]
#[cfg(feature = "nightly")]
struct BigEndian<T>
where
    [(); std::mem::size_of::<T>()]:,
{
    _repr_type: std::marker::PhantomData<T>,
    bytes: [u8; std::mem::size_of::<T>()],
}

Currently crashes my nightly compiler with:

declan@declan-MS-7C02:~/repos/github/declanvk/blart$ cargo +nightly build --features nightly
   Compiling blart v0.1.0 (/home/declan/repos/github/declanvk/blart)
warning: the feature `generic_const_exprs` is incomplete and may not be safe to use and/or cause compiler crashes
  --> src/lib.rs:11:9
   |
11 |         generic_const_exprs
   |         ^^^^^^^^^^^^^^^^^^^
   |
   = note: see issue #76560 <https://github.com/rust-lang/rust/issues/76560> for more information
   = note: `#[warn(incomplete_features)]` on by default

warning: field `bytes` is never read
  --> src/key.rs:34:5
   |
29 | struct BigEndian<T>
   |        --------- field in this struct
...
34 |     bytes: [u8; std::mem::size_of::<T>()],
   |     ^^^^^
   |
   = note: `BigEndian` has derived impls for the traits `Clone` and `Debug`, but these are intentionally ignored during dead code analysis
   = note: `#[warn(dead_code)]` on by default

error: internal compiler error: no errors encountered even though `delay_span_bug` issued

error: internal compiler error: cannot relate constants (Const { ty: fn() -> usize {std::mem::size_of::<T>}, kind: Value(Branch([])) }, Const { ty: fn() -> usize {std::mem::size_of::<_>}, kind: Value(Branch([])) }) of different types: fn() -> usize {std::mem::size_of::<T>} != fn() -> usize {std::mem::size_of::<_>}
  |
  = note: delayed at    0: <rustc_errors::HandlerInner>::emit_diagnostic
             1: <rustc_errors::Handler>::delay_span_bug::<rustc_span::span_encoding::Span, &alloc::string::String>
             2: rustc_middle::ty::relate::super_relate_consts::<rustc_infer::infer::equate::Equate>
             3: <rustc_infer::infer::InferCtxt>::super_combine_consts::<rustc_infer::infer::equate::Equate>
             4: rustc_middle::ty::relate::super_relate_consts::<rustc_infer::infer::equate::Equate>
             5: <rustc_infer::infer::InferCtxt>::super_combine_consts::<rustc_infer::infer::equate::Equate>
             6: <rustc_infer::infer::InferCtxt>::commit_if_ok::<rustc_infer::infer::InferOk<()>, rustc_middle::ty::error::TypeError, <rustc_infer::infer::at::Trace>::eq<rustc_middle::ty::consts::Const>::{closure#0}>
             7: <rustc_infer::infer::at::At>::eq::<rustc_middle::ty::consts::Const>
             8: <rustc_trait_selection::traits::engine::ObligationCtxt>::eq::<rustc_middle::ty::consts::Const>
             9: <rustc_infer::infer::InferCtxt>::probe::<bool, <rustc_trait_selection::traits::const_evaluatable::satisfied_from_param_env::Visitor as rustc_middle::ty::visit::TypeVisitor>::visit_const::{closure#0}>
            10: <rustc_trait_selection::traits::const_evaluatable::satisfied_from_param_env::Visitor as rustc_middle::ty::visit::TypeVisitor>::visit_const
            11: rustc_trait_selection::traits::const_evaluatable::satisfied_from_param_env
            12: rustc_trait_selection::traits::const_evaluatable::is_const_evaluatable
            13: <rustc_trait_selection::traits::fulfill::FulfillProcessor as rustc_data_structures::obligation_forest::ObligationProcessor>::process_obligation
            14: <rustc_data_structures::obligation_forest::ObligationForest<rustc_trait_selection::traits::fulfill::PendingPredicateObligation>>::process_obligations::<rustc_trait_selection::traits::fulfill::FulfillProcessor>
            15: <rustc_trait_selection::traits::fulfill::FulfillmentContext as rustc_infer::traits::engine::TraitEngine>::select_where_possible
            16: <rustc_hir_typeck::fn_ctxt::FnCtxt>::expected_inputs_for_expected_output
            17: <rustc_hir_typeck::fn_ctxt::FnCtxt>::check_expr_struct_fields
            18: <rustc_hir_typeck::fn_ctxt::FnCtxt>::check_expr_with_expectation_and_args
            19: <rustc_hir_typeck::fn_ctxt::FnCtxt>::check_block_with_expected
            20: <rustc_hir_typeck::fn_ctxt::FnCtxt>::check_expr_with_expectation_and_args
            21: <rustc_hir_typeck::fn_ctxt::FnCtxt>::check_return_expr
            22: rustc_hir_typeck::check::check_fn
            23: rustc_hir_typeck::typeck
            24: <rustc_query_system::dep_graph::graph::DepGraph<rustc_middle::dep_graph::dep_node::DepKind>>::with_task::<rustc_middle::ty::context::TyCtxt, rustc_span::def_id::LocalDefId, &rustc_middle::ty::typeck_results::TypeckResults>
            25: rustc_query_system::query::plumbing::try_execute_query::<rustc_query_impl::queries::typeck, rustc_query_impl::plumbing::QueryCtxt>
            26: rustc_data_structures::sync::par_for_each_in::<&[rustc_span::def_id::LocalDefId], <rustc_middle::hir::map::Map>::par_body_owners<rustc_hir_typeck::typeck_item_bodies::{closure#0}>::{closure#0}>
            27: rustc_hir_typeck::typeck_item_bodies
            28: <rustc_query_system::dep_graph::graph::DepGraph<rustc_middle::dep_graph::dep_node::DepKind>>::with_task::<rustc_middle::ty::context::TyCtxt, (), ()>
            29: rustc_query_system::query::plumbing::try_execute_query::<rustc_query_impl::queries::typeck_item_bodies, rustc_query_impl::plumbing::QueryCtxt>
            30: <rustc_query_impl::Queries as rustc_middle::ty::query::QueryEngine>::typeck_item_bodies
            31: <rustc_session::session::Session>::time::<(), rustc_hir_analysis::check_crate::{closure#7}>
            32: rustc_hir_analysis::check_crate
            33: rustc_interface::passes::analysis
            34: <rustc_query_system::dep_graph::graph::DepGraph<rustc_middle::dep_graph::dep_node::DepKind>>::with_task::<rustc_middle::ty::context::TyCtxt, (), core::result::Result<(), rustc_errors::ErrorGuaranteed>>
            35: rustc_query_system::query::plumbing::try_execute_query::<rustc_query_impl::queries::analysis, rustc_query_impl::plumbing::QueryCtxt>
            36: <rustc_query_impl::Queries as rustc_middle::ty::query::QueryEngine>::analysis
            37: <rustc_interface::passes::QueryContext>::enter::<rustc_driver_impl::run_compiler::{closure#1}::{closure#2}::{closure#3}, core::result::Result<(), rustc_errors::ErrorGuaranteed>>
            38: <rustc_interface::interface::Compiler>::enter::<rustc_driver_impl::run_compiler::{closure#1}::{closure#2}, core::result::Result<core::option::Option<rustc_interface::queries::Linker>, rustc_errors::ErrorGuaranteed>>
            39: rustc_span::with_source_map::<core::result::Result<(), rustc_errors::ErrorGuaranteed>, rustc_interface::interface::run_compiler<core::result::Result<(), rustc_errors::ErrorGuaranteed>, rustc_driver_impl::run_compiler::{closure#1}>::{closure#0}::{closure#0}>
            40: <scoped_tls::ScopedKey<rustc_span::SessionGlobals>>::set::<rustc_interface::interface::run_compiler<core::result::Result<(), rustc_errors::ErrorGuaranteed>, rustc_driver_impl::run_compiler::{closure#1}>::{closure#0}, core::result::Result<(), rustc_errors::ErrorGuaranteed>>
            41: std::sys_common::backtrace::__rust_begin_short_backtrace::<rustc_interface::util::run_in_thread_pool_with_globals<rustc_interface::interface::run_compiler<core::result::Result<(), rustc_errors::ErrorGuaranteed>, rustc_driver_impl::run_compiler::{closure#1}>::{closure#0}, core::result::Result<(), rustc_errors::ErrorGuaranteed>>::{closure#0}::{closure#0}, core::result::Result<(), rustc_errors::ErrorGuaranteed>>
            42: <<std::thread::Builder>::spawn_unchecked_<rustc_interface::util::run_in_thread_pool_with_globals<rustc_interface::interface::run_compiler<core::result::Result<(), rustc_errors::ErrorGuaranteed>, rustc_driver_impl::run_compiler::{closure#1}>::{closure#0}, core::result::Result<(), rustc_errors::ErrorGuaranteed>>::{closure#0}::{closure#0}, core::result::Result<(), rustc_errors::ErrorGuaranteed>>::{closure#1} as core::ops::function::FnOnce<()>>::call_once::{shim:vtable#0}
            43: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
                       at /rustc/8996ea93b6e554148c4286e62b613f12a3ee505c/library/alloc/src/boxed.rs:1988:9
            44: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
                       at /rustc/8996ea93b6e554148c4286e62b613f12a3ee505c/library/alloc/src/boxed.rs:1988:9
            45: std::sys::unix::thread::Thread::new::thread_start
                       at /rustc/8996ea93b6e554148c4286e62b613f12a3ee505c/library/std/src/sys/unix/thread.rs:108:17
            46: start_thread
                       at /build/glibc-SzIz7B/glibc-2.31/nptl/pthread_create.c:477:8
            47: clone
                       at /build/glibc-SzIz7B/glibc-2.31/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:95
          

error: internal compiler error: cannot relate constants (Const { ty: fn() -> usize {std::mem::size_of::<T>}, kind: Value(Branch([])) }, Const { ty: fn() -> usize {std::mem::size_of::<_>}, kind: Value(Branch([])) }) of different types: fn() -> usize {std::mem::size_of::<T>} != fn() -> usize {std::mem::size_of::<_>}
  |
  = note: delayed at    0: <rustc_errors::HandlerInner>::emit_diagnostic
             1: <rustc_errors::Handler>::delay_span_bug::<rustc_span::span_encoding::Span, &alloc::string::String>
             2: rustc_middle::ty::relate::super_relate_consts::<rustc_infer::infer::equate::Equate>
             3: <rustc_infer::infer::InferCtxt>::super_combine_consts::<rustc_infer::infer::equate::Equate>
             4: rustc_middle::ty::relate::super_relate_consts::<rustc_infer::infer::equate::Equate>
             5: <rustc_infer::infer::InferCtxt>::super_combine_consts::<rustc_infer::infer::equate::Equate>
             6: <rustc_infer::infer::InferCtxt>::commit_if_ok::<rustc_infer::infer::InferOk<()>, rustc_middle::ty::error::TypeError, <rustc_infer::infer::at::Trace>::eq<rustc_middle::ty::consts::Const>::{closure#0}>
             7: <rustc_infer::infer::at::At>::eq::<rustc_middle::ty::consts::Const>
             8: <rustc_trait_selection::traits::engine::ObligationCtxt>::eq::<rustc_middle::ty::consts::Const>
             9: rustc_trait_selection::traits::const_evaluatable::satisfied_from_param_env
            10: rustc_trait_selection::traits::const_evaluatable::is_const_evaluatable
            11: <rustc_trait_selection::traits::fulfill::FulfillProcessor as rustc_data_structures::obligation_forest::ObligationProcessor>::process_obligation
            12: <rustc_data_structures::obligation_forest::ObligationForest<rustc_trait_selection::traits::fulfill::PendingPredicateObligation>>::process_obligations::<rustc_trait_selection::traits::fulfill::FulfillProcessor>
            13: <rustc_trait_selection::traits::fulfill::FulfillmentContext as rustc_infer::traits::engine::TraitEngine>::select_where_possible
            14: <rustc_hir_typeck::fn_ctxt::FnCtxt>::expected_inputs_for_expected_output
            15: <rustc_hir_typeck::fn_ctxt::FnCtxt>::check_expr_struct_fields
            16: <rustc_hir_typeck::fn_ctxt::FnCtxt>::check_expr_with_expectation_and_args
            17: <rustc_hir_typeck::fn_ctxt::FnCtxt>::check_block_with_expected
            18: <rustc_hir_typeck::fn_ctxt::FnCtxt>::check_expr_with_expectation_and_args
            19: <rustc_hir_typeck::fn_ctxt::FnCtxt>::check_return_expr
            20: rustc_hir_typeck::check::check_fn
            21: rustc_hir_typeck::typeck
            22: <rustc_query_system::dep_graph::graph::DepGraph<rustc_middle::dep_graph::dep_node::DepKind>>::with_task::<rustc_middle::ty::context::TyCtxt, rustc_span::def_id::LocalDefId, &rustc_middle::ty::typeck_results::TypeckResults>
            23: rustc_query_system::query::plumbing::try_execute_query::<rustc_query_impl::queries::typeck, rustc_query_impl::plumbing::QueryCtxt>
            24: rustc_data_structures::sync::par_for_each_in::<&[rustc_span::def_id::LocalDefId], <rustc_middle::hir::map::Map>::par_body_owners<rustc_hir_typeck::typeck_item_bodies::{closure#0}>::{closure#0}>
            25: rustc_hir_typeck::typeck_item_bodies
            26: <rustc_query_system::dep_graph::graph::DepGraph<rustc_middle::dep_graph::dep_node::DepKind>>::with_task::<rustc_middle::ty::context::TyCtxt, (), ()>
            27: rustc_query_system::query::plumbing::try_execute_query::<rustc_query_impl::queries::typeck_item_bodies, rustc_query_impl::plumbing::QueryCtxt>
            28: <rustc_query_impl::Queries as rustc_middle::ty::query::QueryEngine>::typeck_item_bodies
            29: <rustc_session::session::Session>::time::<(), rustc_hir_analysis::check_crate::{closure#7}>
            30: rustc_hir_analysis::check_crate
            31: rustc_interface::passes::analysis
            32: <rustc_query_system::dep_graph::graph::DepGraph<rustc_middle::dep_graph::dep_node::DepKind>>::with_task::<rustc_middle::ty::context::TyCtxt, (), core::result::Result<(), rustc_errors::ErrorGuaranteed>>
            33: rustc_query_system::query::plumbing::try_execute_query::<rustc_query_impl::queries::analysis, rustc_query_impl::plumbing::QueryCtxt>
            34: <rustc_query_impl::Queries as rustc_middle::ty::query::QueryEngine>::analysis
            35: <rustc_interface::passes::QueryContext>::enter::<rustc_driver_impl::run_compiler::{closure#1}::{closure#2}::{closure#3}, core::result::Result<(), rustc_errors::ErrorGuaranteed>>
            36: <rustc_interface::interface::Compiler>::enter::<rustc_driver_impl::run_compiler::{closure#1}::{closure#2}, core::result::Result<core::option::Option<rustc_interface::queries::Linker>, rustc_errors::ErrorGuaranteed>>
            37: rustc_span::with_source_map::<core::result::Result<(), rustc_errors::ErrorGuaranteed>, rustc_interface::interface::run_compiler<core::result::Result<(), rustc_errors::ErrorGuaranteed>, rustc_driver_impl::run_compiler::{closure#1}>::{closure#0}::{closure#0}>
            38: <scoped_tls::ScopedKey<rustc_span::SessionGlobals>>::set::<rustc_interface::interface::run_compiler<core::result::Result<(), rustc_errors::ErrorGuaranteed>, rustc_driver_impl::run_compiler::{closure#1}>::{closure#0}, core::result::Result<(), rustc_errors::ErrorGuaranteed>>
            39: std::sys_common::backtrace::__rust_begin_short_backtrace::<rustc_interface::util::run_in_thread_pool_with_globals<rustc_interface::interface::run_compiler<core::result::Result<(), rustc_errors::ErrorGuaranteed>, rustc_driver_impl::run_compiler::{closure#1}>::{closure#0}, core::result::Result<(), rustc_errors::ErrorGuaranteed>>::{closure#0}::{closure#0}, core::result::Result<(), rustc_errors::ErrorGuaranteed>>
            40: <<std::thread::Builder>::spawn_unchecked_<rustc_interface::util::run_in_thread_pool_with_globals<rustc_interface::interface::run_compiler<core::result::Result<(), rustc_errors::ErrorGuaranteed>, rustc_driver_impl::run_compiler::{closure#1}>::{closure#0}, core::result::Result<(), rustc_errors::ErrorGuaranteed>>::{closure#0}::{closure#0}, core::result::Result<(), rustc_errors::ErrorGuaranteed>>::{closure#1} as core::ops::function::FnOnce<()>>::call_once::{shim:vtable#0}
            41: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
                       at /rustc/8996ea93b6e554148c4286e62b613f12a3ee505c/library/alloc/src/boxed.rs:1988:9
            42: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
                       at /rustc/8996ea93b6e554148c4286e62b613f12a3ee505c/library/alloc/src/boxed.rs:1988:9
            43: std::sys::unix::thread::Thread::new::thread_start
                       at /rustc/8996ea93b6e554148c4286e62b613f12a3ee505c/library/std/src/sys/unix/thread.rs:108:17
            44: start_thread
                       at /build/glibc-SzIz7B/glibc-2.31/nptl/pthread_create.c:477:8
            45: clone
                       at /build/glibc-SzIz7B/glibc-2.31/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:95
          

note: we would appreciate a bug report: https://github.com/rust-lang/rust/issues/new?labels=C-bug%2C+I-ICE%2C+T-compiler&template=ice.md

note: rustc 1.69.0-nightly (8996ea93b 2023-02-09) running on x86_64-unknown-linux-gnu

note: compiler flags: --crate-type lib -C embed-bitcode=no -C debuginfo=2 -C incremental=[REDACTED]

note: some of the compiler flags provided by cargo are hidden

query stack during panic:
end of query stack
warning: `blart` (lib) generated 2 warnings
error: could not compile `blart`; 2 warnings emitted

[declanvk]

This was mainly implemented by a093d47 and ef6543e. Followup items in #23