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

Reenable datatype invariants #686

Merged
merged 19 commits into from
May 19, 2022
Merged

Conversation

zgrannan
Copy link
Collaborator

No description provided.

@zgrannan zgrannan marked this pull request as ready for review October 8, 2021 20:04
@zgrannan zgrannan changed the title [WIP] Datatype invariants Datatype invariants Oct 15, 2021
@vakaras
Copy link
Contributor

vakaras commented Oct 16, 2021

@zgrannan I thought a bit about the possible implementation of type invariants and I think that the cleanest way of integrating them into the refactored system would be by renaming the new TypeDecl into TypeDeclKind and adding a new TypeDecl:

struct TypeDecl {
  kind: TypeDeclKind,
  user_invariant: Box<Expression>,
}

Let me know if you have any questions or need help.

@Aurel300
Copy link
Member

@vakaras Is the new TypeDecl a misnomer then? If these types are meant to be passed around similarly to ty::Ty or (the old) vir::Type, they are not declarations.

@fpoli
Copy link
Member

fpoli commented Oct 16, 2021

Why the Box? It seems unnecessary.

@vakaras
Copy link
Contributor

vakaras commented Oct 16, 2021 via email

@vakaras
Copy link
Contributor

vakaras commented Oct 16, 2021 via email

@fpoli
Copy link
Member

fpoli commented Oct 16, 2021

Why the Box? It seems unnecessary.
Yes, mixed up code.

If it turns out to be needed to cut a mutual recursion with the expression definition then I prefer it over Box<TypeDecl>.

@JonasAlaif
Copy link
Contributor

JonasAlaif commented Nov 5, 2021

So I've been thinking about this a little, particularly with adding "dependent parameters" expressed as const generics. The classic use case with a Type Alias is the following:

#[invariant(_ < s.len())]
type Index<const s: &[u32]> = usize;
fn foo(s: &[i32], i: Index<{s}>) -> i32 { s[i] }

Porting this from my work on Type Aliases to Datatypes wouldn't be so difficult, and it seems to be a super nice solution to the invariants-func.rs example:

#[invariant(self.invariants_enabled ==> self.value <= 100)]
struct Percentage {
    invariants_enabled: bool,
    value: u8,
}
fn print_percentage(percentage: &Percentage) { }

#[requires(percentage.invariants_enabled)]
fn adjust_percentage(percentage: &mut Percentage) {
    percentage.value += 10; // temporarily break invariant

    percentage.invariants_enabled = false;
    print_percentage(percentage);
    percentage.invariants_enabled = true;

    percentage.value -= 10; // restore invariant
}

Would instead be:

#[invariant(invariants_enabled ==> self.value <= 100)]
struct Percentage<const invariants_enabled: bool> {
    value: u8,
}
fn print_percentage(percentage: &Percentage<{false}>) { }

fn adjust_percentage(percentage: &mut Percentage<{true}>) {
    percentage.value += 10; // temporarily break invariant

    print_percentage(percentage);

    percentage.value -= 10; // restore invariant
}

However, my current solution requires the whole thing be wrapped in a prusti!{} macro to find all of the {true} and {false} arguments and hide them from the type system - even though the current Rust const generics support bool types. So an even nicer solution would be to try to write this as 'valid' Rust:

#![feature(const_generics_defaults)]

#[invariant(invariants_enabled ==> self.value <= 100)]
struct Percentage<const invariants_enabled: bool = true> {
    value: u8,
}
fn print_percentage(percentage: &Percentage<false>) {}

fn adjust_percentage(percentage: &mut Percentage) {
    percentage.value += 10; // temporarily break invariant

    print_percentage(percentage as &Percentage<false>);

    percentage.value -= 10; // restore invariant
}

The only snag is that we cannot percentage as &Percentage<false> - I don't know const generics well enough; the only thing I could come up with is creating a new instance: &Percentage::<false> { value: percentage.value }, but that kinda defeats the purpose.

Edit: @vakaras suggested &Percentage::<false> { ..percentage } which actually seems to me as a viable option. For us to support this the invariant macro would have to not panic when processing const generics struct S<const x: bool = true> {} and Prusti would have to include support for const generics.

@zgrannan
Copy link
Collaborator Author

@vakaras
I recall a few weeks ago this was blocked because the datatype invariants encoding uses:

