Skip to content

Default Argument and CHUNK_SIZE Refactor#155

Open
ErikMcClure wants to merge 16 commits intojneem:mainfrom
Fundament-Institute:main
Open

Default Argument and CHUNK_SIZE Refactor#155
ErikMcClure wants to merge 16 commits intojneem:mainfrom
Fundament-Institute:main

Conversation

@ErikMcClure
Copy link

@ErikMcClure ErikMcClure commented Mar 4, 2026

This PR is a large refactor and update of the library, moving it to Rust 2024, updating the rust version and updating most of the dependencies (I opted to ignore the bincode situation #146 for now, though). It also adds a persistent map and fold to the Vector, which I needed for a library, but this shouldn't impact the API.

The major change here is to get rid of the Generic* versions of the structs and instead use default type arguments, while providing a new() function using the default type arguments and with_* functions that provide more granular control. This mirrors how the standard library handles things. In most cases, simply replacing GenericName with Name will suffice in current user code, but it does require using explicit arguments in two cases: ::<_>::from_iter() and ::<_>::unit().

In addition, the RRB data structure and everything that depends on it (like Vector) was changed to use a compiler-time CHUNK_SIZE constant. To be more consistent, this change should also be applied to the btree's NODE_SIZE, which was previously requested (#145) but I'm not currently using the btree. This should at least give you an idea of what the consequences of moving to a const generic will be in case you have your own plans for that.

This is a substantial breaking change to the library API, which would require yet another version increment - apparently the version was just updated to 7.0 a few weeks ago. I don't actually know who the primary users of this library are, so I am offering this PR just in case the current maintainers are interested in any of these changes.

@jneem
Copy link
Owner

jneem commented Mar 4, 2026

Thanks for the PR! I haven't really looked at it much yet, but the current type alias approach was chosen to minimize the need for annotations in the common case, as discussed a bit here: #90 (comment)

@ErikMcClure
Copy link
Author

After taking a look at #90 , it seems I reverted the behavior to the original approach done by @inker0, which adheres to how the standard library handles things and avoids breaking the macros, but does require additional type annotations in the two cases I mentioned. During my usage, I never encountered another location where additional types would actually be needed - they could always be inferred as long as it wasn't a function creating a new Vector. In fact, because I didn't use from_iter, my own code didn't have to be changed at all after moving to default arguments. However, if you prefer the type alias approach, then let me know if there are any other pieces of this PR (such as the 2024 update or CHUNK_SIZE change) that you would be interested in as a separate PR.

If you do end up adding CHUNK_SIZE/NODE_SIZE const generics, even if you keep the type aliases, I would recommend adding default arguments to the Generic* types, as this makes it easier to work with them when they start accumulating additional generic parameters.

@jneem
Copy link
Owner

jneem commented Mar 5, 2026

I don't have very strongly held opinions on this, but

  • I'm inclined to prioritize the common case (i.e. the users that don't want to think about the extra generic parameters)
  • I prefer to avoid changing things (i.e. away from the type aliases in favor of default arguments) unless there's a clear benefit. I'm not sure that "std does it that way" is a clear benefit, because as pointed out in the previous discussion, std's behavior reflects backwards-compatibility constraints and evolution over time.

Absolutely no objection to adding default parameters to the Generic* types, though.

@jneem
Copy link
Owner

jneem commented Mar 5, 2026

Also: starting tomorrow I'll be AFK for a week and a half

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants