This is a regression from 1.6 that should be fixed (before 1.7a, constants weren't propagated at all in JS and the test URL worked as expected, alerting 0 not 1). Patch next. /be
Created attachment 144890 [details] [diff] [review] proposed fix js_LookupCompileTimeConstant was overloading its return value in case of error, leaving the error-as-exception pending even though EmitSwitch returned true. The main fix is to stop looking at the compile-time scope chain (static link) as soon as we find a property named atom. If that property is directly in the variable object, is readonly and permanent, and is not shadowed by a nearer property, then we can propagate the constant. Otherwise we bail with JSVAL_VOID in *vp, which tells the caller to fall through into the case true:/case false: handler. There's an unrelated fix to avoid asserting on a zero-length JS_malloc call when allocating the tableswitch compile-time jump target table. /be
Status: NEW → ASSIGNED
Priority: -- → P1
Target Milestone: --- → mozilla1.7final
Attachment #144890 - Flags: review?(shaver)
Created attachment 144891 [details] [diff] [review] diff -w version of jsemit.c part of patch Just for easier reviewing, mainly of the zero-length JS_malloc avoidance, but please review-stamp the proposed fix. /be
Comment on attachment 144890 [details] [diff] [review] proposed fix Aye, cap'n.
Attachment #144890 - Flags: review?(shaver) → review+
Comment on attachment 144890 [details] [diff] [review] proposed fix The old code in js_LookupCompileTimeConstant was embarrassing -- it failed to drop the property looked up, in addition to overzealously following the compile-time scope chain past shadowing args/vars in functions that should have hidden the outer-scope const. I am filled with shame. Anyway, this is good for 1.7final. /be
Attachment #144890 - Flags: approval1.7?
Comment on attachment 144890 [details] [diff] [review] proposed fix a=chofmann for 1.7
Attachment #144890 - Flags: approval1.7? → approval1.7+
Fix checked in. /be
Status: ASSIGNED → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
js1_5/Regress/regress-238881.js checked in.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.