assemblyscript icon indicating copy to clipboard operation
assemblyscript copied to clipboard

Published 20 hours ago • verified publisher icon AssemblyScript

fix: strictly check operator overload ambiguity

Open HerrCai0907 opened this issue 2 years ago • 12 comments

fixes: #2761

HerrCai0907 avatar Oct 08 '23 09:10 HerrCai0907

Perhaps AS240: Operator '==' overload ambiguity (choosing 'A#__eq' over 'B#__eq').?

dcodeIO avatar Oct 08 '23 15:10 dcodeIO

Perhaps AS240: Operator '==' overload ambiguity (choosing 'A#__eq' over 'B#__eq').?

Maybe AS240: Operator '==' overloading ambiguity (candidate overloads 'operator-overload-ambiguity/A#__eq' and 'operator-overload-ambiguity/B#__eq').

HerrCai0907 avatar Oct 08 '23 23:10 HerrCai0907

AS240: Ambiguous operator overload == (conflicting overloads 'operator-overload-ambiguity/A#__eq' and 'operator-overload-ambiguity/B#__eq') possibly.

CountBleck avatar Oct 08 '23 23:10 CountBleck

ping @dcodeIO @CountBleck

HerrCai0907 avatar Oct 13 '23 01:10 HerrCai0907

@CountBleck @dcodeIO Could I land it now? Since it introduces a break change, how can I upgrade minor version for AS? I remember daily publishing action will upgrade patch version.

HerrCai0907 avatar Jan 15 '24 05:01 HerrCai0907

I can't say all that much about the contents of the PR. @dcodeIO knows what's best.

To upgrade the minor version, you need the BREAKING CHANGE: or major: prefix, so merging the PR as-is should work.

CountBleck avatar Jan 15 '24 05:01 CountBleck

friendly ping @dcodeIO

HerrCai0907 avatar Apr 07 '24 05:04 HerrCai0907

Could this become a warning that mentions which overload has been picked by the compiler? Similar to my prior comment:

AS240: Operator '==' overload ambiguity (choosing 'A#__eq' over 'B#__eq').

Or do you think that there are good reasons to make it an error and hence a breaking change?

dcodeIO avatar Apr 07 '24 08:04 dcodeIO

I think we should forbidden this behavior because:

  1. a == b and b == a should be the same thing. It is so crazy if someone gets true from the first expr and gets false from the second.
  2. Since this is an extended grammar, we don't have references from TS. The language which supports operator overloading (I only know cpp) also have similar behavior.

HerrCai0907 avatar Apr 07 '24 10:04 HerrCai0907

Given that operator overloads are merely an internal mechanism, primarily for string implementation, and not recommended to be used for anything custom anyway as these won't typecheck, I guess it's fine to check these strictly and error otherwise. More generally, I'd probably even prefer that we limit their use, and encouraged the use of .equals methods or similar in own code, and with it eliminate the ambiguity this PR resolves.

dcodeIO avatar Apr 08 '24 17:04 dcodeIO

I'd probably even prefer that we limit their use, and encouraged the use of .equals methods or similar in own code

Agree. And I guess it will not break any actual code because nobody will consciously write two different comparison function.

HerrCai0907 avatar Apr 09 '24 02:04 HerrCai0907

I treat it as a fix instead of a break change since it should not break any user code. @dcodeIO WDYT?

HerrCai0907 avatar Jun 30 '24 14:06 HerrCai0907