Dart2JS collection compiler error #48762
Comments
devoncarew
added
the
area-web-js
|
Thanks for the bug report @danielszopa-wk We've seen this kind of error in the past, and the cause was either a bug in our rti optimization (an optimization that removes type variables if it can determine that the type variable is not used for anything meaningful) or a bug in how data from this optimization was consumed. There are a few things that would help us isolate this issue:
Thanks! FYI @fishythefish |
|
Testing with a newer version of dart2js would be great, but AFAIK all the fixes I landed for similar issues were in 2.10 or earlier, so this may be something new. It's hard to diagnose without a repro because many of the RTI optimizations are affected by the state of the whole program - things like which types all of the closures have, whether certain type tests occur anywhere in the code, etc. |
I can confirm this is correct. I do not hit the error when using that flag.
Since our codebase hasn't fully migrated to support null safety and apparently there are some breaking changes affecting analyzer and build_runner preventing us from using more recent versions of the SDK. @alanknight-wk would be able to elaborate better than I if you'd like more of an explanation.
Ah I didn't even know that function existed. It appears we use var n = lowerBound(_dtoList, dto, compare: _compareDtos);
_dtoList.insert(n, dto);The second callsite is essentially the same, just for a different object type. From testing, it appears the two callsites are both being considered by the compiler as removing both appears to result in a successful build, but leaving one results in a failure. I tried removing the callsites and attempted to reproduce this on a smaller scale but had no luck. I noticed that final n = algo.lowerBoundBy<Dto, Dto>(_dtoList, (value) => value, _compareDtos, dto);Additionally, I think it's worth calling out that we're passing the |
|
Thanks for the updates! @fishythefish for reference this is the definition of int lowerBound<E>(List<E> sortedList, E value, {int Function(E, E)? compare}) {
compare ??= defaultCompare;
return lowerBoundBy<E, E>(sortedList, identity, compare, value);
}Nothing stands out to me at the moment unforutnately. |
|
Alright one more update. I poked at this some more and I found that if I updated int lowerBound<E>(List<E> sortedList, E value, {int Function(E, E)? compare}) {
compare ??= defaultCompare;
return lowerBoundBy<E, E>(sortedList, (value) => value, compare, value);
} |
|
@sigmundch: Yup, I was playing around with some small programs using @danielszopa-wk: I wouldn't expect My initial hunch is that this is related to closures, as was the case with similar issues in the past. |
I actually found that
Here's the two implementations: static int _compareDto1s(Dto1 left, Dto1 right) {
var result = right.stringVar1.compareTo(left.stringVar1);
if (result == 0) {
result = left.stringVar2.compareTo(right.stringVar2);
}
return result;
static int _compareDto2s(Dto2 left, Dto2 right) {
return left.stringVar.compareTo(right.stringVar);
}I did think it was slightly odd that we were declaring these as
Neither calls specify the type argument on Edit: Something that's probably worth noting here is that |
|
Thanks for the additional information! I'll try playing around with this some more to see if I can distill a repro.
Sorry, what I meant was: are the two (implicit) type arguments similar types, or do they have some distinguishing features (e.g. one is generic) or major differences in how they're used? |
Yeah the two callsites are similar, no use of generics, they look like this: // Call site 1
// _dtoList1 is a List<Dto1>
// dto1 is a Dto1 object, no parent type like TBase is referenced / used.
var n = lowerBound(_dto1List, dto1, compare: _compareDto1s); // See _compareDto1s in previous comment
// Call site 2
// _dtoList2 is a List<Dto2>
// dto2 is a Dto2 object, no parent type like TBase is referenced / used.
var n = lowerBound(_dto2List, dto2, compare: _compareDto2s) // See _compareDto2s in previous comment |
|
So the fact that the breakage starts at Unfortunately, I haven't yet been able to repro the crash itself, so I'm not sure what triggers the lookup of the missing type information. |
|
@danielszopa-wk The locals which are in scope when the crash occurs look like we're setting up state for inlining. Does the problem persist if you pass |
|
@fishythefish I can confirm the build passes if I add the |
|
@danielszopa-wk Okay, great, that helps. I'll keep trying to get a repro and I may have some follow-up questions, but just a heads up that this will likely be a slow, intermittent process without direct access to an existing failure. |
|
We just hit another instance of this issue, where The full error:There only seemed to be one codepath to This specific instance only started happening when we removed code in an unrelated package (which is unfortunately private). I tried reproducing the issue in a smaller test case with the version of that package before the code was removed, but didn't have any luck. |
|
Thanks, @greglittlefield-wf, that's helpful! Notably, both |
|
@fishythefish It seems like that's still the only usage of I tried looking for other usages of I also diffed the dart2js output between a build with dependencies that failed unless I couldn't find any different calls to
|
|
Thanks, @greglittlefield-wf! Between that info and what @danielszopa-wk provided, I think I might have a rough idea of what's going wrong. What's the best way to provide you with a fix to test? |
|
@fishythefish You're welcome! Thank you for looking into it! Oh, awesome!! If there's a branch with a fix, we should be able to compile the Dart SDK locally and give it a whirl. |
|
Would just a change on Gerrit work? (I think @robbecker-wf might know how to patch this in.) https://dart-review.googlesource.com/c/sdk/+/247609 If this doesn't fix the issue, we may have to do a bit of back-and-forth |
|
That sounds perfect—we'll give that a try! |
|
We ended up not being able to compile that test case in the latest Dart SDK, but @alanknight-wk was able to apply the patch locally to Dart 2.13.4's builder_kernel.dart, and that appears to have fixed the issue!! 🎉 |
|
Wow, that's great to hear! As a software engineer, I'm already stunned when any of my code actually works, and this was a shot in the almost-dark. :P Ironically, an actual repro remains the most elusive part of this fix, but I'll try to land this ASAP once I get one. |
|
That feeling when shot in the almost-dark solutions actually work is the best 😁. Awesome, thank you! Please let us know if we can help provide more clues around how to repro the issue. |
|
@danielszopa-wk @greglittlefield-wf I'm starting to suspect that there's another latent bug related to instantiations which is making this tricky to reproduce. Could you uncomment the following code and share what it prints for a successful build and for a failing build? Ideally, this is with a minimal change to the code, like replacing sdk/pkg/compiler/lib/src/js_backend/runtime_types_resolution.dart Lines 1367 to 1387 in e157e59 Depending on the size of the application, this might dump a lot of info - hopefully it isn't too sensitive. |
|
Hmm. It is indeed a lot of data, about 1.1MB in each case. I'll have to ask about the sensitivity. However, the difference between the two is extremely slight. The passing version is created by adding the line
In the passing version, there is an extra line for that method and in the failing version at the end there is where in the passing version it still lists but then succeeds. |
|
Thanks for looking into it, @alanknight-wk. I'm going to mark this issue closed for now since it sounds like the CL fixes the issue and we don't have a repro to investigate further. There have been some minor changes to the way we handle generic instantiations since 2.13.4, so that might make it difficult to isolate a repro as well. Please feel free to reopen or file a new issue if you run into this again. |
Uh oh!
There was an error while loading. Please reload this page.
Sadly, I don't have a small case to reproduce this & the codebase is private, but I can summarize the change that resulted in this error.
This started occurring after removing the
_dtoList.removeWhereline of code below, but I found that it would continue to happen even if this_replaceDtofunction & its callsites were removed.Edit: I realize it's probably useful if I include that our
pubspec.lockis usingcollection:1.16.0.Name & Version of OS:
10.15.7FROM dart:2.13.4.Dart SDK Build Number:
2.13.4Command run:
pub global run webdev build --output build -- --delete-conflicting-outputsError:
The text was updated successfully, but these errors were encountered: