Restore Label's asserts

RESOLVED FIXED in mozilla33

Status

()

Core
JavaScript Engine: JIT
--
enhancement
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: sunfish, Assigned: sunfish)

Tracking

unspecified
mozilla33
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
Created attachment 8447369 [details] [diff] [review]
label-asserts.patch

Bug 976446 moved LabelBase, Label, and NonAssertingLabel into their own header file, but dropped the Label class' assert -- the assert that NonAssertingLabel exists to suppress. Attached is a patch to restore the assert. If this isn't desired, we should delete the NonAssertingLabel class.
Attachment #8447369 - Flags: review?(jdemooij)
Comment on attachment 8447369 [details] [diff] [review]
label-asserts.patch

Review of attachment 8447369 [details] [diff] [review]:
-----------------------------------------------------------------

Good catch. I didn't notice the asserts being removed as part of the move..

::: js/src/jit/Label.h
@@ +6,5 @@
>  
>  #ifndef jit_Label_h
>  #define jit_Label_h
>  
> +#include "jit/Ion.h"

Hm isn't this a cyclic include?

@@ +82,5 @@
> +#ifdef DEBUG
> +        // The assertion below doesn't hold if an error occurred.
> +        if (OOM_counter > OOM_maxAllocations)
> +            return;
> +        if (IonContext *context = MaybeGetIonContext())

Nit: add {}
Attachment #8447369 - Flags: review?(jdemooij) → review+
(In reply to Jan de Mooij [:jandem] from comment #1)
> Hm isn't this a cyclic include?

I thought Ion.h had more #includes but it's not too bad. Nevermind.
(Assignee)

Comment 3

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/b5cf6f6dfa83
https://hg.mozilla.org/mozilla-central/rev/b5cf6f6dfa83
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.