Closed Bug 1137523 Opened 5 years ago Closed 5 years ago

Unprefix most js_* functions

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla39
Tracking Status
firefox39 --- fixed

People

(Reporter: jandem, Assigned: jandem)

References

Details

Attachments

(2 files)

Attached patch PatchSplinter Review
This patch moves all js_* functions inside the 'js' namespace and removes the js_* prefix. There were a lot of them, here are some examples:

* js_Report{OverRecursed,OutOfMemory,...}
* js_Array/js_String/js_Date, I renamed those to js::{Array,String,Date}Constructor.
* js_IdIsIndex
* js_Init*Class
* ...

I didn't touch functions like js_strlen, js_strtod, js_free etc: strlen/strtod/free will probably be ambiguous/confusing. I also didn't rename constants/globals like js_CodeSpec.

The patch is huge, but the changes were pretty mechanical for the most part. I also removed some unused functions from jsopcode.cpp.
Attachment #8570229 - Flags: review?(bhackett1024)
Comment on attachment 8570229 [details] [diff] [review]
Patch

Review of attachment 8570229 [details] [diff] [review]:
-----------------------------------------------------------------

Nice!
Attachment #8570229 - Flags: review?(bhackett1024) → review+
Realized I forgot js_Dump*, js_GetSrcNoteOffset/js_SrcNoteLength and js_generic_native_method_dispatcher. These should be pretty trivial though.
Keywords: leave-open
Depends on: 1137998
Hmm, I guess I missed this but why does this patch remove js_DumpPC and js_DumpScript?  These are important debugging functions and I don't see any obvious replacements for them.
Flags: needinfo?(jdemooij)
OK, I see this mentioned in comment 0 now.  js_DumpPC and js_DumpScript can still be called from the debugger.
Attached patch Part 2Splinter Review
This patch unprefixes the js_Dump* functions, js_SrcNoteLength and js_GetSrcNoteOffset, and renames js_generic_native_method_dispatcher to GenericNativeMethodDispatcher.

I also added js_DumpScript and js_DumpPC back as js::DumpScript/js::DumpPC, sorry about that.
Flags: needinfo?(jdemooij)
Attachment #8571314 - Flags: review?(bhackett1024)
Comment on attachment 8571314 [details] [diff] [review]
Part 2

Review of attachment 8571314 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!
Attachment #8571314 - Flags: review?(bhackett1024) → review+
https://hg.mozilla.org/mozilla-central/rev/06f2d4958f52
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
You need to log in before you can comment on or make changes to this bug.