mozjs-0.8.0 compilation error

[gterzian]

Recently rebased off master, now getting an error with ./mach check that strangely arises from mozjs

error[E0277]: the trait bound `u8: std::convert::From<rust::MutableHandle<'_, u8>>` is not satisfied                                                             
    --> /mozjs-0.8.0/src/rust.rs:1205:52
     |
1205 |             wrap!(@inner $saved <> ($($acc,)* $arg.into(),) <> $($rest)*);
     |                                                    ^^^^ the trait `std::convert::From<rust::MutableHandle<'_, u8>>` is not implemented for `u8`
     | 
    ::: /mozjs-0.8.0/src/jsapi_wrappers.in:44:1
     |
44   | wrap!(jsapi: pub fn FinishMultiOffThreadScriptsDecoder(cx: *mut JSContext, token: *mut ::std::os::raw::c_void, scripts: MutableHandle<ScriptVector>) -> bool);
     | -------------------------------------------------------------------------------------------------------------------------------------------------------------- in this macro invocation
     |
     = help: the following implementations were found:
               <u8 as std::convert::From<bool>>
     = note: required because of the requirements on the impl of `std::convert::Into<u8>` for `rust::MutableHandle<'_, u8>`

error[E0277]: the trait bound `u8: std::convert::From<rust::MutableHandle<'_, u8>>` is not satisfied
    --> /mozjs-0.8.0/src/rust.rs:1348:113
     |
1348 |             wrap!(@inner $saved <> ($($declargs)* $arg: &mut MutableHandle<$gentype> , )  <> ($($acc,)* (*$arg).into(),) <> $($rest)*);
     |                                                                                                                 ^^^^ the trait `std::convert::From<rust::MutableHandle<'_, u8>>` is not implemented for `u8`
     | 
    ::: /mozjs-0.8.0/src/jsapi_wrappers.in:44:1
     |
44   | wrap!(jsapi: pub fn FinishMultiOffThreadScriptsDecoder(cx: *mut JSContext, token: *mut ::std::os::raw::c_void, scripts: MutableHandle<ScriptVector>) -> bool);
     | -------------------------------------------------------------------------------------------------------------------------------------------------------------- in this macro invocation
     |
     = help: the following implementations were found:
               <u8 as std::convert::From<bool>>
     = note: required because of the requirements on the impl of `std::convert::Into<u8>` for `rust::MutableHandle<'_, u8>`

error: aborting due to 2 previous errors

Am I missing an update command? I tried with ./mach clean a few times already, also update rustup. Could it be that my machine is using an old rust version despite mach?

[SimonSapin]

I’ve seen the same on Ubuntu, and was able to work around it by removing old clang/LLVM packages. (Namely 3.9 and 4.0, leaving only 6.0. Try apt search libclang or whatever is similar for your system and see what’s installed.)

[gterzian]

Thanks for the pointer.

Did anyone encounter this on Mac?

I just upgraded to 10.13, and installed brand new "command line tools" which come with their latest version of clang(and nothing was installed as clang right after the system upgrade), and the issue persists...

[gterzian]

by the way rustup which cargo inside the Servo dir says .rustup/toolchains/nightly-2018-07-17-x86_64-apple-darwin/bin/cargo

and both

/mach clean-nightlies --keep 3 --force
./mach clean-cargo-cache --keep 3 --force

also didn't help...

[gterzian]

looks like a dupe of servo/rust-mozjs#435, yet it's noteworthy that the issue persists after upgrading to the latest Mac command line tools. Perhaps it requires the absolutely latest LLVM via a manual install?

[jdm]

If there are multiple versions installed, the oldest one will often be selected by default. This can be changed via the LIBCLANG_PATH variable.

[gterzian]

Thanks!

Ok so Mac users: it worked for me with an install of llvm via homebrew, followed by:

1. ./mach clean
2. LIBCLANG_PATH="/usr/local/Cellar/llvm/6.0.1/lib" ./mach build -d

The clean is absolutely necessary...

[nox]

The LIBCLANG_PATH variable isn't necessary AFAIK, at least on my machine, this path takes priority over others.

[nox]

If there are multiple versions installed, the oldest one will often be selected by default. This can be changed via the LIBCLANG_PATH variable.

Oh I missed this, disregard me. I would prefer if we find a way to not have to tell people to export environment variables.

@qdot Would you consider a bug that lllvm/4.* takes priority over llvm/6.* in clang-sys?

[jdm]

@KyleMayes, not qdot.

[jdm]

We may want to see if https://github.com/KyleMayes/clang-sys#supported-versions could force a newer version that's supported.

[nox]

Alternatively, calling .rev() on the search path iterator would also fix the issue.

https://github.com/KyleMayes/clang-sys/blob/67f7d8c25eff694d7ba58ff42da6e5b502413b7d/build.rs#L277

[gterzian]

it seems to work for me, without the homebrew one actually. Perhaps the homebrew uninstall llvm actually also removed an older version that was previously there? I don't know. Anyway, default mac, without flag, seems to work after all...

[nox]

So without my patch, it builds with LLVM 4 only (AFAIK). With my patch, it builds with everything.

I think we should make clang-sys reverse its search path nonetheless.

[KyleMayes]

I agree this should be improved.

Instead of changing the order in which directories are visited, I think it might be better to collect all libclang binaries that can be found, then picking the highest version found (preferring the unversioned libclang.so if found).

The way it works now simply picks the highest version in the first directory in which one or more instances of libclang are found, which has the issue that there are often multiple directories that have one or more versions of libclang scattered around.

Thoughts? (since this would be a "breaking" change for users)

[KyleMayes]

I have implemented what is hopefully an improved selection of versioned binaries in v0.24.0 of libclang.

You can read more details in the README.

[tigercosmos]

the latest bindgen 0.46 has already update clang-sys to 0.26
current use bindgent 0.39
shall we update the version?

[atouchet]

@tigercosmos there is a PR to update bindgen here: #22221.

[tigercosmos]

I install clang 6 and remove all other old version. also update bindgen to lastest. But still encounter the same issue. I have no idea. orz
btw, why travis install clang 3.9?

[jdm]

You may need to remove the mozjs* directories from your build directory to observe changes.

[tigercosmos]

deleting mozjs* folders is similar to mach clean. Unfortunately, still not work. :(

[technicalguy]

I had this same issue on Linux Mint 18 and removing (lib)clang 4.0 and installing 6.0 seems to have fixed it.

[tigercosmos]

It works by following http://apt.llvm.org/ steps

[danShumway]

Removing clang 4.0 also worked for me on Ubuntu 16.04.

I'm not a fan of needing to set a bunch of system variables, but also not a fan of needing to uninstall software from my computer to get a build working. Would be preferable if the system was smarter about finding the most recent version of libclang.

Not sure if @KyleMayes's pull request solves it. I still needed to remove the old version, but maybe that's just because I'm building off of master?

[KyleMayes]

The current libclang selection process works as described here.

This works well when there is a versioned instance of libclang that has the minimum version you want, but breaks down when there is a versioned instance of libclang "hiding" the unversioned instance you want or multiple unversioned instances.

The ideal solution would be to somehow determine the version of these unversioned instances of libclang so that the latest (or minimum required) version could be selected without these issues.

An idea I had for this, which would only work when linking dynamically, would be to determine the version of an unversioned instance of libclang by dynamically loading the instance and then attempting to load functions only available on certain versions of libclang.

For example:

• attempt to load clang_CXIndex_setInvocationEmissionPathOption (only available on 6.0 and later)
• attempt to load clang_Cursor_isExternalSymbol (only available on 5.0 and later)
• attempt to load clang_EvalResult_getAsLongLong (only available on 4.0 and later)
• etc...

I will look into this further and report back.

[KyleMayes]

The above idea works, so I have released v0.27.0 of clang-sys with improved version detection.

This should mean that the most recent version of libclang.so should be selected regardless of which instances of libclang.so have version numbers in their filenames.

I also implemented an assert-minimum feature which causes the build script to panic with a descriptive error message if the most recent version of libclang.so available does not meet the specified minimum version.

[dralley]

Is this fixed?

[CYBAI]

We've updated our to mozjs to 0.13.0 with 81c88926aa57f8571b3784216907e4e06363b345 commit hash so maybe we can file new issues if there's any new compilation error. I'm going to close (please help me reopen it if it's better to track mozjs compilation error here 🙇 )

[KyleMayes]

This has not been fixed, unfortunately.

My attempt at a solution (speculatively loading libclang dynamic libraries to determine their version) caused some weird bugs where the loaded libraries could not be properly unloaded (or something like that, not sure exactly what was happening) causing serious issues for some users.

I had to revert those changes, so I think the process going forward will have to be to either uninstall old libclang versions or set either the LIBCLANG_PATH or LLVM_CONFIG_PATH environment variable.

However, it would be possible to detect the libclang version once it is loaded which could be used to assert that an acceptable minimum version of libclang has been loaded.

[KyleMayes]

I added support to clang-sys for querying the version of libclang linked to when using runtime linking (which is the default used by bindgen) in the most recent version of clang-sys (0.29.3).

See here for the method I added.

This could be used to ensure that the loaded version of libclang meets your needs. For example this code checks that the loaded version is at least 6.0 and panics otherwise:

clang_sys::load().unwrap();

let library = clang_sys::get_library().unwrap();
let version = library.version();

if version.map_or(false, |v| v < clang_sys::Version::V6_0) {
    let name = version
        .map(|v| format!("{:?}", v))
        .unwrap_or_else(|| "3.4 or earlier".into());

    panic!(
        "the loaded version of libclang ({} @ {}) is too old, 6.0 or later is required",
        name,
        library.path().display(),
    );
}

Perhaps bindgen could expose a function which does something like this for downstream crates to verify the version in use?