search

dalek-cryptography / curve25519-dalek

A pure-Rust implementation of group operations on Ristretto and Curve25519
Other
850 stars 416 forks source link

Enable unexpected cfgs lint #656

Open Wuelle opened 1 month ago

Wuelle commented 1 month ago

The unexpected_cfgs lint was previously disabled in 9252fa5c0d09054fed. Since the lint can now be configured using Cargo.toml^2 no build.rs shenanigans are necessary anymore.

This PR also fixes some immediate bugs found, two tests used comma-seperated config flags cfg(target_feature = "avx512ifma,avx512vl"), which is invalid (and always false). Instead, it should be cfg(all(target_feature = "avx512ifma", target_feature = "avx512vl")). From what I've seen, this pattern also exists in the proc-macros used to enable target features for certain functions. But I don't understand the codebase well enough to say this for certain, and I didn't look too much into it either.

I believe usage cfg(nightly) is also a bug - the documentation^1 seems to suggest that features are automatically enabled when compiled

If compiled on a non-nightly compiler, curve25519-dalek will not include AVX512 code, and therefore will never select it at runtime.

This is incorrect, since cfg(nightly) does not exist, and AFAIK there is no way to detect whether code is being compiled by a "nightly" compiler (and the definition of "nightly" changes all the time anyways). But perhaps this was intended to be used manually? (via --cfg nightly)

tarcieri commented 1 month ago

I believe usage cfg(nightly) is also a bug - the documentation2 seems to suggest that features are automatically enabled when compiled

They're automatically enabled when compiled because the build script detects the nightly compiler and sets cfg(nightly) accordingly:

https://github.com/dalek-cryptography/curve25519-dalek/blob/31ccb67/curve25519-dalek/build.rs#L38-L44

You'll still need to preserve cfg(nightly) for this to work.

Wuelle commented 1 month ago

I believe usage cfg(nightly) is also a bug - the documentation2 seems to suggest that features are automatically enabled when compiled

They're automatically enabled when compiled because the build script detects the nightly compiler and sets cfg(nightly) accordingly:

https://github.com/dalek-cryptography/curve25519-dalek/blob/31ccb67/curve25519-dalek/build.rs#L38-L44

You'll still need to preserve cfg(nightly) for this to work.

I believe usage cfg(nightly) is also a bug - the documentation2 seems to suggest that features are automatically enabled when compiled

They're automatically enabled when compiled because the build script detects the nightly compiler and sets cfg(nightly) accordingly:

https://github.com/dalek-cryptography/curve25519-dalek/blob/31ccb67/curve25519-dalek/build.rs#L38-L44

You'll still need to preserve cfg(nightly) for this to work.

Ah thanks, i didn't see the build script. Fixed