Skip to content

Introduce generic saver/loader and make saving const#9

Merged
jermp merged 5 commits intojermp:masterfrom
adamant-pwn:master
Jun 21, 2024
Merged

Introduce generic saver/loader and make saving const#9
jermp merged 5 commits intojermp:masterfrom
adamant-pwn:master

Conversation

@adamant-pwn
Copy link
Copy Markdown
Contributor

Hi @jermp! When merging some sshash-related changes, it occured to us that your libraries currently require passing by non-const reference even when saving something, which, from our perspective, idiomatically should be done with const ref. We also often need to save/load stuff using a general ifstream/ofstream. Due to this I took it upon myself to

  • implement a more generic version of saver/loader which can work with arbitrary streams. I made sure that this change is non-intrusive, so it will only add some functionality, but shouldn't affect your existing workflows.
  • Make saving work with const references.

The later is, unfortunately, a bit more intrusive, but I strongly believe that it would be a better way to handle this. I also think that while propagating it to actual usecases require some work, it is straightforward enough, as I already did this in custom forks of pthash and sshash. If it is of interest to you, I can make separate pull requests to pthash and sshash to merge the new approach to "visit" them, and also help with the migration in your other projects, if necessary.

@jermp
Copy link
Copy Markdown
Owner

jermp commented Jun 21, 2024

Hi @adamant-pwn, and thanks for this. This was actually intentional to let us write just one visit method and not to have the const version as well. But, yes, I agree that it is straightforward to do as it requires adding another identical method.

@jermp jermp merged commit 2a546e3 into jermp:master Jun 21, 2024
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.

3 participants