664882 - IonMonkey: Add spew mechanism

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: spew

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)

Comment 1

[Andrew Drake [: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"?

Comment 2

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: v2

addresses comments

Comment 3

[David Anderson [:dvander] - inactive, e-mail if emergency]

http://hg.mozilla.org/users/danderson_mozilla.com/ionmonkey/rev/9e51c35ab223