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

Question regarding the design of the extension functions (subspan etc.) #65

Closed
vvish opened this issue May 4, 2021 · 5 comments
Closed

Comments

@vvish
Copy link
Contributor

vvish commented May 4, 2021

Hello,

I have a question regarding the design of the free functions:subspan, first and last:
As they do not explicitly accept span<T> are they intended to be used with any type that can be accepted by make_span function? If it is the case then the implementation seems to have an issue: as ADL is not finding them with non-span-specialization parameters the call to this functions should be qualified (currently they are not available in the nonstd namespace) and make_span in the trailing return type expression should be visible for non-ADL. Unfortunately the unit-tests cover only the cases where the span specialization is passed to the functions (works because of ADL), so it is not clear if the use-case was considered, although I personally find it to be useful (would be glad to contribute here).

If the functions are designed to be used with span-s only as wrappers for methods to avoid .template then maybe they could accept span<T> ?

Could you please clarify the use cases and rational behind the design?

Thank you.

vvish added a commit to vvish/span-lite that referenced this issue May 4, 2021
…inmoene#65)

Makes non-standard functiones available in the nonstd namespace and
moves their definitions to be located after make_span function definitions.
vvish added a commit to vvish/span-lite that referenced this issue May 4, 2021
…inmoene#65)

Makes non-standard functiones available in the nonstd namespace and
moves their definitions to be located after make_span function definitions.
vvish added a commit to vvish/span-lite that referenced this issue May 4, 2021
…inmoene#65)

Makes non-standard functions available in the nonstd namespace and
moves their definitions to be located after make_span function definitions.
vvish added a commit to vvish/span-lite that referenced this issue May 4, 2021
…inmoene#65)

Makes non-standard functions available in the nonstd namespace and
moves their definitions to be located after make_span definitions.
@martinmoene
Copy link
Owner

Hi @vvish

Thanks for bringing this up.

I've trouble to trace back where the idea for non-member first() etc. comes from. Given these functions are not available from namespace nonstd carries the suggestion that the idea/implementation isn't 'fully baked'.

Think the implementation is chosen to reduce the number of overloads needed by letting make_span()do most of the work. Wondering if the wide contract of first() etc. can lead to problems.

The changes in your fork seem good to me (after a quick-ish look).

Whether it would be better & enough to have first() etc. accept span<T> as argument, I'd appreciate to be educated on :)

@vvish
Copy link
Contributor Author

vvish commented May 5, 2021

Thank you @martinmoene for the quick reply.

The documentation https://github.com/martinmoene/span-lite/blob/master/README.md#first-last-and-subspan suggests that they were introduced "to avoid having to use the dot template syntax when the span is a dependent type". It looks like if it is the only intended usage then they could receive already constructed span<T> and simply delegate call to the member (this needs to be validated).

If they also act as 'factory' functions constructing sub-spans from span-compatible types (instead of chaining, like make_span().first() ), then they require wide contract and changes from my patch seem to be relevant.

I think in both cases, this functions should be available in nonstd namespace.

@martinmoene
Copy link
Owner

Hi @vvish

I think merging the changes you prepared seems a good start.

Then look if first() etc. can be made to take span<T> as argument or if they should remain factory functions.

vvish added a commit to vvish/span-lite that referenced this issue May 5, 2021
…inmoene#65)

Makes non-standard functions available in the nonstd namespace and
moves their definitions to be located after make_span definitions.
martinmoene pushed a commit that referenced this issue May 5, 2021
Makes non-standard functions available in the nonstd namespace and
moves their definitions to be located after make_span definitions.
@vvish
Copy link
Contributor Author

vvish commented May 6, 2021

Hi @martinmoene ,
That was quick :). Thank you.

And thank you very much for your lib-s. They are of great aid.

@martinmoene
Copy link
Owner

Thank you, nice to hear :) You're welcome.

Perhaps continue with a new issue like 'Change free functions first(), last(), subspan() to take a span?' to investigate this aspect?

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

No branches or pull requests

2 participants