-
Notifications
You must be signed in to change notification settings - Fork 136
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
Merge master
into next
#344
Conversation
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.
Looks good, thanks!
The problem is that asm_const is still unstable, so I added a new feature asm_const to solve that.
Hmm, so we will have support for generating software interrupts on stable in the next 0.14 release, but version 0.15 removes that support again? This does not seem like a good solution. Perhaps we should keep providing the macro as an alternative on stable until asm_const
is stablized?
Good point, I added the macro back. |
/// Generate a software interrupt by invoking the `int` instruction. | ||
/// | ||
/// This currently needs to be a macro because the `int` argument needs to be an | ||
/// immediate. This macro will be replaced by a generic function when support for | ||
/// const generics is implemented in Rust. | ||
#[macro_export] | ||
macro_rules! software_interrupt { | ||
($x:expr) => {{ | ||
asm!("int {id}", id = const $x, options(nomem, nostack)); | ||
}}; | ||
} | ||
|
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.
I don't think keeping this macro around actually lets us call software_interrupt!(0x42)
on stable. At least when I try to do this, I still get an error about asm_const
. I don't think 0.14 has a stable way to do this, so we don't lose anything by only having the software_interrupt
function.
In fact, having the function is slightly nicer, as the x86_64 crate can enable the feature for the user, rather than the user having to manually deal with 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.
Oh yeah, you're right.
You can get it to work if you're willing to commit some macro crimes ;)
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.
That marco approach is interesting, but I think it might lead to confusing behavior. For example, this works
software_interrupt!(2*3 + 7 - 2*3);
but this doesn't:
const N: u8 = 5;
software_interrupt!(N);
nor does this:
software_interrupt!(5u8);
So having this might be confusing to users.
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.
I'm fine with removing the macro, given that it's apparently unusable in its current form on stable.
If there is demand for it, we can still add a stable-compatible macro at a later time, but I would wait until it is requested.
This pr merges the
master
branch into thenext
branch.There was one non-obvious merge conflict:
#259 activated another rust nightly feature
asm_const
whenever theinline_asm
was active.Inline assembly was recently stabilized, so #343 deprecated the
inline_asm
andexternal_asm
features. The problem is thatasm_const
is still unstable, so I added a new featureasm_const
to solve that.