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

First bite of implementing span #113

Closed
wants to merge 1 commit into from
Closed

First bite of implementing span #113

wants to merge 1 commit into from

Conversation

gongminmin
Copy link

@gongminmin gongminmin commented Sep 18, 2019

Description

Implements span by following P0122R7 and P1024R3 Enhancing span Usability.

Related work item: #4

Checklist:

  • I understand README.md.
  • If this is a feature addition, that feature has been voted into the C++
    Working Draft.
  • Any code files edited have been processed by clang-format 8.0.1.
    (The version is important because clang-format's behavior sometimes changes.)
  • Identifiers in any product code changes are properly _Ugly as per
    https://eel.is/c++draft/lex.name#3.1 .
  • Identifiers in test code changes are not _Ugly.
  • Test code includes the correct headers as per the Standard, not just
    what happens to compile.
  • The STL builds and test harnesses have passed (must be manually verified
    by an STL maintainer before CI is online, leave this unchecked for initial
    submission).
  • This change introduces no known ABI breaks (adding members, renaming
    members, adding virtual functions, changing whether a type is an aggregate or
    trivially copyable, etc.). If unsure, leave this box unchecked and ask a
    maintainer for help.

P0122R7 <span>
P1024R3 Enhancing span Usability
@StephanTLavavej
Copy link
Member

Thank you! As we are working on getting our tests and CI active, our full review will be delayed until then.

@miscco
Copy link
Contributor

miscco commented Sep 18, 2019

Given the discussion in #104 wouldnt span also need the special non-member overloads for begin and end wrt to rvalues

@CaseyCarter
Copy link
Member

CaseyCarter commented Sep 18, 2019

Given the discussion in #104 wouldnt span also need the special non-member overloads for begin and end wrt to rvalues

Yes, there have been substantial changes to span in the working draft in addition to P1024R3 since P122R7 was merged.

This header appears to be derived from the MIT-licensed GSL implementation of span.

Copy link
Member

@BillyONeal BillyONeal left a comment

Choose a reason for hiding this comment

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

This header appears to be derived from the MIT-licensed GSL implementation of span.

I don't think we can accept this change in any form.

@StephanTLavavej
Copy link
Member

Filed #117 as we haven’t described our license concerns yet (we didn’t expect to receive major PRs this early).

@StephanTLavavej
Copy link
Member

Closing this PR; due to license concerns and Working Paper churn, we expect to implement span from scratch in the near future, instead of basing it on GSL's implementation in any way.

@gongminmin gongminmin deleted the mgong/span branch October 9, 2019 06:45
@SuperWig SuperWig mentioned this pull request Mar 8, 2020
4 tasks
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.

5 participants