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

Implement View and DeepView in slice and array #1078

Merged
merged 4 commits into from
May 8, 2024

Conversation

Chris-Hawblitzel
Copy link
Collaborator

This is related to #1070 .

@Chris-Hawblitzel
Copy link
Collaborator Author

At the moment, verusfmt complains about the trait syntax pub trait ArrayAdditionalSpecFns<T>: View<V = Seq<T>> { ... }.

Copy link
Contributor

@jaybosamiya jaybosamiya left a comment

Choose a reason for hiding this comment

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

Overall looks good to me, thanks! Minor comment added about a possible concern (I don't think it should be a blocker though). Also, I see you've already removed the now-unnecessary verusfmt::skip, thanks!

source/vstd/slice.rs Outdated Show resolved Hide resolved
Comment on lines +22 to +25
open spec fn deep_view(&self) -> Seq<T::V> {
let v = self.view();
Seq::new(v.len(), |i: int| v[i].deep_view())
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Just making a quick note here that @y1ca1 had some concerns (during a separate discussion) about this adding more triggers than explicitly reimplementing deep_view on each of the vstd data structures individually.

Personally I think that implementing deep_view separately on each of (say) [u8], [u32], [u64] and so on is not really a good idea (because it doesn't scale and would prevent usage on user-defined types without them manually also implementing deepview on [Foo]), and that we should just go with the current approach.

However, @y1ca1, if I've misunderstood what you meant, please do comment here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for mentioning this. What I meant by "explicitly reimplementing" deep_view is to provide separate implementation on each of the methods for the generic slice type[T] whose specs are described in deep_view. This would arguably save a lot of triggers if one uses deep_view ubiquitously, but yeah, definitely less ergonomic and harder to maintain.

// This axiom is slightly better than defining spec_slice_len to just be `[email protected]() as usize`
// (the axiom also shows that [email protected]() is in-bounds for usize)
#[verifier(external_body)]
pub broadcast proof fn axiom_spec_len<T>(slice: &[T])
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will now need to be added to the vstd_default broadcast group to be visible to users.

@gancherj
Copy link

gancherj commented May 6, 2024

Hi! Is it possible to get this PR merged this week? Thanks

@Chris-Hawblitzel Chris-Hawblitzel merged commit a3d91ba into main May 8, 2024
5 checks passed
@Chris-Hawblitzel Chris-Hawblitzel deleted the view-slice-array branch May 8, 2024 03:55
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.

5 participants