Take advantage of C++14 lambda capture initialization syntax, where possible

[hkaiser]

Proposed Changes

• utilize C++14 capture initialization syntax for lambdas wherever possible
• flyby: removed unneeded captures

[taeguk]

hpx/hpx/parallel/algorithms/merge.hpp

Lines 262 to 294 in 0b2baf1

	typedef typename std::remove_reference<ExPolicy>::type ExPolicy_;
	typedef typename std::remove_reference<Comp>::type Comp_;
	typedef typename std::remove_reference<Proj1>::type Proj1_;
	typedef typename std::remove_reference<Proj2>::type Proj2_;
	hpx::future<result_type> f = execution::async_execute(
	policy.executor(),
	std::bind(
	[first1, last1, first2, last2, dest](
	ExPolicy_& policy, Comp_& comp,
	Proj1_& proj1, Proj2_& proj2) -> result_type
	{
	try {
	parallel_merge_helper(
	std::move(policy), first1, last1, first2, last2, dest,
	std::move(comp), std::move(proj1), std::move(proj2),
	false, lower_bound_helper());
	return hpx::util::make_tuple(last1, last2,
	dest + (last1 - first1) + (last2 - first2));
	}
	catch (...) {
	util::detail::handle_local_exceptions<ExPolicy>::call(
	std::current_exception());
	}
	// Not reachable.
	HPX_ASSERT(false);
	},
	std::forward<ExPolicy>(policy),
	std::forward<Comp>(comp),
	std::forward<Proj1>(proj1),
	std::forward<Proj2>(proj2)));

@hkaiser I think that this PR also can be applied to above codes like:
https://github.com/STEllAR-GROUP/hpx/pull/3092/files#diff-f80eb11bc86e0246f9228af312f375e4R345
for consistency and simplicity.

[hkaiser]

@taeguk not sure if it makes sense to move the iterators in the capture list of the lambda. Did I misunderstand what you were trying to say?

[hkaiser]

The failing tests are unrelated.

[sithhell]

Hmm, I don't think this is a good idea in general. We'll introduce inefficiencies when not having the captures or compile errors (if the objects are not copyable in C++11 mode, for example).

[hkaiser]

Huh? This improves the picture for C++14 and leaves things unchanged as they are in C++11.

[sithhell]

I didn't through all changes. It might lead to those problems. It does improve the picture for C++14 indeed, but leaves C++11 code behind. As said, I'm not sure if it's a good idea. If we need to keep C++11 compatibility, a better option might be to capture via bind/deferred_call.

[hkaiser]

Improving the picture for C++11 is independent of this PR.

[sithhell]

That's exactly why I'm saying this might not be a good idea. A solution that would work for both would be significantly different, and requires way more boilerplate. What you propose though, is a C++14 only solution (with a C++11 facade, that's not doing anything). I don't think it's bad to have it that way, but I think it should be without that macro that's not giving us any benefit for older compilers, and will lead to problems eventually.

[hkaiser]

Sure, I understand. This is completely orthogonal to this PR, though. Things do not change for C++11 based compilation, no matter whether we apply this PR or not. But things improve for C++14 based compilation, so it's worth going ahead.

[sithhell]

Sure, I understand. This is completely orthogonal to this PR, though. Things do not change for C++11 based compilation, no matter whether we apply this PR or not.

Right, which I think is a potential problem ...

But things improve for C++14 based compilation, so it's worth going ahead.

... as only C++14 code improves. there isn't any problem in particular right now. The problem is the code that will be written against those interfaces. We won't notice any problem since all our compilers actually support lambda init captures. There is no gain in artificially introducing this distinction, that's my main problem. We either solve the problem for all supported flavors of c++ properly (which I strongly support by bumping the requirements) or we keep it as it is to avoid future problems with the two incompatible code paths.

[hkaiser]

@sithhell so you're refusing to accept an improvement for 80% of the systems we're running on for the sake of a potential problem which might accidentally be introduced by some careless change on 20% of the systems?

What alternative solution do you suggest?

[sithhell]

What I'm suggesting is to leave the 20% behind properly, by removing support for them and bump the minimal version to C++11 with C++14 lambda init captures. This way, we ensure that no such careless change sneaks in. Besides, problems will most likely manifest in user code first where we don't have any control.

[hkaiser]

That would currently leave behind all of CUDA. I think that's not what we want.

[taeguk]

@hkaiser I didn't say about iterators like first1. I said about policy, comp, proj1, and proj2.
In that code, I used std::bind() for perfect forwarding policy, comp, proj1, and proj2 in both c++11 and c++14. Maybe this could be one of better solutions like @sithhell said.
But, I wanted to say about inconsistency. And I suggested removing std::bind() and using HPX_CAPTURE_FORWARD for consistency. As I think, if we will modify all codes that use HPX_CAPTURE_FORWARD, it can be helpful because we can just find that string to know modification points.
So I think, for consistency, that codes I refered above should be modified, too.

[hkaiser]

@taeguk: it's almost funny that you're proposing the exact opposite of what @sithhell would like to see. You would like to get rid of bind()/deferred_call() while @sithhell would like to get rid of the lambdas replacing those with bind et.al.

All what I propose for now is to improve the existing lambdas for modern compilers. We can move the existing bind()'s to C++14 lambdas over time as compilers catch up.

For your other suggestion: would you mind creating a patch demonstrating what you have in mind, please?

[taeguk]

@hkaiser

All what I propose for now is to improve the existing lambdas for modern compilers. We can move the existing bind()'s to C++14 lambdas over time as compilers catch up.

Okay, I agree. I think I was wrong. Now I think just leaving it as it is is better. We can move the existing bind()'s to C++14 lambdas later. I think that we will search not only HPX_CAPTURE_FORWARD but also std::bind() to do that in future.

For your other suggestion: would you mind creating a patch demonstrating what you have in mind, please?

Now there is no need to make patch because my opinion is changed like above.

[sithhell]

Bumping the requirements or fixing it properly such that the remaining 20% aren't left behind. What are the 20% that don't support init captures?

[hkaiser]

@sithhell approved these changes: http://irclog.cct.lsu.edu/ste~b~~b~ar/2018-01-05#93869;