Closed Bug 664882 Opened 14 years ago Closed 14 years ago

IonMonkey: Add spew mechanism

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: dvander, Assigned: dvander)

Details

Attachments

(1 file, 1 obsolete file)

Attached patch spew (obsolete) — 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 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"?
Attached patch v2Splinter Review
addresses comments
Attachment #539937 - Attachment is obsolete: true
Attachment #539937 - Flags: review?(adrake)
Attachment #541164 - Flags: review?(adrake)
Attachment #541164 - Flags: review?(adrake) → review+
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: