Compilation warning involving rput on MacOS

[Scott Baden]

The code is in the scott repo scott/rb1d/upcxx

You want to build OrderlyOutput

See the attached log for details.

This problem doesn't occur on Cori (where I used the nightly build)

[Paul Hargrove]

Does this occur with clang on Kotten?
module load upcxx/nightly/clang-5.0.0 .
I suspect that this is specific to the compiler, not to MacOS.

[Dan Bonachea]

The code that triggered the warning:

 rput(cstr,sbuff+myrank*maxLen,len,operation_cx::as_future()).wait();

This appears to be valid input, but the completion argument is also redundant in this case because that is the default completion. It's possible that deleting the redundant completion argument may silence the warning.

It may also explain wy we've never seen this in other codes because they usually don't explicit specify the default completion.

However, this warning looks to me like it's pointing to a real bug in the assertion code in rput.hpp (copied in two places):

332     UPCXX_ASSERT_ALWAYS((
333       detail::completions_has_event<Cxs, operation_cx_event>::value |
334       detail::completions_has_event<Cxs, remote_cx_event>::value,
335       "Not requesting either operation or remote completion is surely an "
336       "error. You'll have know way of ever knowing when the target memory is "
337       "safe to read or write again."
338     ));

The bug is there should not be two sets of parentheses in the call to UPCXX_ASSERT_ALWAYS - this bug causes the compiler to discard the expression it is meant to check (which is why the compiler throws an unused-value warning), and more importantly disables the check the assertion is meant to enforce!

[john bachan]

Confirmed. rget.hpp and vis.hpp have similar asserts but do not have this bug, their double parens are correctly nested only around the (comma containing) condition argument.

[Scott Baden]

Actually, I added the completion object hoping that it would remove the problem. I just tested again without the completion object and the error remains (I tested on the mac)

[Dan Bonachea]

Issue #150 was marked as a duplicate of this issue.

[john bachan]

Fixed issue 146. rput had a UPCXX_ASSERT_ALWAYS that was generating compile errors due to misplaced parenthesis (which are there to "hide" commas from the preprocessor).

→ <<cset 5b2c1c5ea284>>