Closed Bug 1289617 Opened 3 years ago Closed 3 years ago

Add a bindgen opt-out for the compile-time DEBUG checker

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla50
Tracking Status
firefox50 --- fixed

People

(Reporter: bholley, Assigned: bholley)

References

Details

Attachments

(1 file, 1 obsolete file)

This causes problems with our workflow for stylo.
Attachment #8774929 - Flags: review?(sphink)
Attachment #8774929 - Flags: feedback?(ealvarez)
Comment on attachment 8774929 [details] [diff] [review]
Add an opt-out for bindgen in the SpiderMonkey DEBUG checker. v1

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

::: js/src/js-config.h.in
@@ +22,2 @@
>  #ifdef JS_DEBUG
> +#if defined(DEBUG) && !defined(RUST_BINDGEN)

Shouldn't this be |if !defined(DEBUG) && !defined(RUST_BINDGEN)|?

Other than that LGTM.
Attachment #8774929 - Flags: feedback?(ealvarez)
Good catch.
Attachment #8774929 - Attachment is obsolete: true
Attachment #8774929 - Flags: review?(sphink)
Blocks: 1289620
Comment on attachment 8774931 [details] [diff] [review]
Add an opt-out for bindgen in the SpiderMonkey DEBUG checker. v2

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

::: js/src/js-config.h.in
@@ +16,5 @@
>  #undef JS_DEBUG
>  
> +/*
> + * NB: We have a special-case for rust-bindgen, which wants to be able to
> + * generate both debug and release bindings on a single objdir.

Just "special case", no hyphen.

This makes me wonder if a more general cut-out is needed. But I don't understand what is going on here well enough to suggest anything, and I'm fine waiting for the next person who wants this before generalizing.
Attachment #8774931 - Flags: review?(sphink) → review+
Pushed by bholley@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/94789ca41a8d
Add an opt-out for bindgen in the SpiderMonkey DEBUG checker. r=sfink
https://hg.mozilla.org/mozilla-central/rev/94789ca41a8d
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.