-
Notifications
You must be signed in to change notification settings - Fork 287
1/3 mips: Convert rustc_args_required_const(0) functions to const generics #1028
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
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Amanieu (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
Can you change your code to use uppercase names for const generics? I am changing this for existing code in #1035. |
☔ The latest upstream changes (presumably 46efde1) made this pull request unmergeable. Please resolve the merge conflicts. |
}; | ||
} | ||
|
||
macro_rules! static_assert_imm_s13 { |
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.
For the intrinsics using these there are 2 conditions:
- Range is
-4096
to4095
. - Immediate is a multiple of 8.
It may be better to split these 2 conditions into separate static_assert
s.
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.
This macro *_s{5,11,12,13}
are unused in this PR, but they will be used in patch 2 and 3.
I will add static_assert_multiply_of
macro for asserting multiple issue.
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.
Just use a normal static_assert!
for that. The specific static_assert_imm*
macros are mainly used to avoid duplication.
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.
Moved *imm_s to subsequent patches.
☔ The latest upstream changes (presumably 9dc65c7) made this pull request unmergeable. Please resolve the merge conflicts. |
CI should be fixed, you need to rebase. |
I'm away from computer in a few weeks. If anybody wants to take off this PR, feels free. |
☔ The latest upstream changes (presumably 37dbe1c) made this pull request unmergeable. Please resolve the merge conflicts. |
I intended to work on mips in many small PRs for easier reviewing.
cc #1022