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

Add is_const_eval intrinsic #959

Closed
wants to merge 1 commit into from
Closed

Conversation

gnzlbg
Copy link

@gnzlbg gnzlbg commented Sep 22, 2019

This adds support for an is_const_eval intrinsic that returns true during constant evaluation.

@bjorn3
Copy link
Member

bjorn3 commented Sep 22, 2019

commit message: *intrinsic

@@ -223,6 +223,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
let discr_val = this.read_discriminant(place.into())?.0;
this.write_scalar(Scalar::from_uint(discr_val, dest.layout.size), dest)?;
}
"is_const_eval" => {
this.write_scalar(Scalar::from_u8(1), dest)?;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is correct from the what we can do perspective, but that means the naming of the intrinsic is wrong.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Idk anything better though

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In C++ this is called is_constant_evaluated.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this code even be executed? We usually prefer the librustc_mir implementation of an intrinsic, and only fall back here when none exists there.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I started working on this, I started here and not in rust-lang/rust because I thought that rust-lang/rust would call this library to evaluate the intrinsics. Now I am not sure if this makes sense.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see. No, that's not how the code is organized. The code in this repo is only used by Miri-the-tool; rustc (and its test suite) to not use the code here at all.

@gnzlbg
Copy link
Author

gnzlbg commented Sep 23, 2019

So I'm closing this, I misunderstood how the code was structured.

@gnzlbg gnzlbg closed this Sep 23, 2019
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.

4 participants