Bug 147475 – jsc-tailcall: Add enums for type-safety

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Basile Clement]

We should have:

 - An enum to specify whether we want to keep or trash the caller frame instead of using reinterpret_cast<void*>(0) and reinterpret_cast<void*>(1)

 - An enum to indicate whether we are performing a regular call, a tail call, or a constructor call.

[Basile Clement]

Created attachment 257881 [details]
Patch

[Basile Clement]

Building to run the tests but this should solve the build failures from the merge from trunk.

[Basile Clement]

Created attachment 257882 [details]
Patch

Now it builds

[Michael Saboff]

Comment on attachment 257881 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=257881&action=review

r-
You need to add CallMode.{cpp,h} & DeferredSourceDump.{cpp,h} to Windows and cmake builds.

> Source/JavaScriptCore/bytecode/CallMode.h:31
> +enum class CallMode { Tail = 0, Regular, Construct };

Minor nit - Wouldn't it make more logical sense to order these Regular, TailCail, Construct, is there special meaning to TailCall being 0?

> Source/JavaScriptCore/bytecode/CallMode.h:33
> +enum FrameAction { KeepTheFrame = 0, TrashTheFrame };

Doesn't ReuseTheFrame sound better?

[Basile Clement]

(In reply to comment #4)
> Comment on attachment 257881 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=257881&action=review
> 
> r-
> You need to add CallMode.{cpp,h} & DeferredSourceDump.{cpp,h} to Windows and
> cmake builds.

Oh oops, I knew there was a reason I kept this locally instead of submitting a patch earlier...
(Note that there is nothing to do w/ DeferredSourceDump, it was just reordered when I did "Sort by name" in XCode).

> 
> > Source/JavaScriptCore/bytecode/CallMode.h:31
> > +enum class CallMode { Tail = 0, Regular, Construct };
> 
> Minor nit - Wouldn't it make more logical sense to order these Regular,
> TailCail, Construct, is there special meaning to TailCall being 0?

It would make sense. Tail is 0 because CallMode and FrameAction were a single enum initially, changing this.

> 
> > Source/JavaScriptCore/bytecode/CallMode.h:33
> > +enum FrameAction { KeepTheFrame = 0, TrashTheFrame };
> 
> Doesn't ReuseTheFrame sound better?

ReuseTheFrame is much nicer, changing this.

[Basile Clement]

Created attachment 257885 [details]
Patch

[Basile Clement]

Created attachment 257887 [details]
Patch

[Michael Saboff]

Comment on attachment 257887 [details]
Patch

r=me

[Basile Clement]

Landed in r187629 <http://trac.webkit.org/changeset/187629>