Skip to content
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

Recompile host.dart #2448

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Recompile host.dart #2448

wants to merge 2 commits into from

Conversation

natebosch
Copy link
Member

Recompile with the latest dart2js.

Recompile with the latest dart2js.
Copy link

github-actions bot commented Jan 17, 2025

PR Health

Changelog Entry
Package Changed Files
package:test pkgs/test/lib/src/runner/browser/static/host.dart.js

Changes to files need to be accounted for in their respective changelogs.

This check can be disabled by tagging the PR with skip-changelog-check.

_lazyFinal($, "_v8EvalLocation", "$get$_v8EvalLocation", () => A.RegExp_RegExp("^eval at (?:\\S.*?) \\((.*)\\)(?:, .*?:\\d+:\\d+)?$", false));
_lazyFinal($, "_firefoxEvalLocation", "$get$_firefoxEvalLocation", () => A.RegExp_RegExp("(\\S+)@(\\S+) line (\\d+) >.* (Function|eval):\\d+:\\d+", false));
_lazyFinal($, "_firefoxSafariFrame", "$get$_firefoxSafariFrame", () => A.RegExp_RegExp("^(?:([^@(/]*)(?:\\(.*\\))?((?:/[^/]*)*)(?:\\(.*\\))?@)?(.*?):(\\d*)(?::(\\d*))?$", false));
_lazyFinal($, "_firefoxSafariJSFrame", "$get$_firefoxSafariJSFrame", () => A.RegExp_RegExp("^(?:([^@(/]*)(?:\\(.*\\))?((?:/[^/]*)*)(?:\\(.*\\))?@)?(.*?):(\\d*)(?::(\\d*))?$", false));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the blocking line and regex.

@natebosch
Copy link
Member Author

https://github.com/dart-lang/test/security/code-scanning/149

Inefficient regular expression

This part of the regular expression may cause exponential backtracking on strings starting with '(' and containing many repetitions of ')('.

First detected in commit on Aug 9, 2023

As far as I can tell this regex is not changing, but other content on the same line as the regex is changing.

Confusingly the error is reported with the old line number, but the regex doesn't change between the two copies.

@devoncarew @kevmoo - do you know what caused this check to start happening when we previously had this regex checked in? Is it something we can turn off temporarily?

@sigmundch - is this potentially inefficient regex a concern for the dart2js team?

@sigmundch
Copy link
Member

I'm not particularly concerned. This seems to come from https://github.com/dart-lang/tools/blob/ce3003493b86333e7652ee5bb62bc7913b2d11b7/pkgs/stack_trace/lib/src/frame.dart#L71 - this logic is used only to parse frame lines from stack traces in FF/Safari, which usually don't start with ( and usually only contain one pair of parens.

@kevmoo
Copy link
Member

kevmoo commented Jan 21, 2025

that's a HUGE drop in code size. Did we drop dart:html here?

@natebosch
Copy link
Member Author

that's a HUGE drop in code size. Did we drop dart:html here?

it does look like there is a big chunk of removed stuff that look like dart:html wrappers A.AreaElement.prototype = {...

This does not yet migrate to the new interop though. Maybe something made treeshaking work better here? Nothing that is dropped strikes me as something we would be using.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants