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

Enum mixin not used as runtime Type #51730

Closed
felipeuematsu opened this issue Mar 14, 2023 · 9 comments
Closed

Enum mixin not used as runtime Type #51730

felipeuematsu opened this issue Mar 14, 2023 · 9 comments
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P2 A bug or feature request we're likely to work on

Comments

@felipeuematsu
Copy link

felipeuematsu commented Mar 14, 2023

When we have an Enum class that implements a mixin, the analyzer will accept any uses of that enum as it would any class, but when tried to run the code, the compiler will complain something along the lines of

Error: A value of type 'Enum?' can't be assigned to a variable of type 'LinkEnum?'.

Even if the variable was originally of LinkEnum,

Casting the variable to LinkEnum works, but raises an unnecessary_cast warning.

Example code below:

mixin LinkEnum on Enum {
  String get link;
}

enum Websites implements LinkEnum {
  google('google.com')
  ;
  const Websites(this.link);
  final String link;
}

Dart SDK version: 2.18.4 (stable) (Tue Nov 1 15:15:07 2022 +0000) on "macos_arm64"

@lrhn
Copy link
Member

lrhn commented Mar 14, 2023

Which compiler gives the error? (The error itself seems correct, a value of type Enum? really cannot be assigned to LinkEnum?, so the question is whether those types are correct. Also Enum? is probably a static type, since no object has runtime type Enum? or even Enum.)

Can you show the code which causes the error? My own attempts on dartpad.dev shows no problem.

For example LinkEnum? e = Websites.google; works.

(I'd use abstract class LinkEnum { String get link; } instead of a mixin if you only use it for the interface. If you also use it as a mixin, it should obviously be a mixin.)

@lrhn lrhn added the needs-info We need additional information from the issue author (auto-closed after 14 days if no response) label Mar 14, 2023
@lrhn lrhn closed this as completed Mar 17, 2023
@felipeuematsu
Copy link
Author

felipeuematsu commented Mar 17, 2023

@lrhn
Sorry for the late response

Here's the example:

import 'dart:math';

mixin LinkEnum on Enum {
  String get link;
}

enum GitWebsites implements LinkEnum {
  google('bitbucket.org'),
  github('github.com');

  const GitWebsites(this.link);
  final String link;
}

enum SearchWebsites implements LinkEnum {
  google('google.com'),
  github('bing.com');

  const SearchWebsites(this.link);
  final String link;
}

void main() {
  final randomNumberGenerator = Random();
  final condition = randomNumberGenerator.nextBool();

  final link = (condition ? GitWebsites.values : SearchWebsites.values)
      .map((e) => e.link);
  final allWebsites = [...GitWebsites.values, ...SearchWebsites.values];

  final allLinks = allWebsites.map((e) => e.link);
}

Edited with a more complete example

@github-actions github-actions bot removed the needs-info We need additional information from the issue author (auto-closed after 14 days if no response) label Mar 17, 2023
@github-actions github-actions bot reopened this Mar 17, 2023
@lrhn
Copy link
Member

lrhn commented Mar 17, 2023

That does look like an inconsistency between analyzer and front-end.

A simpler example:

void main(List<String> args) {
  var rnd = DateTime.now().millisecondsSinceEpoch.isEven;
  // LUB of `E1`, `E2`.
  //
  // Type hierarchy used by analyzer/front-end
  //                             
  //       Object        or             Object
  //         |                            |
  //       Enum                          Enum
  //         |                           /  \
  //        Inf                      _Enum  Inf
  //         |                           \  /
  //       E1, E2                       E1, E2
  //
  var o = rnd ? E1.e1 : E2.e2;
  print(o.staticType);
  o.expectStaticType<Exactly<Enum>>();  // Successful on front-end
  // o.expectStaticType<Exactly<Inf>>();  // Successful on analyzer
}

abstract class Inf implements Enum {
  int get i;
}

enum E1 implements Inf {
  e1;
  int get i => 1;
}

enum E2 implements Inf {
  e2;
  int get i => 2;
}

// Check static type.
extension <T> on T {
  Type get staticType => T;
  T expectStaticType<R extends Exactly<T>>() {
    return this;
  }
}
typedef Exactly<T> = T Function(T);

This is actually correct and expected behavior for our LUB (least upper bound) operation when presented with two different type hierarchies. And why our LUB is sometimes completely unpredictable.

If I remove the implements Enum from Inf, the analyzer decides the LUB of E1 and E2 is Object, front-end uses _Enum. Which would again be correct for both.

So, the issue is that the analyzer and the front-end are working on different type hierarchies, because the analyzer doesn't include the internal _Enum class used to implement Enum.

I'm not sure what to do.

Pretending _Enum doesn't exist would be nice, because nobody should ever need to know, but that would require redefining LUB.
Making the analyzer know about the internal implementation class will give consistent behavior, but still not behavior we can actually explain to users without admitting _Enum exists.

@lrhn lrhn added the area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. label Mar 17, 2023
@srawlins
Copy link
Member

It sounds like this is a language issue? Analyzer team should not take action until a language decision is made.

@lrhn
Copy link
Member

lrhn commented Mar 19, 2023

This is a consistency issue.
Every tool is working according to, and within, specification, and the specification is working as intended.
The tools just don't agree on which classes exist. I'd consider that primarily an analyzer issue, if the compilers agree.

That doesn't mean that we can't choose to make the compilers work differently. Currently there is an _Enum class that is used to implement enum classes. We could choose to do away with that.

But currently it's there, and is a superclass of enum declared classes, and the analyzer is not correctly modelling that.

(It does mean that changing whether we have the _Enum internal implementation helper class affects type inference and can be breaking. And that's a horrible position to be in, but until we fix LUB to be "reasonable", it's the position we get into by default.)

@srawlins srawlins added the P2 A bug or feature request we're likely to work on label Mar 20, 2023
@eernstg
Copy link
Member

eernstg commented Apr 11, 2023

Currently there is an _Enum class that is used to implement enum classes. We could choose to do away with that.

Why don't we?

@lrhn
Copy link
Member

lrhn commented Apr 12, 2023

Why don't we?

It was never an issue before, and it provided an easy way to introduce the name functionality without changing anything in the visible Enum class.

And it shouldn't be an issue that a class has a private superclass somewhere that nobody can directly access.
The fact that it does make any difference at all suggests that we have a different issue that should be solved.
In particular, if we could make the class invisible to the UP algorithm (and DOWN too, probably), there would probably not be any issue at all, it wouldn't matter whether the class exists or not (dart-lang/language#2994).

@srawlins
Copy link
Member

@eernstg I think you just closed an issue similar to this that was based on a language decision. Is this now resolved as well?

@eernstg
Copy link
Member

eernstg commented Apr 29, 2024

Good catch, @srawlins!

The language team has decided that the analyzer should take _Enum into account such that the superinterface graph according to the analyzer and according to the CFE will be identical. The discrepancy will then go away.

The specification update occurs in dart-lang/language#3671, and it will be implemented soon.

@eernstg eernstg closed this as completed Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P2 A bug or feature request we're likely to work on
Projects
None yet
Development

No branches or pull requests

4 participants