Navigation Menu

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

Implement parallel::unique_copy. #2754

Merged
merged 8 commits into from Jul 12, 2017
Merged

Conversation

taeguk
Copy link
Member

@taeguk taeguk commented Jul 11, 2017

Check Box

@hkaiser hkaiser added this to the 1.1.0 milestone Jul 11, 2017
@taeguk taeguk changed the title Implement parallel::unique_copy. [WIP] Implement parallel::unique_copy. Jul 11, 2017
@taeguk taeguk changed the title [WIP] Implement parallel::unique_copy. Implement parallel::unique_copy. Jul 11, 2017
@hkaiser
Copy link
Member

hkaiser commented Jul 11, 2017

@taeguk: so is this ready to be reviewed now?

@taeguk
Copy link
Member Author

taeguk commented Jul 11, 2017

@hkaiser I'm ready to be reviewed.

Copy link
Member

@hkaiser hkaiser left a comment

Choose a reason for hiding this comment

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

Otherwise, LGTM. Thanks!


auto base_val = *first;

*dest++ = *first;
Copy link
Member

Choose a reason for hiding this comment

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

You can't dereference input iterators more than once for the same 'position'.

Copy link
Member Author

@taeguk taeguk Jul 11, 2017

Choose a reason for hiding this comment

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

As I know, until input iterator is incremented, several dereferences are permitted.
GCC implementation : https://github.com/gcc-mirror/gcc/blob/master/libstdc%2B%2B-v3/include/bits/stl_algo.h#L1093-L1097
MSVC14 (VS2015) implementation :

...
for (*_Dest++ = _Val; ++_First != _Last; )
			if (!_Pred(_Val, *_First))
				{	// copy unmatched
				_Val = *_First;
				*_Dest++ = _Val;
				}
...

Copy link
Member

Choose a reason for hiding this comment

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

Ok, I read the standard again - you're right. I'm not sure why I thought this to be undefined, I think I mixed it up with requirements for output iterators.

hpx::util::invoke(proj, *first)))
{
base_val = *first;
*dest++ = *first;
Copy link
Member

Choose a reason for hiding this comment

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

Same here, calling *first more than once without interleaving ++first is not allowed for input iterators. Not sure if this is worth changing as we know this will not be called with input iterators in the first place...

@hkaiser hkaiser merged commit 7e6627c into STEllAR-GROUP:master Jul 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Standard Algorithms
  
Merged to master
Development

Successfully merging this pull request may close these issues.

None yet

2 participants