search

skypjack / uvw

Header-only, event based, tiny and easy to use libuv wrapper in modern C++ - now available as also shared/static library!
MIT License
1.88k stars 211 forks source link

fix ambiguous overloaded from c++20 with explicit cast #280

Closed Ender-events closed 1 year ago

Ender-events commented 1 year ago

Hello, the same problem as #278

Starting with c++20, comparison operators can now be reversed and lead to ambiguous overloads. (cf https://brevzin.github.io/c++/2019/07/28/comparisons-cpp20/#reversing-primary-operators, https://stackoverflow.com/questions/65946930/ambiguous-overloaded-operator-c20)

An minimal reproductive example: https://godbolt.org/z/jTP469TMf

Fix it with an explicit type conversion

relative issue from an other project: https://github.com/boostorg/date_time/issues/132

fiorentino-at-adesso commented 1 year ago

@skypjack is `uvw' explicitly supporting C++20 ? (we should definitely aim to it) If yes, we should change the CMake conf accordingly and see what happens:

e.g., target_compile_features(uvw INTERFACE cxx_std_17)

skypjack commented 1 year ago

We don't actually @fiorentino-at-adesso the focus so far was to release the v3 😅 wanna set migrating to C++20 as our next step?

fiorentino-at-adesso commented 1 year ago

We don't actually @fiorentino-at-adesso the focus so far was to release the v3 😅 wanna set migrating to C++20 as our next step?

Of course @skypjack! before that we should tag the version 3.x.x so I can add it to meta-uvw, though.

skypjack commented 1 year ago

Oh, sorry, I was sure I did 🤔 I try to fill the gap asap (although not right away cuz I'm a bit on a rush today 😅 forgive me). I'd wait to merge these PRs then and make them part of a larger review aimed at fixing everything to fully support C++20. Makes sense?

skypjack commented 1 year ago

Damn, that's impressive. I saw an almost ready release in libuv and was waiting for it to pop up before tagging uvw but it's getting incredibly long. 😡 Let's do it right away and who cares. I'll create a new tag later on. 👍

skypjack commented 1 year ago

@fiorentino-at-adesso I created the new versio nand also updated a few things here and there. What about creating a new branch to target C++20, move all these PRs there, then fix everything before merging upstream?

fiorentino-at-adesso commented 1 year ago

@fiorentino-at-adesso I created the new versio nand also updated a few things here and there. What about creating a new branch to target C++20, move all these PRs there, then fix everything before merging upstream?

We should definitely address all this c++20 warnings/errors together.