Closed
Bug 1150855
Opened 9 years ago
Closed 9 years ago
Method definitions require curly brackets
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla41
Tracking | Status | |
---|---|---|
firefox41 | --- | fixed |
People
(Reporter: evilpie, Assigned: evilpie)
References
(Blocks 1 open bug)
Details
(Keywords: addon-compat, dev-doc-complete)
Attachments
(2 files)
2.61 KB,
patch
|
efaust
:
review+
|
Details | Diff | Splinter Review |
11.01 KB,
patch
|
jaws
:
review+
|
Details | Diff | Splinter Review |
I blame FunctionType vs FunctionSyntaxKind, we really need to unify those! This is not valid in ES6: var o = {x() 12};
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → evilpies
Assignee | ||
Comment 1•9 years ago
|
||
Attachment #8607576 -
Flags: review?(efaustbmo)
Assignee | ||
Comment 2•9 years ago
|
||
We also need to wait for https://github.com/mozilla/addon-sdk/pull/1973 to merge.
Assignee | ||
Updated•9 years ago
|
Attachment #8607578 -
Flags: review?(jaws)
Updated•9 years ago
|
Attachment #8607578 -
Flags: review?(jaws) → review+
Comment 3•9 years ago
|
||
Comment on attachment 8607576 [details] [diff] [review] Disallow method syntax without curly brackets Review of attachment 8607576 [details] [diff] [review]: ----------------------------------------------------------------- Wfm. We should address getters and setters in some way. ::: js/src/frontend/Parser.cpp @@ +2563,5 @@ > TokenKind tt; > if (!tokenStream.getToken(&tt, TokenStream::Operand)) > return false; > if (tt != TOK_LC) { > + if (funbox->isStarGenerator() || kind == Method || kind == ClassConstructor) { This should also include kind == Getter && kind == Setter, but my understanding is that this will be a lot more painful to update our code to be compliant with that. We still need to do that, though. Can we leave-open this bug, or file a followup to handle that? ::: js/src/tests/js1_8_5/reflect-parse/methodDefn.js @@ +12,5 @@ > > +// Method definitions without braces > +assertError("({ a() void 0 })", SyntaxError); > +assertError("({ a() 1 })", SyntaxError); > +assertError("({ a() false })", SyntaxError); Can we test this in classes as well?
Attachment #8607576 -
Flags: review?(efaustbmo) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/6cf58656d6fb https://hg.mozilla.org/integration/mozilla-inbound/rev/0135ea20609a
Comment 5•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/6cf58656d6fb https://hg.mozilla.org/mozilla-central/rev/0135ea20609a
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox41:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Assignee | ||
Updated•9 years ago
|
Keywords: addon-compat,
dev-doc-needed
Comment 6•8 years ago
|
||
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/Method_definitions#SpiderMonkey-specific_notes https://developer.mozilla.org/en-US/Firefox/Releases/41#JavaScript Leaving ddn open for Kohei/site-compat for now.
Comment 7•8 years ago
|
||
> Leaving ddn open for Kohei/site-compat for now. https://developer.mozilla.org/en-US/Firefox/Releases/41/Site_Compatibility#JavaScript
Keywords: dev-doc-needed → dev-doc-complete
You need to log in
before you can comment on or make changes to this bug.
Description
•