Handling of ambiguous conversions

[radeusgd]

With #8245 I've fixed the wrong rendering of the ambiguous conversion error, but I have also demonstrated with a test that in non-strict mode, the first conversion just works and the user is not robustly notified of the ambiguity error: https://github.com/enso-org/enso/pull/8245/files#diff-86c751927e1df682de78976263b76bf4ae7098b14dc5a3c1938a47135c2030f1R60-R77

This may not be ideal, so likely we may want to further improve it (although I think it will be best done as a separate PR - to get this one - fixing a compiler crash when the duplicate conversions are encountered - in without delay).

What should we do with ambiguous conversions?

1. One solution proposed by @JaroslavTulach is to replace both clashing nodes in a file with an error node. This will make the code from the example above no longer work. However, the downside is that because the 2 definitions are replaced by error nodes, what it will mean for conversion resolution is just that no conversion is registered to be defined in the given module. That is not ideal, because it means that:
   i. Either the conversion resolution will fail with No_Such_Conversion - with no mention of the 'ambiguous' error.
   ii. Or even worse, if there is a conversion defined somehwere else, that conversion may be used, even if the current file has these 2 definitions.

I think this shows us that it may be beneficial to enable reporting of diagnostics even in non-strict mode. That way, at least the ambiguous conversion error would be logged (and I imagine in later iterations, should also be displayed in the IDE), so that the user can have any indication of the error.

2. If our primary concern is to ensure that non-strict mode behaves consistently with strict mode, with the exception that only errors affecting the execution path are reported, what we could do is - if there are 2 conversions like this - we can register a 'fake' conversion that will replace the 2 clashing ones and will throw a "Ambiguous" error when executed - ensuring that the 'ambiguity' error is reported consistently with strict mode.

3. Related to that is the general issue of ambiguous conversions. Maybe we should have a separate IR pass that can check if the current scope contains ambiguous conversions for any type pair, and in such a case, report an error - in strict mode - statically (because the scopes are known statically); and in non-strict mode - when such ambiguous conversion pair is executed.

What do you think? cc: @JaroslavTulach @hubertp @jdunkerley

[radeusgd]

Note that the current behaviour on conversions in non-strict mode is actually not at all an outlier - the exact same behaviour can be ovserved for normal methods redefined in non strict mode, as presented by this (currently passing) test:

  @Test
  public void testAmbiguousMethodDef() {
    String src = """
       foo x = x+100
       foo x = x+200
       
       main = foo 23
       """;
    Value res = evalModule(nonStrictCtx, src);
    System.out.println(res);
    assertEquals(123, res.asInt());
  }

[radeusgd]

Just noting that, so that if we want to solve this issue through something like (1), we should handle normal methods in the same way - as there does not seem to be any reason why this aspect of handling ambiguity should differ in any way.

[JaroslavTulach]

I believe the current behavior is supporting cluelessness of our business users. Current behavior selects the first foo method. Shall we change that to yield an error?

That'd be more rationalistic. However is that what our users want? Do we really want their program to fail without computing 123? Do we really think it is better for our users?

My background is built on top of Minimizing Understanding in Constructions of Complex Systems - I am convinced that if there is a way to run the program, the program shall execute somehow.

[4e6]

We can detect overloaded methods with the OverloadsResolution compiler pass. In text mode, it results in a compilation error but in interactive mode, we still run the program and thus get an undefined behavior.

To fix the interactive case, we can replace the method body of the overloaded method with an error during the codegen. This way when the overloaded method is invoked, the user will see an error in IDE.

[JaroslavTulach]

There is a difference between non-recoverable error like invocation of non-existing method and duplicated method error. In the first case there is no other way then to fail execution in the non-strict mode. In the later case there is a way to proceed: we shall invoke something! It may be reasonable to attach a warning to result of such ambiguous invocation - but in the name of cluelessness we shall compute something, when we can compute something.

[JaroslavTulach]

@wdanilo mentioned today that ambiguity can lead to hard to diagnose errors and we shouldn't try to relax the strictError check - e.g. we should just tell the IDE the code is wrong and there is nothing to execute even in non-strict mode. I don't agree with such idea - in the name of cluelessness we should do better - but at least it is an idea which is easy to implement - duplication always mean static compiler error. Let's try it and see how bad UX that delivers.