Closed
Bug 1082679
Opened 10 years ago
Closed 6 years ago
Rename JSTrapStatus
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
DUPLICATE
of bug 1443583
People
(Reporter: evilpie, Unassigned)
References
Details
Attachments
(2 files)
14.54 KB,
patch
|
Details | Diff | Splinter Review | |
39.00 KB,
patch
|
Details | Diff | Splinter Review |
No description provided.
I am a newbie . I would like to work on this bug . I have read the comments section in the bug1069694 but I could not figure out what JSTrapStatus should be renamed as?? Can anyone please help me??
Comment 2•6 years ago
|
||
It would be nice to (1) change it to an enum class (2) rename to TrapStatus (3) move into the |js| namespace, something like this: enum class TrapStatus { Error, Continue, Return, Throw, Limit }; Thanks for working on this!
Attachment #8944241 -
Flags: review?(jdemooij)
Comment 5•6 years ago
|
||
Comment on attachment 8944241 [details] [diff] [review] bug-1082679-v1.patch Review of attachment 8944241 [details] [diff] [review]: ----------------------------------------------------------------- Hi, thanks for the patch! Did you compile the browser or JS shell with these changes? I'm asking because this patch changes Debugger.h, but JSTrapStatus is used in a lot of other files: https://searchfox.org/mozilla-central/search?q=symbol:T_JSTrapStatus&redirect=false Let me know if you need help with that. ::: js/src/vm/Debugger.h @@ +43,5 @@ > + ERROR, > + CONTINUE, > + RETURN, > + THROW, > + LIMIT It would be nice to rename these values to Error, Continue, Return, Throw, Limit @@ +548,1 @@ > MutableHandleValue vp, We should also fix the "alignment" here and elsewhere.
Attachment #8944241 -
Flags: review?(jdemooij)
Thanks for reviewing the patch. I do not know to compile a patch. Is it just rebuilding Mozilla-central? I will rename the values you talked about in the next patch. Do I have to change jsTrapStatus everywhere by going into the code again and again or is there any other way. I know little but I would love to learn. Can you please help me? Thank you.
Comment 7•6 years ago
|
||
(In reply to dvabhinav31@gmail.com from comment #6) > I do not know to compile a patch. Is it just rebuilding Mozilla-central? Yeah. This page might be useful for building the JS shell: https://wiki.mozilla.org/JavaScript:New_to_SpiderMonkey > Do I have to change jsTrapStatus everywhere by going into the code again and > again or is there any other way. You could use sed or something: https://stackoverflow.com/questions/9704020/recursive-search-and-replace-in-text-files-on-mac-and-linux Some text editors also let you search-and-replace in all files in a directory.
Attachment #8946260 -
Flags: review?(jdemooij)
Comment 9•6 years ago
|
||
Comment on attachment 8946260 [details] [diff] [review] bug-1082679.patch Review of attachment 8946260 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jit/VMFunctions.cpp @@ +1094,4 @@ > MOZ_ASSERT(script->stepModeEnabled() || script->hasBreakpointsAt(pc)); > > RootedValue rval(cx); > + TrapStatus status = JSTRAP_CONTINUE; You renamed JSTRAP_CONTINUE so this won't compile. Please make sure you can build the JS shell or Firefox with these changes - see the link I gave in comment 7. Let me know if you need help with that.
Attachment #8946260 -
Flags: review?(jdemooij)
Comment 10•6 years ago
|
||
Ok I will change that. I will recompile JS shell before uploading the next patch. But, I have a doubt. If I push a new patch, The patch will contain only the new changes. But what about the other changes that are not there in the tip i.e, the changes made in the earlier patch?
Comment 11•6 years ago
|
||
(In reply to dvabhinav31@gmail.com from comment #10) > But, I have a doubt. If I push a new patch, The patch will contain only the > new changes. But what about the other changes that are not there in the tip > i.e, the changes made in the earlier patch? Do you use Mercurial or Git? If you use Mercurial with mq you can use qref to add changes to a patch or qfold to merge patches. Git also lets you squash patches.
Comment 12•6 years ago
|
||
I use mercurial with hg
Comment 13•6 years ago
|
||
Jason has a patch for this in bug 1443583.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → DUPLICATE
Comment 14•6 years ago
|
||
Oops. Sorry, abhinavdv!
You need to log in
before you can comment on or make changes to this bug.
Description
•