Open Bug 1447701 Opened 4 years ago Updated 4 years ago

Find better solution for making JSID_VOID constexpr (and fix SM Rust breakage)

Categories

(Core :: JavaScript Engine, enhancement, P2)

enhancement

Tracking

()

Tracking Status
firefox61 --- affected

People

(Reporter: peterv, Unassigned)

References

Details

Attachment 8957292 [details] [diff] for bug 888600 made JSID_VOID constexpr, so that PinnedStringId could be constexpr. Waldo had some comments about how to improve the code (see bug 888600 comment 92). Also, the solution we have breaks SM Rust compilation, because now JSID_VOID isn't exposed to Rust anymore, so we need to find a solution for that.
I think we should consider making it a function (maybe also rename to JS::VoidId() or something..). Then it's very similar to other constexpr functions like JS::UndefinedValue() so it should be easy to fix the Rust bindings.
Priority: -- → P2
Sounds like my bug 888600 comment 92 "if it isn't necessary" turns out to be necessary.  Remove JSID_VOID and introduce JS::VoidId, and similar for the other constexpr global variables.

(I assume bindgen can do constexpr functions?  Seems like a bindgen deficiency if it can't, IMO.)
(In reply to Jeff Walden [:Waldo] from comment #2)
> Sounds like my bug 888600 comment 92 "if it isn't necessary" turns out to be
> necessary.  Remove JSID_VOID and introduce JS::VoidId, and similar for the
> other constexpr global variables.
> 
> (I assume bindgen can do constexpr functions?  Seems like a bindgen
> deficiency if it can't, IMO.)

Bindgen definitely can't see through constextpr functions. This is a libclang limitation which (I, at least) don't have time to fix.
(In reply to Emilio Cobos Álvarez [:emilio] from comment #4)
> Bindgen definitely can't see through constextpr functions. This is a
> libclang limitation which (I, at least) don't have time to fix.

To be more specific, it can't see through constexpr structs / unions, like this case. Constexpr constants should work just fine.

This is basically https://github.com/rust-lang-nursery/rust-bindgen/issues/1266 / https://bugs.llvm.org/show_bug.cgi?id=36576.
Hello, this started permafailing since Bug 1449051 was landed. Jeff, Peter could you please take a look at this? Thank you

https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&filter-searchStr=e5e9b629d089b999dcd27744fdf5b783872a4103&fromchange=a05e09c6fde3780a474a7e8964da03ca4decd05b&selectedJob=171156299

New failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=171156299&repo=mozilla-inbound

[task 2018-03-30T01:17:37.109Z] error[E0422]: cannot find struct, variant or union type `Value_layout` in module `JS`
[task 2018-03-30T01:17:37.109Z]   --> js/rust/src/jsval.rs:73:19
[task 2018-03-30T01:17:37.109Z]    |
[task 2018-03-30T01:17:37.110Z] 73 |         data: JS::Value_layout {
[task 2018-03-30T01:17:37.110Z]    |                   ^^^^^^^^^^^^ not found in `JS`
[task 2018-03-30T01:17:37.110Z] 
[task 2018-03-30T01:17:37.398Z] error[E0422]: cannot find struct, variant or union type `Value_layout` in module `JS`
[task 2018-03-30T01:17:37.398Z]   --> js/rust/src/jsval.rs:73:19
[task 2018-03-30T01:17:37.398Z]    |
[task 2018-03-30T01:17:37.398Z] 73 |         data: JS::Value_layout {
[task 2018-03-30T01:17:37.398Z]    |                   ^^^^^^^^^^^^ not found in `JS`
[task 2018-03-30T01:17:37.398Z] 
[task 2018-03-30T01:17:37.864Z] error[E0560]: union `jsapi::root::JS::Value` has no field named `data`
[task 2018-03-30T01:17:37.864Z]   --> js/rust/src/jsval.rs:73:9
[task 2018-03-30T01:17:37.864Z]    |
[task 2018-03-30T01:17:37.864Z] 73 |         data: JS::Value_layout {
[task 2018-03-30T01:17:37.864Z]    |         ^^^^^ `jsapi::root::JS::Value` does not have this field
[task 2018-03-30T01:17:37.864Z]    |
[task 2018-03-30T01:17:37.864Z]    = note: available fields are: `asBits_`, `asDouble_`, `debugView_`, `s_`
[task 2018-03-30T01:17:37.864Z] 
[task 2018-03-30T01:17:37.896Z] error[E0609]: no field `data` on type `&jsapi::root::JS::Value`
[task 2018-03-30T01:17:37.896Z]    --> js/rust/src/jsval.rs:195:14
[task 2018-03-30T01:17:37.896Z]     |
[task 2018-03-30T01:17:37.896Z] 195 |         self.data.asBits
[task 2018-03-30T01:17:37.896Z]     |              ^^^^
[task 2018-03-30T01:17:37.896Z] 
[task 2018-03-30T01:17:38.366Z] error[E0560]: union `jsapi::root::JS::Value` has no field named `data`
[task 2018-03-30T01:17:38.366Z]   --> js/rust/src/jsval.rs:73:9
[task 2018-03-30T01:17:38.366Z]    |
[task 2018-03-30T01:17:38.366Z] 73 |         data: JS::Value_layout {
[task 2018-03-30T01:17:38.366Z]    |         ^^^^^ `jsapi::root::JS::Value` does not have this field
[task 2018-03-30T01:17:38.366Z]    |
[task 2018-03-30T01:17:38.366Z]    = note: available fields are: `asBits_`, `asDouble_`, `debugView_`, `s_`
[task 2018-03-30T01:17:38.366Z] 
[task 2018-03-30T01:17:38.400Z] error[E0609]: no field `data` on type `&jsapi::root::JS::Value`
[task 2018-03-30T01:17:38.400Z]    --> js/rust/src/jsval.rs:195:14
[task 2018-03-30T01:17:38.400Z]     |
[task 2018-03-30T01:17:38.400Z] 195 |         self.data.asBits
[task 2018-03-30T01:17:38.400Z]     |              ^^^^
[task 2018-03-30T01:17:38.400Z] 
[task 2018-03-30T01:17:38.569Z] error: aborting due to 3 previous errors
[task 2018-03-30T01:17:38.570Z] 
[task 2018-03-30T01:17:38.812Z] error: Could not compile `js`.
[task 2018-03-30T01:17:38.812Z] 
[task 2018-03-30T01:17:38.812Z] Caused by:
[task 2018-03-30T01:17:38.813Z]   process didn't exit successfully: `rustc --crate-name js js/rust/src/lib.rs --crate-type lib --emit=dep-info,link -C opt-level=1 -C debuginfo=2 -C debug-assertions=on --cfg feature="debugmozjs" --cfg feature="mozjs_sys" -C metadata=d115284a5694d6a6 -C extra-filename=-d115284a5694d6a6 --out-dir /builds/worker/workspace/build/src/target/debug/deps -C incremental=/builds/worker/workspace/build/src/target/debug/incremental -L dependency=/builds/worker/workspace/build/src/target/debug/deps --extern mozjs_sys=/builds/worker/workspace/build/src/target/debug/deps/libmozjs_sys-c94cb805f9e2d92b.rlib --extern num_traits=/builds/worker/workspace/build/src/target/debug/deps/libnum_traits-de77720ffad8de8e.rlib --extern log=/builds/worker/workspace/build/src/target/debug/deps/liblog-eccf837517dbc577.rlib --extern lazy_static=/builds/worker/workspace/build/src/target/debug/deps/liblazy_static-881e34c4c5bd96a8.rlib --extern libc=/builds/worker/workspace/build/src/target/debug/deps/liblibc-3a25fb3e68598894.rlib -L native=/builds/worker/workspace/build/src/target/debug/build/js-d6945339b91f1018/out/lib -l static=jsglue -L native=/builds/worker/workspace/build/src/target/debug/build/mozjs_sys-576dcf852bb553ef/out/js/src/build -L native=/builds/worker/workspace/build/src/target/debug/build/mozjs_sys-576dcf852bb553ef/out/js/src -L native=/builds/worker/workspace/build/src/target/debug/build/mozjs_sys-576dcf852bb553ef/out/dist/bin -L native=/usr/lib/x86_64-linux-gnu` (exit code: 101)
[task 2018-03-30T01:17:38.813Z] warning: build failed, waiting for other jobs to finish...
[task 2018-03-30T01:17:40.411Z] error: aborting due to 3 previous errors
[task 2018-03-30T01:17:40.411Z] 
[task 2018-03-30T01:17:40.664Z] error: Could not compile `js`.
[task 2018-03-30T01:17:40.664Z] 
[task 2018-03-30T01:17:40.664Z] Caused by:
[task 2018-03-30T01:17:40.664Z]   process didn't exit successfully: `rustc --crate-name js js/rust/src/lib.rs --emit=dep-info,link -C debuginfo=2 --test --cfg feature="debugmozjs" --cfg feature="mozjs_sys" -C metadata=9b7625c1573e823a -C extra-filename=-9b7625c1573e823a --out-dir /builds/worker/workspace/build/src/target/debug/deps -C incremental=/builds/worker/workspace/build/src/target/debug/incremental -L dependency=/builds/worker/workspace/build/src/target/debug/deps --extern mozjs_sys=/builds/worker/workspace/build/src/target/debug/deps/libmozjs_sys-c94cb805f9e2d92b.rlib --extern num_traits=/builds/worker/workspace/build/src/target/debug/deps/libnum_traits-de77720ffad8de8e.rlib --extern log=/builds/worker/workspace/build/src/target/debug/deps/liblog-eccf837517dbc577.rlib --extern lazy_static=/builds/worker/workspace/build/src/target/debug/deps/liblazy_static-881e34c4c5bd96a8.rlib --extern libc=/builds/worker/workspace/build/src/target/debug/deps/liblibc-3a25fb3e68598894.rlib -L native=/builds/worker/workspace/build/src/target/debug/build/js-d6945339b91f1018/out/lib -l static=jsglue -L native=/builds/worker/workspace/build/src/target/debug/build/mozjs_sys-576dcf852bb553ef/out/js/src/build -L native=/builds/worker/workspace/build/src/target/debug/build/mozjs_sys-576dcf852bb553ef/out/js/src -L native=/builds/worker/workspace/build/src/target/debug/build/mozjs_sys-576dcf852bb553ef/out/dist/bin -L native=/usr/lib/x86_64-linux-gnu` (exit code: 101)
[taskcluster 2018-03-30 01:17:41.019Z] === Task Finished ===
[taskcluster 2018-03-30 01:17:41.135Z] Artifact "public/build" not found at "/builds/worker/artifacts/"
[taskcluster 2018-03-30 01:17:41.707Z] Unsuccessful task run with exit code: 101 completed in 335.151 seconds
Flags: needinfo?(peterv)
Flags: needinfo?(jwalden+bmo)
That failure has nothing to do with this bug. It's just to do with the structure of Value changing in bug 1449051 and the rust code that was poking at the internals therefore needing to be updated.
Flags: needinfo?(peterv)
I filed bug 1450311 on the problem.
Flags: needinfo?(jwalden+bmo)
You need to log in before you can comment on or make changes to this bug.