NoAvailableAlias / nano-signal-slot

Pure C++17 Signals and Slots
MIT License
407 stars 60 forks source link

-Wunused-variable warnings in nano_observer.hpp #30

Closed jmarrec closed 3 years ago

jmarrec commented 3 years ago

When compiling with -Wall on gcc, the -Wunused-variable warnings are emitted in nano_observer.hpp.

NoAvailableAlias commented 3 years ago

Hello! Unfortunately this PR can't proceed as it currently is as those statements are absolutely necessary for the threading policies to function. Not sure what the best mitigation of this warning would be as adding "(void)lock;" after the definition is a bit ugly but does work.

The "auto lock" statements must exist due to being a mechanism through which threading policies inject their "policy" into Observer. (threading policy behaviors are injected at every location of "MT_Policy::" within Observer)

This PR also explains your opened issue as well and I will go over there to respond.

Edit:

Oh there appears to be a [[maybe_unused]] attribute now in c++17 which I wasn't aware of: https://en.cppreference.com/w/cpp/language/attributes/maybe_unused That would probably be a better resolution than (void)lock;

jmarrec commented 3 years ago

@NoAvailableAlias I was wary of commenting it out but looking briefly at the policy stuff for ST_policy here I figure it would affect results https://github.com/NoAvailableAlias/nano-signal-slot/blob/eafbceee9d8b4c56996e9e221bbf6bbae3550a73/nano_mutex.hpp#L55-L58. I'll try putting it back with a maybe_unused and report back and adjust PR and the issue either way. Thanks a lot for the prompt help and feedback!

jmarrec commented 3 years ago

@NoAvailableAlias I replace it with [[maybe_unused]]. (This doesn't solve my unit test problem but that is uncorrelated to this PR)

NoAvailableAlias commented 3 years ago

Thanks for the PR! I'll review issue #31 again after work today.