-
Notifications
You must be signed in to change notification settings - Fork 39
CPLAT-17467 Remove usages of .slice to work around a dart2js compiler bug #145
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
Conversation
Security InsightsNo security relevant content was detected by automated scans. Action Items
Questions or Comments? Reach out on Slack: #support-infosec. |
@greglittlefield-wf I see this failing a format step. But the same step would fail on master. 🤷 |
@tonybathgate-wk Thanks for letting me know! Just pushed a commit to format |
😬 Looks like unit tests are failing now. |
lib/src/json_schema/json_schema.dart
Outdated
@@ -607,7 +607,7 @@ class JsonSchema { | |||
// When there are 2 possible path to be resolve, traverse both paths. | |||
JsonSchema _resolveParallelPaths( | |||
Uri pathUri, // The path being resolved | |||
ListSlice<String> fragments, // A slice of fragments being traversed. | |||
List<String> fragments, // A slice of fragments being traversed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: is this comment out of date now?
lib/src/json_schema/json_schema.dart
Outdated
@@ -607,7 +607,7 @@ class JsonSchema { | |||
// When there are 2 possible path to be resolve, traverse both paths. | |||
JsonSchema _resolveParallelPaths( | |||
Uri pathUri, // The path being resolved | |||
ListSlice<String> fragments, // A slice of fragments being traversed. | |||
List<String> fragments, // A slice of fragments being traversed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: is this comment out of date now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 from RM
Ultimate problem:
For some reason, code using ListSlice sometimes triggers the following dart2js compiler bug: dart-lang/sdk#48762
The error it produces (click to expand)
How it was fixed:
Remove code using ListSlice (replace
.slice
with.sublist
, which seems to work around the issue in at least some cases.This seemed safe to do, since the slices weren't being mutated, and only read from.
Testing suggestions:
CI passes (assuming there's good test coverage around this method)
Potential areas of regression: