I guess this might explain why the call was made ? On OSX:

../src/histogram.cc:162:5: error: ignoring return value of function declared with 'warn_unused_result' attribute [-Werror,-Wunused-result]
map->Set(
^~~~~~~~

Not sure why it did not fail locally, as seems to be reported for linux as well. Maybe my local compiler version?

I think I understand what is going on here:

• Set returns MaybeLocal and has V8_WARN_UNUSED_RESULT -> which results in a warning if the return value from Set is not used.
• IsEmpty() is one of the few methods on MaybeLocal that does not also have a V8_WARN_UNUSED_RESULT. Since it is marked const however, coverity knows that it does do anything other than satisfying the compiler.

Our options would seem to be:

• use ToLocalChecked instead of IsEmpty as it is not const and does not have V8_WARN_UNUSED_RESULT
• add a co-verity comment telling it to ignore the line.

ToLocalChecked will crash if the result is empty so we'd only use that if we want to know that the Set failed versus just ignoring and continuing.

No local test failures with ToLocalChecked so I think that means it's not expected that the Set can fail.

@jasnell since I think you are the main contributor to this part of the code do you think I should:

• use ToLocalChecked instead of IsEmpty as it is not const and does not have V8_WARN_UNUSED_RESULT
• add a co-verity comment telling it to ignore the line.

Some additional explanation/discussion above.

+1 to what @RaisinTen said.

If that's not an option, we should use USE(x) instead of x.IsEmpty(), which is literally there for the very purpose of avoiding "unused value" warnings.

I suggest just using USE(...) here as Anna mentioned.

Thanks, I'll look at @RaisinTen's suggestion and if that does not seem practical add the USE()

Code higher up looks like

 get percentilesBigInt() {
    if (!isHistogram(this))
      throw new ERR_INVALID_THIS('Histogram');
    this[kMap].clear();
    this[kHandle]?.percentilesBigInt(this[kMap]);
    return this[kMap];
  }

Returning the result of map-Set up to the JavaScript would only let us return undefined/null instead of an empty map, but I think this[kHandle]?.percentilesBigInt(this[kMap]); may already lead to the possibility of an empty map in case of an error/invalid state.

For that reason I think just using USE makes sense, will update with that.

Commit message will need updating to reflect the new approach.

@RaisinTen pushed update to change to add USE()

CI: https://ci.nodejs.org/job/node-test-pull-request/42190/

Landed in d86dcaa