Closed Bug 602695 Opened 9 years ago Closed 9 years ago

TM: [meta] clean up and break up jstracer.cpp

Categories

(Core :: JavaScript Engine, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED WONTFIX

People

(Reporter: njn, Assigned: njn)

References

Details

jstracer.cpp is huge (16,513 lines as of revision 54966:7fc2209ef579) and rather ugly.  It should be beautified (but not beatified) and split into multiple files.

The multiple files should probably go in a sub-directory, like JM is in methodjit/.  The obvious name for the sub-directory is tracejit/, but that shares a prefix with trace-tests/, and trace-tests/ comes up first in Vim's name completion so that'll annoy me.  Maybe trace-tests could be renamed tracetests/?

This is a meta-bug.
trace-tests should be renamed, but to something not starting with t! It contains jaeger tests too. I'd go with daves-tests for fun but someone may have a better idea :-P.

/be
Depends on: 602703
Depends on: 576186
(In reply to comment #1)
> trace-tests should be renamed, but to something not starting with t! It
> contains jaeger tests too. I'd go with daves-tests for fun but someone may have
> a better idea :-P.

I also wanted to combine the harnesses for trace-tests and jstests someday, since there is a lot of overlap in the basic features required. It seems nice to have one directory for JS testing stuff.

We could move the tests back to js/tests (although I know we moved them away from there a while back). Or else we could rename to 'jstests'? 'smtests' for SpiderMonkey?
Depends on: 600734
(In reply to comment #2)
> 
> I also wanted to combine the harnesses for trace-tests and jstests someday,
> since there is a lot of overlap in the basic features required. It seems nice
> to have one directory for JS testing stuff.

That would have the added benefit of fixing bug 530953 :)
Depends on: 605374
Depends on: 605700
I suspect that jstracer.cpp is not going to be broken up so much as replaced some time in the future -- in the world where we have JM, TM's needs/uses are quite different, and it's likely to be overhauled.

So I'm going to mark this bug as WONTFIX, not because it'll never happen per se, but because this bug isn't really helping anyone in its current form.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.