Closed Bug 1215744 Opened 10 years ago Closed 10 years ago

Unnamed class expressions should not have a .name at all.

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla44
Tracking Status
firefox44 --- fixed

People

(Reporter: efaust, Assigned: efaust)

Details

Attachments

(1 file, 1 obsolete file)

Attached patch Fix (obsolete) — Splinter Review
I believe, from reading the spec, that we missed a piece in bug 1192412, which is that if the class constructor has no name, then we should not set a name at all. I cannot find a call to SetFunctionName that isn't under an "if className is not undefined" condition. This patch will do that. Modify fun_resolve to just pass on class constructors with no name. There are three steps, here: 1) Make sure that the default class constructors (which are natives, and never needed to care before) are marked as class constructors. 2) Standardize on fun->atom() == nullptr, including default constructors, which had to use the empty atom because of constraints in the emitter. 3) Teach fun_resolve to skip this case appropriately.
Attachment #8675192 - Flags: review?(arai.unmht)
Waldo pointed out that we should make sure that |static name| stuff still works. arai wrote this lovely test for just that, which I had forgotten about. Now with more updates.
Attachment #8675192 - Attachment is obsolete: true
Attachment #8675192 - Flags: review?(arai.unmht)
Attachment #8675194 - Flags: review?(arai.unmht)
Comment on attachment 8675194 [details] [diff] [review] Fix with more correct tests Review of attachment 8675194 [details] [diff] [review]: ----------------------------------------------------------------- Thank you :D ::: js/src/jsfun.cpp @@ +487,5 @@ > if (fun->hasResolvedName()) > return true; > > + if (fun->isClassConstructor()) { > + // It's impossible to have an empty named class expression. We use empty as a Comment should be wrapped to 80-chars ;)
Attachment #8675194 - Flags: review?(arai.unmht) → review+
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: