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

conflicting static extensions #9681

Closed
vonagam opened this issue Jul 2, 2020 · 0 comments · Fixed by #9740
Closed

conflicting static extensions #9681

vonagam opened this issue Jul 2, 2020 · 0 comments · Fixed by #9740
Milestone

Comments

@vonagam
Copy link
Member

vonagam commented Jul 2, 2020

Example:

using Main.FooTools;
using Main.IntTools;

abstract Foo(Int) from Int to Int {}

private class FooTools {
  public static function ext(that: Foo) trace('Foo');
}

private class IntTools {
  public static function ext(that: Int) trace('Int');
}

class Main {
  public static function main() {
    var foo: Foo = 0;
    foo.ext(); // traces Int, not Foo
  }
}

It is impossible right now to make FooTools work with Foo and IntTools with Int.

There are two types of static extensions:

Module scoped static extensions that are used with using on top of a module. Pros and cons:

  • + have high and flexible priority.
  • + have only one filter for a target - unifiability with first argument, thus implicitly match all applicable types at once.
  • + add an ability to boost third-party types.
  • + are scoped to importing modules, so you can use different tools in different places.
  • - because they implicitly apply to many types can create unresolvable conflicts.
  • - need to be repeated in every relevant module (questionable minus, imports.hx can help with that too).

Type scoped static extensions that are used with @:using metadata on a relevant type. Have opposite traits:

  • - have low and fixed priority.
  • - work only for specifically marked types (might require an explicit cast for usage because of that).
  • - can't be used for third-party types (there is --macro addMetadata, but it doesn't seem to help).
  • + are bound to a type so don't create unexpected conflicts.
  • + are applied once, no repetition needed.

The example above is a blind spot that can't be covered by those two types as they are now:

  • IntTools can't be converted to a type scoped extension since Int is a std type defined outside.
  • FooTools and IntTools do not have type scopes and because of implicit casts match the same types, thus creating the conflict.

As an example from real world can be used Safety extension RealyUniqueName/Safety, since it uses Null<T> type it unifies with anything (except dynamic) and makes method names such as apply, run, check unusable for other extensions. Ideally it should apply only to Null abstract type (and those who forward to it).

I see three possible non-breaking improvements that will allow to resolve the issue:

  1. Allow to apply @:using on typedefs. With that the above example can be solved like this with only type scoped extensions:
@:using(main.FooTools) abstract Foo(Int) from Int to Int {}
@:using(main.IntTools) typedef MyInt = Int;
  1. Allow to mark a module extension as one which uses a type equality check, not unification one, on a first argument, so no implicit casts. As a solution to the example:
using Main.IntTools strictly;
using Main.FooTools strictly;
  1. The last addition is a modifier which adds type scopes to a module extension. Here is how it can look:
using Main.IntTools for Int;
using Main.FooTools for Foo;

Related:
#5924 last comment is the same problem as in this example (but there it can be fixed by converting to @:using).
#8188 is related to @:using on typedefs.

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

Successfully merging a pull request may close this issue.

2 participants