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

[Analyzer] evaluation of const expression fails when List.length is used in assertion #37503

Closed
dnfield opened this issue Jul 11, 2019 · 8 comments
Labels
area-front-end Use area-front-end for front end / CFE / kernel format related issues. closed-duplicate Closed in favor of an existing report

Comments

@dnfield
Copy link
Contributor

dnfield commented Jul 11, 2019

class ColorFilter {
  const ColorFilter.matrix(this.matrix)
      : assert(matrix != null),
        assert(matrix.length == 20);

  final List<double> matrix;
}

const ColorFilter sepia = ColorFilter.matrix(<double>[
    0.39, 0.769, 0.189, 0, 0, //
    0.349, 0.686, 0.168, 0, 0, //
    0.272, 0.534, 0.131, 0, 0, //
    0, 0, 0, 1, 0, //
  ]);

void main() {
  print(sepia.matrix); 
}

As far as I can tell this should just work (it works at runtime in the VM), but it's failing the analyzer.

/cc @stereotype441 @pq

@dnfield
Copy link
Contributor Author

dnfield commented Jul 11, 2019

Removing the assertion on matrix.length makes the analysis error go away.

@dnfield dnfield changed the title [Analyzer] evaluation of const expression fails [Analyzer] evaluation of const expression fails when List.length is used in assertion Jul 11, 2019
@dnfield
Copy link
Contributor Author

dnfield commented Jul 11, 2019

Looks like this wouldn't compile in dart2js either... At least for dartpad, it complains that the value isn't a string.

@dnfield
Copy link
Contributor Author

dnfield commented Jul 11, 2019

Error compiling to JavaScript:
main.dart:10:39:
Error: `const <double>[0.39, 0.769, 0.189, 0.0, 0.0, 0.349, 0.686, 0.168, 0.0, 0.0, 0.272, 0.534, 0.131, 0.0, 0.0, 0.0, 0.0, 0.0, 1.0, 0.0]` of type 'List<double>' is not a valid operand for a .length expression. Must be a value of type 'String'.
const ColorFilter sepia = ColorFilter.matrix(<double>[
                                      ^
Error: Compilation failed.

@eernstg
Copy link
Member

eernstg commented Jul 11, 2019

You cannot get the length of a list in a constant expression. The language specification says so here:

\item An expression of the form \code{$e$.length} is potentially constant
  if $e$ is a potentially constant expression.
  It is further constant if $e$ is a constant expression that
  evaluates to an instance of \code{String}.

So it must be rejected at compile-time. This is one of many situations where it's tempting to make Dart constant expressions just a little bit more expressive, but it is quite costly (in so many ways) so it's not very likely to happen. One issue is that you can write a class MyList extends List<String> {...} and make the length getter run arbitrary code, and still have a const constructor in MyList, and constant expressions will not run arbitrary code, so you'd need to find the right borderline between the lists where .length is OK and the lists where it isn't, and the type system wouldn't be able to prevent the latter from occurring where a List or List<T> for some T is specified.

@dnfield
Copy link
Contributor Author

dnfield commented Jul 11, 2019

In my example, isn't the <double>[] constant? Shouldn't that fall into the case where it's potentially constant and be allowed?

@dnfield
Copy link
Contributor Author

dnfield commented Jul 11, 2019

Ahh I see, if I did it for some other arbitrary type it would fail.

So is the bug that the VM compiler allows this?

@vsmenon
Copy link
Member

vsmenon commented Jul 11, 2019

I believe the bug here is that the front-end is not rejecting this at compile time.

@vsmenon vsmenon added the area-front-end Use area-front-end for front end / CFE / kernel format related issues. label Jul 11, 2019
@vsmenon
Copy link
Member

vsmenon commented Jul 11, 2019

Here's the existing bug:

#35420

Closing this in favor of that.

@vsmenon vsmenon closed this as completed Jul 11, 2019
@vsmenon vsmenon added the closed-duplicate Closed in favor of an existing report label Jul 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-front-end Use area-front-end for front end / CFE / kernel format related issues. closed-duplicate Closed in favor of an existing report
Projects
None yet
Development

No branches or pull requests

3 participants