Skip to content

Support implicit new and const in the analyzer #31633

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

Closed
bwilkerson opened this issue Dec 13, 2017 · 7 comments
Closed

Support implicit new and const in the analyzer #31633

bwilkerson opened this issue Dec 13, 2017 · 7 comments
Assignees
Labels
legacy-area-analyzer Use area-devexp instead. P1 A high priority bug; for example, a single project is unusable or has many test failures

Comments

@bwilkerson
Copy link
Member

In order to minimize the impact of these changes on clients of analyzer, it might be better to not change the AST structure, but to have the resolver rewrite the AST when it is able to interpret the syntax based on resolution information.

@bwilkerson bwilkerson added legacy-area-analyzer Use area-devexp instead. P1 A high priority bug; for example, a single project is unusable or has many test failures labels Dec 13, 2017
@bwilkerson
Copy link
Member Author

I believe that this CL (https://dart-review.googlesource.com/c/sdk/+/31960) updates all of the code that assumed that the keyword would not be null to check for null.

The next step is to update the resolver to resolve constructors when the new/const keyword is missing and to re-write the AST as appropriate. That work will need to be behind a flag until other platforms start implementing this feature.

@bwilkerson
Copy link
Member Author

This feature should be behind the --preview-dart-2 flag (after a new --use-cfe flag has replaced the current use of that flag) until the VM supports this syntax.

@jwren jwren self-assigned this Jan 3, 2018
@devoncarew devoncarew added this to the 2.0-alpha2 milestone Jan 26, 2018
@devoncarew
Copy link
Member

@jwren, can you update this with the progress for implicit new and const? Thanks!

@jwren
Copy link
Member

jwren commented Feb 1, 2018

@bwilkerson and I are working on this right now.

The flag changes mentioned above are done, and there is now support in the analysis_options.yaml file, adding the enablePreviewDart2 bit (in a recently built Dart SDK) and reanalyzing sources will yield the current status of where we are if you want to go try it out:

analyzer:
  language:
    enablePreviewDart2 : true

For correct errors and warnings with the implicit constructors, additional logic is required to know when some constructor is const, this change should land soon: https://dart-review.googlesource.com/c/sdk/+/38140

There are 8 cases to cover: [libraryPrefix.]?Class[<type-arguments>]?[.name]?()

The cases "Class()" and "Class.constructorName()" are currently working and required changes in the resolver of the analyzer.

I am working on the cases of "libraryPrefix.Class()" and "libraryPrefix.Class.constructorName()" now, which are also changes in the resolver.

The remaining cases (all cases) should be a changes in the parser only.

My best guess is that pending other tasks, this will be done into early next week.

@dgrove
Copy link
Contributor

dgrove commented Feb 13, 2018

Any updates?

@bwilkerson
Copy link
Member Author

I am waiting for some fixes to the tests to land (https://dart-review.googlesource.com/c/sdk/+/40301) so that the tests are correct. Then I can verify that my changes related to the recent spec change are correct and working.

@devoncarew devoncarew modified the milestones: 2.0-alpha2, 2.0-beta1 Feb 15, 2018
@bwilkerson
Copy link
Member Author

I believe that this is now implemented, though I'm sure we'll find a few bugs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
legacy-area-analyzer Use area-devexp instead. 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

4 participants