chore: please make unstable truly unstable

[dzmitry-lahoda]

currently unstable tests on 2 years old version of rust, it is wrong. 2 years version of rust is not unstable anymore - i would say that version does not exist and testing on is dummy stub (so it tests nothing at all).

[Veetaha]

The test-unstable tests run against the latest beta and nightly versions. See the logs here. Could you elaborate?

[dzmitry-lahoda]

[dzmitry-lahoda]

rust-version = "1.59.0"

so bon-macro is tested with old version of rust, not nightly/beta.

so not sure why limit macro to old version (it runs on dev machine with for sure has compiler not 2 years old).

it is clear why bon crate limited so.

[Veetaha]

It's just a clippy lint called clippy::incompatible_msrv. The jobs still test against latest beta/nightly

[Veetaha]

The rust-version field in Cargo.toml declares the minimum required version of rustc for this library to compile. Given that people often upgrade their rust toolchain lazily libraries have to support older compiler versions if they want to support projects with older rustc versions.

Also Linux distros come with a preinstalled rust toolchain by default which is often quite outdated (#81 (comment))

[dzmitry-lahoda]

For stable version fine to support old. No problem.
So still problem. Was Debian that Linux distro dicussed to have old version of Rust? Debian 11 LTS 1.48 rust. Next current Debian LTS is 1.63. Why not 1.48 than ? Because 1.59 will not compile right? Or 1.63? Or other target Linux system was used?

For unstable version, that weird. Considering 1.59 version to be unstable is weird. It does not makes any sense. Is there any prominent crate doing that?

[dzmitry-lahoda]

I mean fine to compile stable version of bon with rust target to be 1.59(so may be 1.63 is more reasonable?). But compiling unstable version of bon with rust target 1.59 does not have any sense. Unless I miss something?

[Veetaha]

bon must always comply with 1.59 MSRV no matter what version of toolchain is used to be always be consistent (except for known exceptions marked with rustversion attribute). There is no exception for the unstable toolchain

[Veetaha]

I don't understand what change you propose, and why

[Veetaha]

I assume you confuse unstable toolchain and "unstable" cargo feature flag, which absolutely have nothing in common. The feature flag is just a marker for the user to sign the contract that bon may break the API of #[builder(getter)] attributes in patch releases. And my intention with that feature flag was to check for it's presence in the validation method of parameters before allowing the usage of builder(getter).

See my PR comment. I didn't ask to put #[cfg(feature = "unstable")] all over the implementation, I meant a simple

if !cfg!(feature = "unstable") && self.getter.is_some() {
    bail!(...)
}

#118 (review).

The getters feature must compile on all versions of rustc supported by bon because it'll eventually become stable. Anyone trying out that feature is not required to install the unstable rust toolchain

[Veetaha]

This is following the similar approach of clap https://github.com/clap-rs/clap/blob/df1efca03509f736e26d2f16766b7ea06acc3ecf/Cargo.toml#L169. I think it means the feature flag should be called unstable-getters, not just unstable or get to mark a specific feature as unstable

[dzmitry-lahoda]

installed 1.59 rust, it fails

Updating crates.io index
error: failed to select a version for the requirement `syn = "^2.0.56"`
candidate versions found which didn't match: 2.0.56, 2.0.55, 2.0.54, ...
location searched: crates.io index
required by package `bon-macros v2.2.1 (/home/dz/github.com/elastio/bon/bon-macros)`

so not clear at all what currently checked, but for sure no that specific linuxes with old version of rust can use bon macro. they cannot use bon macro using rust compiler version of 1.59.

[Veetaha]

They can, they just need to use lower versions of dependencies. Take a look at how test-msrv.sh works:

bon/scripts/test-msrv.sh

Lines 35 to 38 in 3217b4b

	step cargo update -p syn --precise 2.0.56
	step cargo update -p tokio --precise 1.29.1
	step cargo update -p expect-test --precise 1.4.1
	step cargo update -p windows-sys --precise 0.52.0

[dzmitry-lahoda]

I didn't ask to put #[cfg(feature = "unstable")] all over the implementation, I meant a simple
if !cfg!(feature = "unstable") && self.getter.is_some()

@Veetaha not sure I get that. So you propose to have attribute to be available to put by users, but when macro generates code it just ignores it? Is not that bad? I supposed if get does not work it should not be available to put as macro attribute by user?

[Veetaha]

just ignores it

No, the macro must return an error. See the bail!(...) inside of the if?

[dzmitry-lahoda]

unstable-getters

why not singular getter? so attribute name equals feature? why it is unstable when it will be build on stable super old rust? or why it is unstable needed if it is behind feature getter as has one and only one impl possible (not so much can think for getter unless whole gen to be changed - but than whole bon is unstable)?

[dzmitry-lahoda]

No, the macro must return an error. See the bail!(...) inside of the if?

would not rust suggest option to type that field and than error during compilation in this case? how to ensure there is not suggestion to user to type getter if feature is off?

[Veetaha]

There are no completions in IDEs for proc macro attributes (except for some manual hacks that I did, which are limited and don't cover member-level attributes).