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)

enhancement
Not set
normal

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.
Attachment #8895595 - Flags: review?(jorendorff)
I think this may not work on the shimmed in browser JS reftest. I need to check that.
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+
(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
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: