`cargo test` is broken despite specified `--rust-target 1.47`

[heinrich5991]

#2203 caused std::ptr::addr_of! to be emitted in tests, which is only available since 1.51.

Do tests fall under the guarantee? It'd be nice if this was called out in the changelog, if it was intentional.

[pvdrz]

some tests have the bindgen-flags which take care of generating bindings for a specific target. That being said. The msrv for bindgen is 1.57.0 which would explain why this feature is no longer tracked in all tests.

[pvdrz]

Additionally I couldn't even get bindgen to compile with rust 1.45 due to clap requiring a higher version.

[heinrich5991]

@pvdrz This is about the generated code by bindgen. Are you saying the --rust-target version has no effect? Perhaps it should be removed then.

$ bindgen --help
[…]
        --rust-target <rust-target>
            Version of the Rust compiler to target. Valid options are: ["1.0", "1.17", "1.19", "1.20", "1.21", "1.25",
            "1.26", "1.27", "1.28", "1.30", "1.33", "1.36", "1.40", "1.47", "nightly"]. Defaults to "1.47".
[…]

[pvdrz]

@heinrich5991 oh ok this is an interesting point. The MSRV to run bindgen is 1.57 but I'm not sure if generating code for an older rust target is supported or not. Deferring to @emilio for this one.

Even if tests are broken, bindgen should be able to generate valid code for any --rust-target. My guess is that this commit broke the tests because most people will use the stable toolchain (or 1.57 at best) locally and won't have an easy way to know what features aren't available in older versions.

A possible way to mitigate this would be asking contributors to always include the --rust-target flag for new tests with the version of the compiler they used.

[emilio]

Yeah, generally it should be supported. That said, we should consider start deprecating some of the older rust targets. This bug is valid but my guess is that specially since it only affects tests, it's relatively low priority.

[pvdrz]

Maybe we could add extra CI jobs for each rust target to guarantee that this never happens again, not sure if the increased run time is worth it though.

[kulp]

not sure if the increased run time is worth it though

I think it could be worthwhile if it were run only upon releases; but running on every PR I tend to agree might become excessive. Maybe a middle ground would be running them only on master itself, therefore only after merge? particularly since they would fire pretty infrequently.

[pvdrz]

Solving this issue is proving to be harder than I expected. There are several issues and I don't have a clear solution for them:

• First, the test suite does not even compile with a rust target lower than 1.30.0 because of the edition entry in the manifest. We cannot remove it either due to the change about the semantics of leading :: in a path between the 2015 and 2018 edition. To solve this we would have to deprecate the following targets: 1.0, 1.17, 1.19, 1.20, 1.21, 1.25, 1.26, 1.27, 1.28. Or figure out how to emit 2015-edition valid code.

• Second, dynamic loading cannot be tested with a rust target of1.36 or lower due to the msrv of libloading. We could try to use an older version of libloading or somehow hack our way to avoid including libloading and the dynamic loading tests for older versions.