Closed
Bug 664882
Opened 13 years ago
Closed 13 years ago
IonMonkey: Add spew mechanism
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
People
(Reporter: dvander, Assigned: dvander)
Details
Attachments
(1 file, 1 obsolete file)
11.03 KB,
patch
|
adrake
:
review+
|
Details | Diff | Splinter Review |
Works like JM and TM: dvander@dvander:~/mozilla/ionmonkey/js/src$ IONFLAGS=all ./Debug32/js Y.js [MIR] Analying script Y.js:9 [Abort] Unsupported opcode: 226 (line 10)
Attachment #539937 -
Flags: review?(adrake)
Comment 1•13 years ago
|
||
Comment on attachment 539937 [details] [diff] [review] spew Review of attachment 539937 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/ion/IonBuilder.cpp @@ +96,5 @@ > +IonBuilder::fail() > +{ > + if (error_) > + return false; > + return ABORT("out of memory"); If this is only for OOM situations, wouldn't outOfMemory or something other than "fail" be more specific? ::: js/src/ion/IonBuilder.h @@ +162,5 @@ > bool build(); > > + // Prints a message about compilation failure, if none has been printed > + // yet. > + bool fail(); I guess this is technically true now, but it seems like this function should take a message or error code or something? ::: js/src/ion/IonSpew.cpp @@ +212,5 @@ > + > + " all Everything\n" > + "\n" > + ); > + exit(0); If this were exit(1) it would be a little more obvious that we took an abnormal way out of the engine, but it doesn't necessarily indicate failure. Thoughts? @@ +224,5 @@ > + LoggingBits = uint32(-1); > +} > + > +void > +ion::IonSpewFmt(IonSpewChannel channel, const char *fmt, va_list ap) I'd probably call this IonSpewVA or something that indicates that it takes a va_list, rather than Fmt which implies to me that it takes a format string, which is true of just IonSpew too. ::: js/src/ion/MIRGraph.h @@ +115,5 @@ > } > > + // Set an error state and prints a message. Returns false so errors can be > + // propagated up. > + bool ABORT(const char *message, ...); Having this be SHOUTY makes it stand out a bit too much and makes me think it's a macro when it isn't. Can it just be "abort"?
![]() |
Assignee | |
Comment 2•13 years ago
|
||
addresses comments
Attachment #539937 -
Attachment is obsolete: true
Attachment #539937 -
Flags: review?(adrake)
Attachment #541164 -
Flags: review?(adrake)
Updated•13 years ago
|
Attachment #541164 -
Flags: review?(adrake) → review+
![]() |
Assignee | |
Comment 3•13 years ago
|
||
http://hg.mozilla.org/users/danderson_mozilla.com/ionmonkey/rev/9e51c35ab223
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•