1345261 - Dead code in js::FunctionToString

Status: RESOLVED INVALID

(Core :: JavaScript Engine, enhancement)

Description

[Eric Faust [:efaust]]

Attachment: Remove dead code from js::FunctionToString

hasSource is the same as the else if, making the elseif stone dead.

Comment 1

[Steve Fink [:sfink] [:s:]]

Comment on attachment 8844654 [details] [diff] [review]
Remove dead code from js::FunctionToString

Review of attachment 8844654 [details] [diff] [review]:
-----------------------------------------------------------------

Now the only question is if [native code] looks better, or [sourceless code]. Personally, I think they both kinda suck.

But never mind me.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/efec0bd16f22
Remove dead code from js::FunctionToString. (r=sfink)

Comment 3

[Phil Ringnalda (:philor)]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/54f868ca2c74 - js/xpconnect/tests/chrome/test_discardSystemSource.xul doesn't seem to think it's dead, since https://treeherder.mozilla.org/logviewer.html#?job_id=82303315&repo=mozilla-inbound

Comment 4

[Jeff Walden [:Waldo]]

Oh, we're dumb.  |haveSource| can be overwritten between its initialization and the point of this removed code, necessitating the double-check.  Whee.

https://hg.mozilla.org/integration/mozilla-inbound/file/efec0bd16f22/js/src/jsfun.cpp#l1016

Comment 5

[Steve Fink [:sfink] [:s:]]

Gah! Sorry, totally should have noticed that. It's almost like the original author knew what he was doing...

Comment 6

[Jeff Walden [:Waldo]]

efaust ran it by me over IRL before filing the bug, so that makes 3 (count 'em) engineers who sometimes claim to know what they're doing who looked at this and thought it was perfectly cromulent.  AREN'T WE AWESOME.  :-D