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

Dart 3: Analyzer wants const, compiler doesn't like it #51800

Closed
goderbauer opened this issue Mar 20, 2023 · 13 comments
Closed

Dart 3: Analyzer wants const, compiler doesn't like it #51800

goderbauer opened this issue Mar 20, 2023 · 13 comments
Labels
analyzer-constants area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P1 A high priority bug; for example, a single project is unusable or has many test failures
Milestone

Comments

@goderbauer
Copy link
Contributor

goderbauer commented Mar 20, 2023

With the prefer_const_constructors lint is turned on, the analyzer complains about the following code when running with sdk: '>=3.0.0-0 <4.0.0' as pubspec sdk constraints:

class ExpansionTile {
  const ExpansionTile({
    this.expandedCrossAxisAlignment,
  }) : assert(expandedCrossAxisAlignment != CrossAxisAlignment.baseline);

  final CrossAxisAlignment? expandedCrossAxisAlignment;
}

enum CrossAxisAlignment {
  start,
  end,
  center,
  stretch,
  baseline,
}

void main() {
  print(ExpansionTile(expandedCrossAxisAlignment: CrossAxisAlignment.start)); // 🔥 prefer_const_constructors lint warning
}

However, when giving in to that lint and using const, the code doesn't run anymore:

$ dart pure_dart.dart
lib/pure_dart.dart:21:15: Error: Constant evaluation error:
  print(const ExpansionTile(expandedCrossAxisAlignment: CrossAxisAlignment.start));
              ^
lib/pure_dart.dart:7:42: Context: Binary operator '==' requires receiver constant 'CrossAxisAlignment {}' of type 'Null', 'bool', 'int', 'double', or 'String', but was of type 'CrossAxisAlignment'.
 - 'CrossAxisAlignment' is from 'package:pure_dart/pure_dart.dart' ('lib/pure_dart.dart').
  }) : assert(expandedCrossAxisAlignment != CrossAxisAlignment.baseline);
                                         ^

When lowering the sdk constraints to 2.19, the analyzer does not complain that a const constructor should be used here.

@goderbauer
Copy link
Contributor Author

cc @jakemac53 @scheglov

@pq pq added P1 A high priority bug; for example, a single project is unusable or has many test failures area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. analyzer-constants labels Mar 20, 2023
@pq pq added this to the Dart 3 beta 3 milestone Mar 20, 2023
@pq
Copy link
Member

pq commented Mar 20, 2023

Possibly related: #51567

@eernstg
Copy link
Member

eernstg commented Mar 21, 2023

Yes, this behavior is consistent with the status of #51566: Constant == expressions based on primitive equality has not yet been implemented; old constant == did not admit enum values, it just had a short whitelist of platform types, and this usage needs the generalized constant ==.

@goderbauer
Copy link
Contributor Author

Is generalized constant == gonna land in the next few days for Dart 3? If not, can the analyzer stop recommending const where it doesn’t work (yet) until generalized constant == lands?

@scheglov
Copy link
Contributor

I don't know how to handle this, when one part of Dart implements the specification, and another is not.
The analyzer is not wrong, I think, so I don't feel like removing these changes.
One solution is to disable prefer_const_constructors for now.

@goderbauer
Copy link
Contributor Author

I also don't know what the right path is here. However, once users are migrating to Dart 3 it's gonna be a very odd (maybe even frustrating) user experience when users are told by one system to make these const only to be yelled at by another system for doing so...

cc @itsjustkevin

@bwilkerson
Copy link
Member

I added the milestone to #51566 to raise visibility of the need to have it in.

@scheglov
Copy link
Contributor

FYI, there are at least two other issues for this problems, in this milestone.
#51688
#51566

@itsjustkevin
Copy link
Contributor

itsjustkevin commented Mar 21, 2023

I also don't know what the right path is here. However, once users are migrating to Dart 3 it's gonna be a very odd (maybe even frustrating) user experience when users are told by one system to make these const only to be yelled at by another system for doing so...

This seems like a pretty significant user experience issue. @pq I am still learning how Dart handles priorities, but shouldn't this be a P0?

@johnniwinther
Copy link
Member

I'll look into supporting this in the CFE:

@itsjustkevin
Copy link
Contributor

@leafpetersen do we have clarity around what behavior we actually want here?

@johnniwinther
Copy link
Member

CL in the pipeline: https://dart-review.googlesource.com/c/sdk/+/290320

@eernstg
Copy link
Member

eernstg commented Mar 22, 2023

what behavior we actually want here?

The language team decided a while back to use primitive equality for constant expressions of the form e1 == e2 (and e1 != e2), and decided on March 1 2023 that this rule should be extended a bit to allow double operands as well (double does not have primitive equality, but it was an unintended breaking change to make expressions like 1.5 == 2.5 non-constant, so we fixed that). Every enum has primitive equality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer-constants area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P1 A high priority bug; for example, a single project is unusable or has many test failures
Projects
None yet
Development

No branches or pull requests

7 participants