I need an opinion on this, we can - back out the ECMA deprecation - mark as WONTFIX - reassign to Evangelism
Compatibility is king, perl set precedent. I see why ECMA deprecated, but it has a different purview (future specs and impls); meanwhile, in the real world, the Mozilla implementation wants to gain market share. Evangelizing may help a bit, but with frontpage spewing octal regexp char codes, it seems to me it won't win enough. I say back out the ECMA deprecation. This matters for js1.5, so for mozilla1.1 and mozilla1.0.1. /be
Created attachment 91232 [details] [diff] [review] Allow old style octal references. I switched to emitting the error only if the '-strict' option is in effect, does that seem like the right thing? Or should it be no more than a warning even in that case?
Comment on attachment 91232 [details] [diff] [review] Allow old style octal references. In general, you shouldn't test JS_HAS_STRICT_OPTION -- instead, call js_ReportCompileErrorNumber(..., JSREPORT_STRICT | JSREPORT_WARNING...) and if it returns false, someone set option.werror, so you should fail immediately (propagate the error via false or null return code). Otherwise, do the bad old thing. Here, I see why you want to preserve the if-else control flow, but I hope the strict user gets a nice strict warning (not an error, as I thought users get now given \377). Where is the error report for overlarge backref? Can it change into a strict warning and be issued iff the else clause of the if condition shown in this patch is taken? /be
Created attachment 91540 [details] [diff] [review] Revert to earlier octal handling, but with 'strict' imposing ECMA3 rules. Here's another attempt. I think I need to go with the strict flag testing because this is simply a parsing choice - either \007 is an octal value or a NUL followed by '07', \12 is an octal value or a decimal back-reference. Neither is an error - except that the back-reference may turn out to be so, later, in which case it's a real error, not a warning.
Testcase added to JS testsuite: mozilla/js/tests/ecma_3/RegExp/octal-001.js I have filed bug 158159 against Rhino for this same issue -
Created attachment 91986 [details] [diff] [review] Fixed tabs. Fixed whitespace issues, previous patch was -wu, I'm not sure which is more readable.
Another testcase for this added to JS testsuite: mozilla/js/tests/ecma_3/RegExp/octal-002.js
Rogerl, how about an up-to-date diff -u and a diff -wu (the latter for reviewing purposes)? Also, let's target this at 1.2alpha (timeless prodding works) and I'll help get it reviewed. Thanks. /be
Created attachment 97538 [details] [diff] [review] Updated patch
I'll review and checkin before the deadline tonight. /be
I checked the fix into the trunk (I tweaked a multiline && expression in an if condition to put the && consistently at the end of each line). /be
Marking Verified Fixed. Both testcases above now pass, in the debug/optimized JS shell: mozilla/js/tests/ecma_3/RegExp/octal-001.js mozilla/js/tests/ecma_3/RegExp/octal-002.js