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

Revisit usize / u64 use for resource mappings in context of index #29

Open
nilslice opened this issue Apr 15, 2022 · 0 comments
Open

Revisit usize / u64 use for resource mappings in context of index #29

nilslice opened this issue Apr 15, 2022 · 0 comments

Comments

@nilslice
Copy link
Contributor

Considering some existing code that logically returns values as u64:

/// Attempt to cast a pointer to an immediate int and return that value if successful
pub(crate) fn pointer_value_to_u64(
context: &QCSCompilerContext,
value: PointerValue,
) -> Option<u64> {
value
.const_to_int(context.base_context.i64_type())
.get_sign_extended_constant()
.and_then(|value| u64::try_from(value).ok())
}

"Result" => {
let result_index = pointer_value_to_u64(context, ptr_value)
.ok_or_else(|| {
eyre!("unable to derive Result index from pointer")
})?;
Ok(OperationArgument::Result(result_index))
}

These values are regularly re-used elsewhere and need to be cast to usize, which technically can cause truncation, e.g.

#[allow(clippy::cast_possible_truncation)]
let index = *index as usize;
if let Some(result) = shot.get(index) {
output.push(format!("[shot:{} result {}]", shot_id, result));
} else {
return Err(Error::NoShotDataAtIndex(shot_id, index));
}

While it's not a major issue now, it's worth re-visiting in the future. This issue tracks the intent to do so.

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

No branches or pull requests

1 participant