#872: add encoding member bits in handler field

[jstrzebonski]

• adds encoding member bit in handler field
• cleans up all the places that previously used member boolean
• fixes handlers' ID extraction
• standardizes making and getting handlers
• remove redundant const refs to HandlerType

Fixes #872

[lifflander]

Here is an overview of what got changed by this pull request:

Clones added
============
- src/vt/pipe/pipe_manager_tl.impl.h  2
         

See the complete overview on Codacy

[jstrzebonski]

OK, so I found a problem, and it's pretty dumb :- |
makeHandler() accepts several boolean, some of them with default values. Adding another one, without taking care of correctly pass it to function, screwed up the code. I'm fixing it right know and adding utest to check such scenario.

Also I was thinking, maybe it might make sense to replace that parameters with structs that have only one explicit ctor? That way it won't be possible anymore to pass arguments in wrong order. Something like this:

struct IsAuto {
  constexpr explicit IsAuto(bool const is_auto) : is_auto_{is_auto} {}
  operator bool() const { return is_auto_; }

private:
  bool is_auto_;
};

and refactor makeHandler to accept args of that special types, instead of regular bools.

HandlerIdentifierType and HandlerControlType could also be refactor that way..

[JacobDomagala]

Looks good overall. After the fixes to CI it should be good to be merged :)

[jstrzebonski]

Looks promising, but not quite finished yet. Rework and recheck some of the files / changes considering especially handlers making and ids extraction.

[jstrzebonski]

I don't know if I should, but I standardized the way the handlers are made and get. There were a lot of test failures before, and now (at least locally on my machine) all of them pass.

[lifflander]

I don't know if I should, but I standardized the way the handlers are made and get. There were a lot of test failures before, and now (at least locally on my machine) all of them pass.

Looking over the code, I like the idea of standardizing how handlers are made and accessed. I think a lot of that code needs cleanup!

[codecov]

Codecov Report

Merging #1118 (5e00be2) into develop (9119934) will increase coverage by 0.04%.
The diff coverage is 90.58%.

@@             Coverage Diff             @@
##           develop    #1118      +/-   ##
===========================================
+ Coverage    79.19%   79.24%   +0.04%     
===========================================
  Files          714      715       +1     
  Lines        26939    26984      +45     
===========================================
+ Hits         21335    21383      +48     
+ Misses        5604     5601       -3     

Impacted Files	Coverage Δ
src/vt/messaging/active.cc	80.98% <ø> (ø)
src/vt/messaging/active.h	87.09% <ø> (ø)
src/vt/messaging/envelope/envelope_setup.impl.h	100.00% <ø> (ø)
src/vt/parameterization/parameterization.h	100.00% <ø> (ø)
src/vt/pipe/callback/cb_union/cb_raw_base.h	100.00% <ø> (ø)
...t/pipe/callback/handler_bcast/callback_bcast_tl.cc	100.00% <ø> (ø)
.../vt/pipe/callback/handler_send/callback_send_tl.cc	100.00% <ø> (ø)
.../callback/objgroup_bcast/callback_objgroup_bcast.h	100.00% <ø> (ø)
src/vt/pipe/pipe_manager_tl.impl.h	99.08% <ø> (-0.04%)	⬇️
...c/vt/registry/auto/functor/auto_registry_functor.h	100.00% <ø> (ø)
... and 45 more

[jstrzebonski]

Also, there are two, very very very minor things, that kind of irritates me so much:

1. passing fundamental types by const refs (in this PR's case it's HandlerType, which simply is int64_t)
2. inconsistency with passing params sometimes by const value and sometimes by value (even if const val could be used).

I know that these are the least of our problems and things on the TODO list, but I wonder if we could, and want to, somehow unify this, going step by step, from time to time. I could start with HandlerType in this PR.

What do you think guys?

[jstrzebonski]

OK, so in the end, I've removed redundant const refs to HandlerType.

[jstrzebonski]

@lifflander @JacobDomagala @cz4rs @PhilMiller
Please review the changes.

[PhilMiller]

Looks like some tiny stray changes, but otherwise good.