Closed
Bug 1388191
Opened 7 years ago
Closed 7 years ago
Add way to test evaluation with envChain in the shell
Categories
(Core :: JavaScript Engine, enhancement)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla57
Tracking | Status | |
---|---|---|
firefox57 | --- | fixed |
People
(Reporter: mccr8, Assigned: mccr8)
References
Details
Attachments
(1 file)
While working on bug 1186409, I had a bug, which was entirely in code in Interpreter.cpp, that was caught incidentally only by a single XPConnect browser test, test_bug993423.html. It would be nice if we could write shell functions that have objects on the env chain, like we use in the browser for event handlers and other things. Shu suggested adding a new option to the shell function evaluate().
I've written one, and used this to add a shell test for the issue I was hitting in my JSM sharing code.
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8895595 -
Flags: review?(jorendorff)
Assignee | ||
Comment 2•7 years ago
|
||
I think this may not work on the shimmed in browser JS reftest. I need to check that.
Comment hidden (mozreview-request) |
Comment 4•7 years ago
|
||
mozreview-review |
Comment on attachment 8895595 [details]
Bug 1388191 - Add way to test evaluation with envChain in the shell.
https://reviewboard.mozilla.org/r/166806/#review177098
::: js/src/shell/js.cpp:1671
(Diff revision 2)
> } else {
> mozilla::Range<const char16_t> chars = codeChars.twoByteRange();
> + if (envChain.length() == 0) {
> - (void) JS::Compile(cx, options, chars.begin().get(), chars.length(), &script);
> + (void) JS::Compile(cx, options, chars.begin().get(), chars.length(), &script);
> + } else {
> + (void) JS::CompileForNonSyntacticScope(cx, options, chars.begin().get(), chars.length(), &script);
Is there some reason JS::CompileForNonSyntacticScope is a separate API and not a boolean field in CompileOptions? Please consider changing it and deleting ~40 lines of redundant code. :-P
(While you're at it, we surely do not need four JS::CompileOptions classes...)
Attachment #8895595 -
Flags: review?(jorendorff) → review+
Assignee | ||
Comment 5•7 years ago
|
||
(In reply to Jason Orendorff [:jorendorff] from comment #4)
> Is there some reason JS::CompileForNonSyntacticScope is a separate API and
> not a boolean field in CompileOptions? Please consider changing it and
> deleting ~40 lines of redundant code. :-P
Yeah, it ends up being pretty verbose when you try to use this API. I filed bug 1393270 about merging them.
Pushed by amccreight@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/40aafceb5e48
Add way to test evaluation with envChain in the shell. r=jorendorff
Comment 7•7 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in
before you can comment on or make changes to this bug.
Description
•