Skip to content

use ConstZero instead of Default #838

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

Merged
merged 1 commit into from
Mar 9, 2025
Merged

use ConstZero instead of Default #838

merged 1 commit into from
Mar 9, 2025

Conversation

burrbull
Copy link
Member

@burrbull burrbull commented May 9, 2024

Advantages:

  • more consts
  • zero is more semantically correct then default
  • cleaner code

Disadvantages:

  • rustdoc still not good with const defaults. Prints
    изображение

instead of const ZERO_TO_MODIFY_FIELDS_BITMAP: u32 = 0 for default value.

cc @Emilgardis

@burrbull burrbull requested a review from a team as a code owner May 9, 2024 04:37
@burrbull burrbull force-pushed the const-default branch 2 times, most recently from 96ad0c3 to f6246e3 Compare May 9, 2024 04:43
@burrbull

This comment was marked as outdated.

This comment was marked as outdated.

@burrbull burrbull changed the title use ConstDefault instead of Default use ConstZero instead of Default Dec 11, 2024
@burrbull burrbull force-pushed the const-default branch 5 times, most recently from d22e32e to 379a857 Compare December 14, 2024 17:31
@burrbull
Copy link
Member Author

/ci diff pr

Copy link

Diff for comment

@burrbull
Copy link
Member Author

@Emilgardis @adamgreig what do you think about this change?

@Emilgardis
Copy link
Member

Not sure i understand why num-traits is needed for this, could you elaborate?

@burrbull
Copy link
Member Author

burrbull commented Feb 13, 2025

Not sure i understand why num-traits is needed for this, could you elaborate?

We can introduce our own Zero trait if you are saying about this.

Emilgardis
Emilgardis previously approved these changes Mar 1, 2025
@burrbull

This comment was marked as outdated.

@burrbull burrbull force-pushed the const-default branch 2 times, most recently from 2dba2b6 to 806734c Compare March 2, 2025 11:54
@burrbull
Copy link
Member Author

burrbull commented Mar 2, 2025

cc @Emilgardis rebased

@burrbull
Copy link
Member Author

burrbull commented Mar 2, 2025

/ci diff pr

Copy link

github-actions bot commented Mar 2, 2025

Diff for comment

@burrbull
Copy link
Member Author

burrbull commented Mar 7, 2025

cc @Emilgardis let's merge

@Emilgardis
Copy link
Member

/ci diff semver

Copy link

github-actions bot commented Mar 7, 2025

Diff for comment

@Emilgardis
Copy link
Member

Emilgardis commented Mar 7, 2025

Semver break should be fine, implementations of RawReg shouldnt be done by other crates. That one() is missing should be fine too

@burrbull burrbull mentioned this pull request Mar 8, 2025
@burrbull burrbull added this pull request to the merge queue Mar 9, 2025
Merged via the queue into master with commit e100619 Mar 9, 2025
58 checks passed
@burrbull burrbull deleted the const-default branch March 9, 2025 03:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants