Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove "placeholder constants" #3131

Open
Tracked by #3248
workingjubilee opened this issue Mar 2, 2023 · 3 comments
Open
Tracked by #3248

Remove "placeholder constants" #3131

workingjubilee opened this issue Mar 2, 2023 · 3 comments
Labels
breakage-candidate C-bug Category: bug E-medium E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate.
Milestone

Comments

@workingjubilee
Copy link
Member

It's common for C APIs to define constant values to denote the "last" value in a set of constants, like ELAST or IPPROTO_MAX. This seems prone to weird bugs, and these constants provide very marginal benefit to Rust programmers. Since we're considering some breakage anyways, we should just remove these values where we are made aware of them.

See:

@JohnTitor
Copy link
Member

👍, before 0.3, I'd like to deprecate these values.

@JohnTitor
Copy link
Member

Noticed that we have a lot of *MAX consts and deprecating manually is quite hard. We could write a script but it can cause a false positive, hmm. For now, I've edited the PR template to mention this concern.

@Amanieu
Copy link
Member

Amanieu commented Mar 15, 2024

An alternative to deleting these placeholder constants would be to still provide them but not make any guarantees about their value moving forward. This means that we would be free to change the value of these constants without it being considered a breaking change.

CI tests should specifically exclude these, although it would be nice to make CI fail only if our value for such constants is lower than the one on the system.

@tgross35 tgross35 added E-medium E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. and removed help wanted labels Aug 29, 2024
@thesummer thesummer mentioned this issue Oct 15, 2024
2 tasks
@thesummer thesummer mentioned this issue Oct 17, 2024
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breakage-candidate C-bug Category: bug E-medium E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate.
Projects
None yet
Development

No branches or pull requests

4 participants