pub fn encode_assertion(
&self,
assertion: &typed::Assertion<'tcx>,
mir: &mir::Body<'tcx>,
pre_label: Option<&str>,
target_args: &[vir::Expr],
target_return: Option<&vir::Expr>,
targets_are_values: bool,
assertion_location: Option<mir::BasicBlock>,
error: ErrorCtxt,
parent_def_id: ProcedureDefId,
tymap: &SubstMap<'tcx>,
) -> SpannedEncodingResult<vir::Expr> {
which returns a vir::Expr, but the type invariant encoder:
pub fn encode_invariant_def(self, invariant_name: &str) -> EncodingResult<vir::FunctionDecl> {
needs the body to be vir::Expression, and there is no way to go from polymorphic vir to high vir.

Is this still the case?

@vakaras
Copy link
Contributor

vakaras commented Nov 10, 2021

Is this still the case?

Yes. And I doubt that #756 will be enough to fix that.

@Aurel300 Aurel300 assigned Aurel300 and unassigned zgrannan Mar 2, 2022
@Aurel300 Aurel300 force-pushed the datatype-invariants branch from 68cce47 to 278fe66 Compare March 2, 2022 21:30
@Aurel300
Copy link
Member

Aurel300 commented Mar 2, 2022

I rebased this. I think the last comment still kind of applies. More specifically, I see:

  • mir::types::MirTypeEncoderInterface::encode_type_invariant_def_poly into which I rebased the implementation in this PR. This function returns a vir_poly function and is never used.
  • high::types::HighTypeEncoderInterface::encode_type_invariant_use/def which is currently a stub. The first one is actually used from the procedure encoder. However, even though these are in high/types, they also return a String and a vir_poly::FunctionIdentifier, respectively.

So… should I just remove the encode_type_invariant_def_poly and move it into high?

@vakaras
Copy link
Contributor

vakaras commented Mar 3, 2022

I rebased this. I think the last comment still kind of applies. More specifically, I see:

* `mir::types::MirTypeEncoderInterface::encode_type_invariant_def_poly` into which I rebased the implementation in this PR. This function returns a `vir_poly` function and is never used.

* `high::types::HighTypeEncoderInterface::encode_type_invariant_use/def` which is currently a stub. The first one is actually used from the procedure encoder. However, even though these are in `high/types`, they also return a `String` and a `vir_poly::FunctionIdentifier`, respectively.

So… should I just remove the encode_type_invariant_def_poly and move it into high?

Seems like that would be the easiest option.

@Aurel300 Aurel300 changed the title Datatype invariants Reenable datatype invariants Mar 9, 2022
@Aurel300 Aurel300 mentioned this pull request Mar 13, 2022
4 tasks
JonasAlaif added a commit to JonasAlaif/prusti-dev that referenced this pull request Mar 24, 2022
@JonasAlaif
Copy link
Contributor

It seems that after the rebase, things don't work at all atm (i.e. things behave as if there was no invariant annotation)

@Aurel300
Copy link
Member

@JonasAlaif I do have some WIP for this locally, but I want to wait until after #899, because invariants with generics were not nice.

@Aurel300 Aurel300 force-pushed the datatype-invariants branch from 37cb800 to 05d2592 Compare May 18, 2022 16:14
@Aurel300 Aurel300 force-pushed the datatype-invariants branch from 05d2592 to ca8ef2b Compare May 18, 2022 16:32
@Aurel300
Copy link
Member

bors r+

(@vakaras I'll add the trusted flag shortly)

@bors
Copy link
Contributor

bors bot commented May 19, 2022

@bors bors bot merged commit 4bbf5fd into viperproject:master May 19, 2022
bors bot added a commit that referenced this pull request May 24, 2022
901: NFM22 examples r=JonasAlaif a=Aurel300

This is to allow linking the examples online from the paper. PRs to be done before the deadline:

- [x] #686 
- [x] #882 
- [x] #887 
- [x] #899 


Co-authored-by: Aurel Bílý <aurel.bily@gmail.com>
Co-authored-by: Jonas <jonas.fiala.cardiff@gmail.com>
bors bot added a commit that referenced this pull request May 27, 2022
901: NFM22 examples r=Aurel300 a=Aurel300

This is to allow linking the examples online from the paper. PRs to be done before the deadline:

- [x] #686 
- [x] #882 
- [x] #887 
- [x] #899 


972: Fix slice with range encoding r=JonasAlaif a=JonasAlaif

Fixes #960
Issue is described there. Not sure if this is the correct way to do the error reporting?
It would be really nice to just have a contract for `index` and `index_mut` and report the error as a precondition violation of those. E.g. I really don't want to hardcode the pledge required for `index_mut` by hand.

Co-authored-by: Aurel Bílý <aurel.bily@gmail.com>
Co-authored-by: Jonas <jonas.fiala.cardiff@gmail.com>
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