Implement changes to strong mode top level inference

[leafpetersen]

We have an informal spec for top level inference for strong mode here: #28218 . The proposal is ready for feedback from implementers and other team members, and subsequent implementation in the analyzer. This is the tracking bug for the analyzer implementation.

[jmesserly]

yay!!! 🎉

[kevmoo]

@leafpetersen critical for 1.22? Trying to prioritize...

[leafpetersen]

Lower priority than things that affect non-strong mode (especially syntax).

[mit-mit]

Punting to 1.23 after priority discussion

[dgrove]

Any updates on this?

[scheglov]

It's not done yet.

I'm working toward new inference - we now can test types directly / without comparing with AnalysisContext results (which are not available), added the sets of tests for inference for initializer and instance methods according to the new rules.

But this is again hinges on being able to use inferencer from linker, so the last biggest client - DDC should be migrated to using the new analysis driver first.

[dgrove]

@scheglov Is there any issue tracking moving DDC to the analysis driver? That is starting to sound scary for 1.23.

[scheglov]

I've just opened for tracking purposes.
#29118

[scheglov]

We started reporting required errors with this CL.

[kevmoo]

@scheglov so is this resolved then?

[scheglov]

No, it is implemented only partially.
Summary based linker still attempt to perform inference for cases when it is not allowed by the new rules.

[bwilkerson]

@leafpetersen I believe that we now have enough of this work done that we can ship 1.23, so I'm bumping this issue to 1.24 for the remainder of the work. Please change it back if you disagree.

[dgrove]

How's this looking for 1.24?

[dgrove]

Any updates here?

[scheglov]

I thought it was done even for 1.23, but we decided do postpone it.
@bwilkerson

[dgrove]

What's the plan here? Really happening for 1.24? @bwilkerson

[leafpetersen]

This is, as far as I know fully implemented, but we have not rolled out the full restriction yet (currently it is only marked as a hint). Moving this to 1.25. @scheglov has a CL that relaxes the restriction slightly based on feedback. As early as possible in the 1.25 cycle, we will start working on cleaning up internal code to conform to this restriction, and then flip the hints to errors.

cc @srawlins @keertip

[keertip]

@leafpetersen , we have entered cherry pick season for 1.24, and master is looking good now. So can you go ahead a create a branch now with @scheglov 's cl?

[leafpetersen]

The CL from @scheglov has been pushed to https://github.com/dart-lang/sdk/tree/toplevel-inference-test .

[matanlurey]

@leafpetersen Is there an update here?

I noticed that as of 2.0.0-dev+5, the following resolve to dynamic still:

var x = 'Hello'.codeUnits;

class X {
  var x = 'Hello'.codeUnits;
}

[leafpetersen]

Based on strong user feedback (including yours @matanlurey :) ), we are not implementing these restrictions, but are instead implementing a more general top level inference system. This more powerful system will not be implemented in the existing front end however - it will only be available in the upcoming new front end. Until the new front end comes online, top level inference will not fully match the eventual intended behavior, and will remain in a somewhat less than ideal state. In particular, is is possible that there may be inconsistencies in the inference depending on what mode of analysis is being done (e.g. IDE vs build mode). I landed a CL a while ago to try to resolve the worst of these, but I wouldn't be surprised if there are still lurking issues.

Specifically to your examples, the first one should be resolved to List<int>, and I see it resolved as such in recent bleeding edge when used in the same file. If you are seeing it resolved as dynamic, then feel free to file a bug with a repro, but note that it's unlikely this will be fixed until the new front end comes out.

The second example will be inferred correctly with the new front end, but is not supported by the old front end.

[matanlurey]

Sounds good thanks!

[anders-sandholm]

What is the status of this one?
Can we close it now?

[stereotype441]

@anders-sandholm The new front end implements inference mostly according to the new spec. (There is a long tail of small bugs but I don't think it's necessary to leave this bug open while they are addressed). So if you want this bug to be about the new front end, we can close it.

If you want this bug to be about the analyzer implementation, we should leave it open, since the analyzer implementation won't follow the new spec until analyzer is switched over to use the new front end.

[leafpetersen]

I'm not sure if this bug is serving any purpose anymore since as I understand it most of this is not planned to be implemented. Should it be closed?