Closed
Bug 1273639
Opened 4 years ago
Closed 4 years ago
Add a nonunified spidermonkey build
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Not set
Tracking
()
RESOLVED
FIXED
mozilla49
Tracking | Status | |
---|---|---|
firefox49 | --- | fixed |
People
(Reporter: sfink, Assigned: sfink)
References
(Blocks 2 open bugs)
Details
Attachments
(2 files)
4.63 KB,
patch
|
terrence
:
review+
|
Details | Diff | Splinter Review |
13.97 KB,
patch
|
terrence
:
review+
|
Details | Diff | Splinter Review |
A fair number of people are affected by unified breakage within spidermonkey, so RyanVM suggested that it'd be worth having a build to keep it from breaking. I agree.
Assignee | ||
Comment 1•4 years ago
|
||
You're a fine reviewer for the code part of this, not necessarily for the adding an additional (lightweight) job's worth of load part, but this is such a small thing that it seems worth going the forgiveness route. (Plus, I marked the blocking metabug, so the right people will see it.) Note to readers: the build is currently broken right now, so I will coordinate with the sheriffs about turning it on and its visibility tier.
Attachment #8753515 -
Flags: review?(terrence)
Comment 2•4 years ago
|
||
Comment on attachment 8753515 [details] [diff] [review] Add a nonunified spidermonkey build Review of attachment 8753515 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for whipping this up! ::: js/src/devtools/automation/autospider.sh @@ +56,5 @@ > fi > > +if [[ "$VARIANT" = "nonunified" ]]; then > + # Hack the moz.build files to turn off unified compilation. > + find "$SOURCE/js/src" -name moz.build -exec perl -i.autospider -pe 's/UNIFIED_SOURCES/SOURCES/g' '{}' ';' Is perl required by our build still? I'd hate to add more dependencies on it when sed would work just as well. How about: find ... -name moz.build -exec sed -i 's/UNIFIED_SOURCES/SOURCES/' '{}' ';'
Attachment #8753515 -
Flags: review?(terrence) → review+
Comment 3•4 years ago
|
||
(In reply to Steve Fink [:sfink] [:s:] from comment #0) Thanks for doing this!
Assignee | ||
Comment 4•4 years ago
|
||
For some reason, TH is not showing the result status of these builds, even though TC knows and displays it properly. I'm waiting to land until that is figured out.
Assignee | ||
Comment 7•4 years ago
|
||
Attachment #8754119 -
Flags: review?(terrence)
Comment 8•4 years ago
|
||
Comment on attachment 8754119 [details] [diff] [review] Fix nonunified spidermonkey builds Review of attachment 8754119 [details] [diff] [review]: ----------------------------------------------------------------- Thanks! ::: js/src/gdb/tests/test-unwind.cpp @@ +9,2 @@ > { > + JS::CallArgs args = CallArgsFromVp(argc, vp); CallArgs needs a namespace but CallArgsFromVp doesn't?
Attachment #8754119 -
Flags: review?(terrence) → review+
Comment 9•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/eab16f90bea7 https://hg.mozilla.org/mozilla-central/rev/efa156b65838
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
status-firefox49:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Assignee | ||
Comment 10•4 years ago
|
||
(In reply to Terrence Cole [:terrence] from comment #8) > Comment on attachment 8754119 [details] [diff] [review] > Fix nonunified spidermonkey builds > > Review of attachment 8754119 [details] [diff] [review]: > ----------------------------------------------------------------- > > Thanks! > > ::: js/src/gdb/tests/test-unwind.cpp > @@ +9,2 @@ > > { > > + JS::CallArgs args = CallArgsFromVp(argc, vp); > > CallArgs needs a namespace but CallArgsFromVp doesn't? Of course not! Any idiot can see that! (I just spent 20 minutes minimizing the preprocessed file to figure it out because it intrigued me. It seemed to make no sense at all.) CallArgsFromVp doesn't need qualification here because it has a parameter in the JS namespace. And parameter namespaces are searched for function calls. The only reason I can come up with for that to not be utter lunacy would be if it's needed for operator overloading, so that something like this can work: namespace VectorStuff { struct Vector { int x; int y; }; bool operator==(const Vector& v1, const Vector& v2) { return v1.x == v2.x && v1.y == v2.y; } } VectorStuff::Vector v; if (v == v) { ... } but that's just speculation on my part; I haven't googled it or looked up the spec.
Comment 11•4 years ago
|
||
(In reply to Steve Fink [:sfink] [:s:] from comment #10) Looks like your speculation is correct: https://en.wikipedia.org/wiki/Argument-dependent_name_lookup
Comment 13•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/f0c0e9e19a08
You need to log in
before you can comment on or make changes to this bug.
Description
•