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.

Sign up for GitHub

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

Jump to bottom

[enum.copy_options] Rephrase restriction on copy_options values. #1445

Merged
merged 1 commit into from Mar 20, 2017
Merged

[enum.copy_options] Rephrase restriction on copy_options values. #1445

merged 1 commit into from Mar 20, 2017

Conversation

jensmaurer
Copy link
Member

Fixes #1422.

jwakely
jwakely approved these changes Feb 9, 2017
View reviewed changes
Copy link
Member

@jwakely jwakely left a comment

Choose a reason for hiding this comment

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

As I said in #1422 I think this is more precise, but seems clumsy. I wish we had a better way to say it.

jensmaurer added a commit to jensmaurer/draft that referenced this pull request Feb 10, 2017
@jensmaurer
[fs.op.copy], [fs.op.copy_file] Rephrase requirements on copy_options.
575976b 
Use bitmask 'element' phrasing for restrictions on copy_options.

Fixes cplusplus#1445.
@jensmaurer
Copy link
Member Author

As discovered in #1422, it turns out that the requirements are already spelled out with the two function families that use copy_options. My new patch simply removes the general requirement and slightly adjusts the requires clauses with the individual functions.

@jensmaurer
Copy link
Member Author

@jwakely: What do you think?

@timsong-cpp
Copy link
Contributor

timsong-cpp commented Feb 11, 2017 •
edited

Hmm, one is "from each copy_options option group", the other is simply "from each option group". Might want to make them consistent as a drive-by fix.

jensmaurer added a commit to jensmaurer/draft that referenced this pull request Mar 18, 2017
@jensmaurer
[fs.op.copy], [fs.op.copy_file] Rephrase requirements on copy_options.
6aa71e6 
Use bitmask 'element' phrasing for restrictions on copy_options.

Fixes cplusplus#1445.
@jensmaurer
Copy link
Member Author

Rebased.

@jwakely
Copy link
Member

jwakely commented Mar 18, 2017

Looks good to me.

@jensmaurer
[fs.op.copy], [fs.op.copy_file] Rephrase requirements on copy_options.
e73710b 
Use bitmask 'element' phrasing for restrictions on copy_options.

Fixes cplusplus#1445.
@jensmaurer
Copy link
Member Author

Now also addressing @timsong-cpp's comment.

@jensmaurer jensmaurer added this to the C++17 milestone Mar 18, 2017
@zygoloid zygoloid merged commit 04a9e5d into cplusplus:master Mar 20, 2017
@jensmaurer jensmaurer deleted the b5 branch March 20, 2017 07:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jwakely jwakely jwakely approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
C++17 DIS
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@jensmaurer @timsong-cpp @jwakely @zygoloid