Build warning: s2winding_operation.cc:413:1: warning: control reaches end of non-void function

[spaetz]

Compiler warns about

/<<PKGBUILDDIR>>/src/s2/s2winding_operation.cc: In member function ‘bool s2builderutil::WindingLayer::MatchesRule(int) const’:
/<<PKGBUILDDIR>>/src/s2/s2winding_operation.cc:413:1: warning: control reaches end of non-void function [-Wreturn-type]
  413 | }
      | ^

bool WindingLayer::MatchesRule(int winding) const {
  switch (op_.rule_) {
    case WindingRule::POSITIVE:  return winding > 0;
    case WindingRule::NEGATIVE:  return winding < 0;
    case WindingRule::NON_ZERO:  return winding != 0;
    case WindingRule::ODD:       return (winding & 1) != 0;
  }
}

And indeed, if winding were a different value than POS, NEG, ODD or NON_ZERO, we would return void, so a catchall return is needed to make the compiler happy. Perhaps return False if we fall through?

[jmr]

Could you check that adding __builtin_unreachable(); after the switch makes the warning go away?

[spaetz]

Will do and report back...

-------- Ursprüngliche Nachricht -------- Von: Jesse Rosenstock @.> Gesendet: 17. November 2022 09:40:51 MEZ An: google/s2geometry @.> CC: Sebastian Spaeth @.>, Author @.> Betreff: Re: [google/s2geometry] Build warning: s2winding_operation.cc:413:1: warning: control reaches end of non-void function (Issue #280)

Could you check that adding __builtin_unreachable(); after the switch makes the warning go away?

-- Reply to this email directly or view it on GitHub: https://github.com/google/s2geometry/issues/280#issuecomment-1318279649 You are receiving this because you authored the thread.

Message ID: @.***> -- Sent from mobile device.

[spaetz]

I applied https://salsa.debian.org/DebianOnMobile-team/s2geometry/-/commit/7f2d08e78f29385a706049b0278ba2bda2603a10

and it silences the build warning just fine and compiles. I have not tested running the code.

[jmr]

Thanks for the quick patch. This will be fixed by adding an S2_INTERNAL_UNREACHABLE macro when we do the next code push, so that will break your patch. I'll update this bug when we do that.

[jmr]

Fixed by #297.