Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unintended cast to bool? #471

Closed
KarimLUCCIN opened this issue Mar 28, 2023 · 3 comments
Closed

Unintended cast to bool? #471

KarimLUCCIN opened this issue Mar 28, 2023 · 3 comments

Comments

@KarimLUCCIN
Copy link

return ExecuteRectClip(rect, PathsD{ line }, precision);

It looks like the line should be instead return ExecuteRectClip(rect, PathsD{ line }, false, precision);, otherwise it casts precision as a bool.

@AngusJohnson
Copy link
Owner

AngusJohnson commented Mar 30, 2023

It looks like the line should be instead return ExecuteRectClip(rect, PathsD{ line }, false, precision);, otherwise it casts precision as a bool.

No, it should be returning ...
return ExecuteRectClipLines(rect, PathsD{ line }, precision);

@KarimLUCCIN
Copy link
Author

KarimLUCCIN commented Apr 1, 2023

I got a compiler warning on that line because for all of the possible overloads, it accepts a bool first for convex_only , and then a int for precision:

inline PathsD ExecuteRectClip(const RectD& rect,

inline Paths64 ExecuteRectClip(const Rect64& rect,

And so calling with PathsD{ line }, precision was casting precision into the convex_only parameter

@AngusJohnson
Copy link
Owner

AngusJohnson commented Apr 2, 2023

Thanks for the feedback Karim.

I suspect my reply above was too brief and hence confusing.
The function I was referring to there was the following one:

  inline PathsD ExecuteRectClipLines(const RectD& rect, const PathD& line, int precision = 2)
  {
    return ExecuteRectClipLines(rect, PathsD{ line }, precision);
  }

If that was the function you amended, then I don't see why you'd be getting compiler errors now.
And there's no convex_only parameter in any of the overloaded ExecuteRectClipLines functions.

However, WRT ExecuteRectClip (ie not ExecuteRectClipLines), I can see that having 2 optional parameters where the second one can be implicitly cast to the first is asking for trouble, and is poor design. So I'm inclined to make the convex_only parameter no longer optional.

AngusJohnson added a commit that referenced this issue Apr 2, 2023
Changed a parameter in ExecuteRectClip() (C++ only) (#471)
Fixed a crash in Clipper.Offset when not paths are submitted before offsetting. (C#, Del.)(#476)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants