Mixin type inference tries to do a subtype check with an incomplete class hierarchy #32353
Assignees
Labels
area-front-end
Use area-front-end for front end / CFE / kernel format related issues.
P1
A high priority bug; for example, a single project is unusable or has many test failures
Comments
stereotype441
added
area-analyzer
kmillikin
added
the
P1
|
Analyzer fix is out for review: https://dart-review.googlesource.com/c/sdk/+/44220 |
stereotype441
removed
the
area-analyzer
dart-bot
pushed a commit
that referenced
this issue
Mar 1, 2018
This should be safe since the purpose of the inference is to find an exact match for each type parameter; if an exact match is found but it doesn't satisfy the "extends" constraint, that will be detected and reported later. If an exact match isn't found, the type bound will be filled in my instantiate-to-bounds. Addresses the analyzer portion of issue #32353. Change-Id: Ic9f71eefac2fa3f6f126957b9d1652ca1d990b89 Reviewed-on: https://dart-review.googlesource.com/44220 Commit-Queue: Paul Berry <paulberry@google.com> Reviewed-by: Brian Wilkerson <brianwilkerson@google.com> Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
|
Appears to be fixed by change 45021. |
|
This problem doesn't reproduce in 9f56e28. |
peter-ahe-google
removed
the
P1
|
@stereotype441's analysis of the problem above doesn't match my understanding of how the code worked before 9f56e28, so I'd like to keep this open and see if I can reproduce this problem. |
|
I don't know if @stereotype441's analysis is right or not. The mixin inference in Kernel's class hierarchy analysis has a more fundamental problem which we have to fix in any case. CHA has two traversals of the class hierarchy and it can only safely answer subtype queries after the second pass, when it has built the intervals necessary for those queries. Before that, it will just crash. The mixin inference uses the constraint generator from type inference as an off-the-shelf algorithm during the first pass of CHA, and that constraint generator can ask subtype queries which will crash. We will have to either use a different implementation of constraint generation or else use a different implementation of subtype queries. @stefantsov will fix the crash. |
|
I have a repro that crashes again, should we restore P1 status? |
|
I'm not sure if my repro is a legal program. I think this is: |
kmillikin
added
the
P1
|
I'm not sure if my program is legal. I don't think it is because of the use of _LocalDirectory as a raw type in: |
|
I've simplified the repro code, so that the only raw type is the mixed in interface type: class A {}
class B<X, Y> {}
class C<X> extends B<X, A> {}
class D<X, Y> extends B<X, Y> with C {}Attempting to compile this code results in a compiler crash with the following stack trace: If the type argument to the mixed in Error: 'D' can't implement both '#lib1::B<#lib1::D::X, #lib1::D::Y>' and '#lib1::B<#lib1::D::X, #lib1::A>'
class D<X, Y> extends B<X, Y> with C<X> {}
^ |
|
Nice repro. I suggest that you update tests/language_2/bug32353_test.dart with your smaller repro. The original repro relies on dart:io which can cause problems:
|
copybara-service bot
pushed a commit
that referenced
this issue
Mar 15, 2022
This logic allowed the analyzer to omit consideration of type parameter bounds during execution of the `matchSupertypeConstraints` method (which is used for mixin type parameter inference). I added it back in 2018 (https://dart-review.googlesource.com/c/sdk/+/44220) in an attempt to fix #32353, however after I made the fix, additional work by the front end team made it seem like I had probably misunderstood the issue. In any case, I've verified with both SDK trybots and an internal presubmit that removing this logic doesn't break anything, so I believe it's likely that the bug was later fixed in a different (and presumably more correct) fashion. I'm currently doing work on the analyzer's type inference logic, and GenericInferrer.considerExtendsClause is complicating my efforts, so it seems reasonable to remove it at this point. Change-Id: Ia0a988c7297e1579c2e96f9fa886e280f47ab62e Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/237003 Reviewed-by: Konstantin Shcheglov <scheglov@google.com> Commit-Queue: Paul Berry <paulberry@google.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Consider the following code (adapted from https://github.com/google/file.dart):
When trying to infer the type parameters for
_LocalDirectory's use of the mixinForwardingDirectory, here's what happens:ForwardingFileSystemEntity<T, io.Directory>. Call thisA._LocalFileSystemEntity<_LocalDirectory, io.Directory>), and find that it is a subclass ofForwardingFileSystemEntity<_LocalDirectory, io.Directory>. Call thisB.Tsuch thatA = B. We do this by gathering the constraints implied byA <: BandB <: A.T <: _LocalDirectory(fromA <: B)_LocalDirectory <: T(fromB <: A)T <: Directory(from the definition ofT, which isT extends Directory)T <: GLB(_LocalDirectory, Directory).GLB(_LocalDirectory, Directory)should be_LocalDirectory, however the GLB logic can’t tell that it is, because the class hierarchy hasn’t been fully built. In particular,_LocalDirectory’s subclassing ofDirectoryis through its other mixin (DirectoryAddOnsMixin), which hasn’t been processed yet.The analyzer mixin type inference algorithm fails at this point, saying that it can't infer a type. The front end mixin type inference algorithm crashes.
The text was updated successfully, but these errors were encountered: