-
Notifications
You must be signed in to change notification settings - Fork 184
PR related to #726 and #723 #727
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
Conversation
The module uses the intrinsic assignment operation, `=`, to create a | ||
duplicate of an original bitset. It additionally defines assignments to and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@degawa : should we add additional information based on the Discourse thread?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the late review.
The additional information based on the Discourse thread is optional because the information, like below, is too broad for describing the specification of the bitset assignment:
The user-defined assignment operator causes the self-assignment problem with the argument aliasing; since the type bitset_large
and bitset_64
do not have any pointer component, the intrinsic assignment operation should be used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@degawa Could you approve this PR, if ok for you, please? Then I will merge it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your PR for solving the problem related to the assignment operation.
Looks good to me and approve the PR.
@milancurcic could you review/approve/merge this PR, please @degawa is not a reviewer with write access and the PR is therefore is still blocked. |
As it's just an removal of a problematic intrinsic assignment overload, it's blocking the development of another PR (3 days), all tests are passing and it fixes the problem I will go ahead to approve and squash-merge. |
Thank you @14NGiestas for merging it. |
PR opened following the issue reported in #723 and #726 (reported by @degawa )
Fixes #726