Remove uneeded instrinsic stub

[MabezDev]

The changes from rust-lang/compiler-builtins#517 have landed.

I suppose this might break stuff for people who don't upgrade to 1.70, but they can always add there own stub if they want.

[bugadani]

I think given my curse I'd prefer some conditional compilation trickery that only removes this function when using >= 1.70

[MabezDev]

I think given my curse I'd prefer some conditional compilation trickery that only removes this function when using >= 1.70

I'm not sure we should keep this workaround in tbh, even behind a cfg. Anyone using <1.70.0.0 can just add the workaround back into their bin crate. Patching this sort of thing in a library is not a good idea.

[MabezDev]

welp I take it back because apparently, soft-float is missing intrinsic's for RISCV too? wtf?

[bjoernQ]

I guess a cfg-gate for Xtensa / RISCV would be good enough plus probably mention this in the README for now (i.e. what a user should do if there they get undefined symbol: floor)

[MabezDev]

Maybe (if we don't mind another nightly feature) we can mark it with weak linkage? I really don't like the idea of having this symbol in a library crate, IMO this should really be patched in the final binary crate.

[bugadani]

this should really be patched in the final binary crate.

This is kinda breaking the stability guarantees that rust usually holds (yes, I know the library is independent of the language, but the philosophy is similar enough): a compiler upgrade shouldn't break existing code. The workaround is easy enough in esp-wifi (though cfg(version) is unstable). I agree that the function has no logical place inside esp-wifi but if this is a problem, it can be moved to some support crate.

[MabezDev]

This is kinda breaking the stability guarantees that rust usually holds (yes, I know the library is independent of the language, but the philosophy is similar enough): a compiler upgrade shouldn't break existing code. The workaround is easy enough in esp-wifi (though cfg(version) is unstable). I agree that the function has no logical place inside esp-wifi but if this is a problem, it can be moved to some support crate.

Whilst I agree, this actually introduces a breaking change time bomb for the future, the second this intrinsic is available for RISCV, builds stop working when upgrading the compiler. This is what happened with 1.70.0 using esp-wifi on Xtensa chips. That's what I'm trying to avoid.

Interestingly, it seems the failure is only on the C6 (riscv32imac) but the other chips (riscv32imc) seem to compile fine, no idea why that would be.

[bjoernQ]

This is kinda breaking the stability guarantees that rust usually holds (yes, I know the library is independent of the language, but the philosophy is similar enough): a compiler upgrade shouldn't break existing code. The workaround is easy enough in esp-wifi (though cfg(version) is unstable). I agree that the function has no logical place inside esp-wifi but if this is a problem, it can be moved to some support crate.

Whilst I agree, this actually introduces a breaking change time bomb for the future, the second this intrinsic is available for RISCV, builds stop working when upgrading the compiler. This is what happened with 1.70.0 using esp-wifi on Xtensa chips. That's what I'm trying to avoid.

Interestingly, it seems the failure is only on the C6 (riscv32imac) but the other chips (riscv32imc) seem to compile fine, no idea why that would be.

I think it's only breaking C6 since the blobs for C2/C3 don't include code that needs floor

I guess if it's solvable with weak linking that wouldn't be a bad thing (and we need nightly anyway)

I also agree it shouldn't be ideally solved in the library but will make the user's lives much easier

[MabezDev]

I've gone with the weak linkage option because if we're pulling in an unstable feature we may as well use the one that requires less work to maintain.

By the way it looks like in the future all intrinsics from compiler builtins could become weak by default.

[bjoernQ]

LGTM