Closed
Bug 664882
Opened 14 years ago
Closed 14 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•14 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•14 years ago
|
||
addresses comments
Attachment #539937 -
Attachment is obsolete: true
Attachment #539937 -
Flags: review?(adrake)
Attachment #541164 -
Flags: review?(adrake)
Updated•14 years ago
|
Attachment #541164 -
Flags: review?(adrake) → review+
![]() |
Assignee | |
Comment 3•14 years ago
|
||
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.
Description
•