Closed
Bug 465862
Opened 16 years ago
Closed 16 years ago
Native regexps: case-insensitive comparison bug
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
VERIFIED
FIXED
People
(Reporter: dmandelin, Assigned: dmandelin)
Details
(Keywords: verified1.9.1, Whiteboard: fixed-in-tracemonkey)
Attachments
(2 files)
617 bytes,
patch
|
gal
:
review+
|
Details | Diff | Splinter Review |
4.01 KB,
patch
|
bc
:
review+
|
Details | Diff | Splinter Review |
Due to a 1-line typo, native matchers generated with the 'i' flag perform a bogus "to-uppercase" action on non-letters. The result is that in the following code with the -j option turned on, all 4 match operations match. print("must match:"); print('@'.match(/@/i)); print('`'.match(/`/i)); print("must not match:"); print('@'.match(/`/i)); print('`'.match(/@/i));
Attachment #349092 -
Flags: review?(gal)
Comment 1•16 years ago
|
||
Comment on attachment 349092 [details] [diff] [review] Patch >diff -r d5e92772e8a1 js/src/jsregexp.cpp >+ if (L'A' <= ch && ch <= L'Z' || L'a' <= ch && ch <= L'z') { Drive-by: Please parenthesize the && expressions against the || to avoid GCC warnings.
Updated•16 years ago
|
Flags: blocking1.9.1?
Comment 2•16 years ago
|
||
Comment on attachment 349092 [details] [diff] [review] Patch See Blake's drive-by nit.
Attachment #349092 -
Flags: review?(gal) → review+
Assignee | ||
Comment 3•16 years ago
|
||
Pushed to TM as 21807:65019ca2ece8.
Assignee | ||
Comment 5•16 years ago
|
||
I think a JS regression test would be appropriate. What dir would it go in? js1_1 because regexps have been around forever, or a new regexp dir, or what? I don't know the location conventions.
Comment 6•16 years ago
|
||
ecma_3/RegExp would be fine if it doesn't use any weirdness other than turning jit on/off via jit(true)/jit(false).
Assignee | ||
Comment 7•16 years ago
|
||
Attachment #349358 -
Flags: review?
Assignee | ||
Updated•16 years ago
|
Attachment #349358 -
Flags: review? → review?(bclary)
Comment 8•16 years ago
|
||
Comment on attachment 349358 [details] [diff] [review] Regression test case note the test passes but nothing appears to be traced from what I can tell. Thanks for the test. Do you want to check it in or would you like me to? I normally check into both CVS trunk and mozilla-central.
Attachment #349358 -
Flags: review?(bclary) → review+
Assignee | ||
Comment 9•16 years ago
|
||
(In reply to comment #8) > (From update of attachment 349358 [details] [diff] [review]) > note the test passes but nothing appears to be traced from what I can tell. Correct. The regexp->native compiler shares the js option and some of the machinery with tracing but does not in fact trace. > Thanks for the test. Do you want to check it in or would you like me to? I > normally check into both CVS trunk and mozilla-central. Hmmm. I think I'll check it in to TraceMonkey for now because that's what I did with the actual fix, and let them be picked up together. But if you want to, certainly feel free to check it in to the others now. Pushed to TM as 5f3dfe048606.
Updated•16 years ago
|
Whiteboard: fixed-tm
Updated•16 years ago
|
Whiteboard: fixed-tm → [fixed-tracemonkey]
Updated•16 years ago
|
Whiteboard: [fixed-tracemonkey] → fixed-in-tracemonkey
Updated•16 years ago
|
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Comment 10•16 years ago
|
||
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/548f2e53f637
Keywords: fixed1.9.1
Comment 11•15 years ago
|
||
sync mc with cvs. /cvsroot/mozilla/js/tests/ecma_3/RegExp/regress-465862.js,v <-- regress-465862.js initial revision: 1.1
Flags: in-testsuite+
Flags: in-litmus-
Comment 12•15 years ago
|
||
v 1.9.1, 1.9.2
Status: RESOLVED → VERIFIED
Keywords: fixed1.9.1 → verified1.9.1
You need to log in
before you can comment on or make changes to this bug.
Description
•