Finalize yield syntax to match ES4/JS2 proposal

VERIFIED FIXED in mozilla1.8.1

Status

()

Core
JavaScript Engine
P1
normal
VERIFIED FIXED
12 years ago
12 years ago

People

(Reporter: brendan, Assigned: brendan)

Tracking

({verified1.8.1})

Trunk
mozilla1.8.1
verified1.8.1
Points:
---
Bug Flags:
blocking1.8.1 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Assignee)

Description

12 years ago
This tracks Python 2.5.  Currently we allow

  function f() { yield g(yield 1, 2) }

but the inner yield must be parenthesized, per PEP 342, Python 2.5, and the ES4 / JS2 proposal.  Trivial patch next.

/be
(Assignee)

Comment 1

12 years ago
Created attachment 236932 [details] [diff] [review]
fix
Attachment #236932 - Flags: review?(mrbkap)
(Assignee)

Comment 2

12 years ago
This is important to get right in js1.7/mozilla1.8.1/firefox2 -- we don't want to be hobbled with a compatibility requirement down the road based on js1.7 supported unparenthesized yield expressions as actual arguments.

/be
Status: NEW → ASSIGNED
Flags: blocking1.8.1?
Priority: -- → P1
Target Milestone: --- → mozilla1.8.1

Updated

12 years ago
Attachment #236932 - Flags: review?(mrbkap) → review+
Flags: blocking1.8.1? → blocking1.8.1+
(Assignee)

Comment 3

12 years ago
Comment on attachment 236932 [details] [diff] [review]
fix

This is safe, it merely adds a more specific error, registerizes PN_LAST(pn) in pn2 in the existing bad-yield-syntax case, and adds identical error test-and-report code to the ArgumentList parser.

/be
Attachment #236932 - Flags: approval1.8.1?
(Assignee)

Comment 4

12 years ago
Fixed on trunk.

/be
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
(Assignee)

Updated

12 years ago
Attachment #236932 - Flags: approval1.8.1?
(Assignee)

Comment 5

12 years ago
Created attachment 237043 [details] [diff] [review]
fix for 1.8 branch
Attachment #237043 - Flags: review+
Attachment #237043 - Flags: approval1.8.1?

Comment 6

12 years ago
Checking in regress-351514.js;
/cvsroot/mozilla/js/tests/js1_7/geniter/regress-351514.js,v  <--  regress-351514.js
initial revision: 1.1
Flags: in-testsuite+
Comment on attachment 237043 [details] [diff] [review]
fix for 1.8 branch

a=beltzner on behalf of 181drivers
Attachment #237043 - Flags: approval1.8.1? → approval1.8.1+
(Assignee)

Comment 8

12 years ago
Fixed on the 1.8 branch.

/be
Keywords: fixed1.8.1

Comment 9

12 years ago
verified fixed 1.9a1_2006090707 windows/mac*/linux
Status: RESOLVED → VERIFIED

Comment 10

12 years ago
verified fixed 1.8 1.9 20060909 windows/mac*/linux
Keywords: fixed1.8.1 → verified1.8.1
(Assignee)

Comment 11

12 years ago
Created attachment 238592 [details] [diff] [review]
fix js1_7/geniter/regress-345855.js
Attachment #238592 - Flags: review?(bclary)

Comment 12

12 years ago
Comment on attachment 238592 [details] [diff] [review]
fix js1_7/geniter/regress-345855.js

print will try to print to a real printer if this is run in the browser. writeLineToLog() is preferred in non-e4x tests although we _could_ do what e4x does and redefine print.

These new values are the correct, non-error producing values? Would the old values cause errors?
Attachment #238592 - Flags: review?(bclary) → review-
(Assignee)

Comment 13

12 years ago
Sorry, forgot I added those prints -- it was impossible to tell what was failing before I did that.  Maybe the actual and expected values could encode something about the specific test?  Anyway, just passing through here.

The upshot is that except when used as a statement, and when used on the right hand side of assignment, yield expressions must be parenthesized.  Previously accepted under-parenthesized yield (specifically as a single actual parameter to a function, foo(yield bar) e.g.) is not allowed now.

/be

Comment 14

12 years ago
Created attachment 238595 [details] [diff] [review]
fixier js1_7/geniter/regress-345855.js

The numeric labels were meant to indicate which section failed but I went a head and changed them. This also adds the syntax errors.
Attachment #238592 - Attachment is obsolete: true
Attachment #238595 - Flags: review?(brendan)
(Assignee)

Comment 15

12 years ago
Comment on attachment 238595 [details] [diff] [review]
fixier js1_7/geniter/regress-345855.js

Thanks, works for me.

/be
Attachment #238595 - Flags: review?(brendan) → review+

Comment 16

12 years ago
Checking in regress-345855.js;
/cvsroot/mozilla/js/tests/js1_7/geniter/regress-345855.js,v  <--  regress-345855.js
new revision: 1.2; previous revision: 1.1
done

You need to log in before you can comment on or make changes to this bug.