Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow use of operator-> with huffman::table::const_iterator #52

Merged
merged 1 commit into from
Jun 26, 2023

Conversation

oliverlee
Copy link
Collaborator

Replace use of views::transform with a custom view type
detail::base_view<V, B> that casts a derived type (i.e. elements of
view V) to base B.

detail::base_view<V, B> requires the reference type of underlying view
V be a true reference, thus detail::base_view<V, B>::iterator
defines operator-> to return a pointer to B*.

resolves #49

Change-Id: I2c9d14705bd5a0d8f7e562006913906d40c7fe86

@oliverlee oliverlee requested a review from garymm June 13, 2023 00:45
@oliverlee oliverlee force-pushed the I2c9d14705bd5a0d8f7e562006913906d40c7fe86 branch from a59a5b8 to 115422e Compare June 13, 2023 00:46
@oliverlee oliverlee changed the title Allow use of operator-> with huffman::table iterator Allow use of operator-> with huffman::table::const_iterator Jun 13, 2023
@oliverlee oliverlee force-pushed the I2c9d14705bd5a0d8f7e562006913906d40c7fe86 branch from 115422e to e004d4e Compare June 13, 2023 00:47
@oliverlee
Copy link
Collaborator Author

oliverlee commented Jun 13, 2023

I didn't document much. The implementation of detail::base_view::iterator is largely boilerplate.

See https://wg21.link/p2727

@oliverlee oliverlee force-pushed the I2c9d14705bd5a0d8f7e562006913906d40c7fe86 branch 2 times, most recently from cce3ecc to 52f27b3 Compare June 13, 2023 02:05
Copy link
Owner

@garymm garymm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sort of horrifying how much code it takes for this. Can we go over this in person or on a call?

@oliverlee oliverlee force-pushed the I2c9d14705bd5a0d8f7e562006913906d40c7fe86 branch 2 times, most recently from ee522c2 to 52f27b3 Compare June 16, 2023 00:29
@oliverlee oliverlee requested a review from garymm June 16, 2023 00:30
Copy link
Owner

@garymm garymm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is blocking you from doing something else feel free to merge it in but I'd rather discuss it first, maybe next week.

  1. I'd like to understand it
  2. I don't think it's actually worth all the added code, given that the table const_iterator is just an implementation detail of our library.

@oliverlee
Copy link
Collaborator Author

If this is blocking you from doing something else feel free to merge it in but I'd rather discuss it first, maybe next week.

1. I'd like to understand it

2. I don't think it's actually worth all the added code, given that the table const_iterator is just an implementation detail of our library.

Yeah let's wait. Hopefully I can convince you otherwise after chatting about it :)

@oliverlee oliverlee requested a review from garymm June 16, 2023 04:05
Replace use of `views::transform` with a custom view type
`detail::base_view<V, B>` that casts a derived type (i.e. elements of
view `V`) to base `B`.

`detail::base_view<V, B>` requires the reference type of underlying view
`V` be a true reference, thus `detail::base_view<V, B>::iterator`
defines `operator->` to return a pointer to `B*`.

resolves #49

Change-Id: I2c9d14705bd5a0d8f7e562006913906d40c7fe86
@oliverlee oliverlee force-pushed the I2c9d14705bd5a0d8f7e562006913906d40c7fe86 branch from 52f27b3 to 555ae92 Compare June 26, 2023 00:16
@oliverlee oliverlee enabled auto-merge (rebase) June 26, 2023 00:16
@oliverlee oliverlee merged commit 8599340 into master Jun 26, 2023
@oliverlee oliverlee deleted the I2c9d14705bd5a0d8f7e562006913906d40c7fe86 branch June 26, 2023 00:26
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.

Allow use of arrow operator-> on return value of huffman::table::find
2 participants