Skip to content

Proposals for KeySpecifier and its macro

After working on !1796 (merged) I had some thoughts. I'm not sure which of these are improvements but before I do any of them I thought I'd get some input, especially from @gabi-250.

Things I think are probably a good idea:

  1. Rename the KeySpecifierDefault macro to KeySpecifier.
  2. Have .arti_pattern for SomeSpecifier take &SomeSpecifierPattern, which would be a derived struct which is like SomeSpecifier but contains Options, rather than positional arguments for each field.
  3. Make .arti_pattern a trait method (depends on 2)
  4. Reimplement the path construction in terms of a (private: doc-hidden) helper function which is something like fn arti_path_from_components(path_comps: &[&dyn KeySpecifierComponent], leaf_comps: &[&dyn KeySpecifierComponent]) -> Result<ArtiPath, ...>. Likewise pattern construction. This will help reduce monomorphisation (ie, reduce code size and improve build times) and may be clearer. It will probably also abolish .arti_path_prefix.
  5. See if there is any way to reduce monomorphisation in TryFrom<KeyPath> for SomeSpecifier.
  6. No longer have the KeySpecifier d-a macro generate new. If the caller wants a constructor, they can derive one using some other macro.
  7. Abolish the public macro-generated .prefix method.

Things I'm not sure about:

  1. Split the pattern things into their own trait+macro, KeySpecifierPattern. (If we do 2 and 3, is the pattern trait implemented for SomeKeySpecifier via associated types, or for SomeKeySpecifierPattern? Probably the impl is for SomeKeySpecifierPattern so that it can be &dyn, but the d-a macro is applied to SomeKeySpecifier?)
  2. Change KeySpecifierComponent::to_component to KeySpecifierComponent::fmt_path with the signature of Display:fmt. This reduces allocations but it makes the API less convenient. IDK the right balance here.
  3. Have KeySpecifierComponent have a fmt_human method and use that to provide KeyPathInfo::extra_info (currently the KeyInfoExtractor impl doesn't ever provide those). We want some way to provide the extra_info but I'm not sure this is it. (See also 9; the two stringifications should have the same signature, probably.)

Things I think are probably a bad idea but I wanted to explicitly discuss and probably reject:

  1. Separate out InfoExtractor, so that it's a thing you must derive separately. I think this is a bad idea because forgetting it is probably not detected, and leads to bad output from user-facing key listing utilities.
  2. Add to KeySpecifier, this associated type: type KeyInfoExtractor: KeyInfoExtractor where Self: Sized;. To force KeyInfoExtractor to not be forgotten. But this prevents us implementing KeySpecifier for ArtiPath.