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

Lean: Update models with the datetime extension #558

Merged
merged 34 commits into from
Feb 28, 2025

Conversation

adpaco-aws
Copy link
Contributor

Description of changes: Update the Lean models with the datetime extension, including:

  • Changes to the evaluator and validator to work with datetime types and methods.
  • Fixes existing theorems to work with datetime types and methods.
  • Adds new theorems for datetime types and methods (two for each constructor/method).

Signed-off-by: Adrian Palacios <accorell@amazon.com>
Signed-off-by: Adrian Palacios <accorell@amazon.com>
Signed-off-by: Adrian Palacios <accorell@amazon.com>
Signed-off-by: Adrian Palacios <accorell@amazon.com>
Signed-off-by: Adrian Palacios <accorell@amazon.com>
Signed-off-by: Adrian Palacios <accorell@amazon.com>
Signed-off-by: Adrian Palacios <accorell@amazon.com>
Signed-off-by: Adrian Palacios <accorell@amazon.com>
Signed-off-by: Adrian Palacios <accorell@amazon.com>
Signed-off-by: Adrian Palacios <accorell@amazon.com>
Signed-off-by: Adrian Palacios <accorell@amazon.com>
Signed-off-by: Adrian Palacios <accorell@amazon.com>
Signed-off-by: Adrian Palacios <accorell@amazon.com>
Signed-off-by: Adrian Palacios <accorell@amazon.com>
Signed-off-by: Adrian Palacios <accorell@amazon.com>
Signed-off-by: Adrian Palacios <accorell@amazon.com>
Signed-off-by: Adrian Palacios <accorell@amazon.com>
Signed-off-by: Adrian Palacios <accorell@amazon.com>
Signed-off-by: Adrian Palacios <accorell@amazon.com>
Signed-off-by: Adrian Palacios <accorell@amazon.com>
Signed-off-by: Adrian Palacios <accorell@amazon.com>
Signed-off-by: Adrian Palacios <accorell@amazon.com>
Signed-off-by: Adrian Palacios <accorell@amazon.com>
d₁.val ≤ d₂.val

instance : LT Datetime where
lt := fun d₁ d₂ => Datetime.lt d₁ d₂
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: it could be (Datetime.lt · ·)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't know that, thanks! However, I think I'll keep like this since it's more beginner-friendly and we use the same style for IpAddr defintions.

Copy link
Contributor

@shaobo-he-aws shaobo-he-aws left a comment

Choose a reason for hiding this comment

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

LGTM

Signed-off-by: Adrian Palacios <accorell@amazon.com>
Signed-off-by: Adrian Palacios <accorell@amazon.com>
Signed-off-by: Adrian Palacios <accorell@amazon.com>
Signed-off-by: Adrian Palacios <accorell@amazon.com>
Signed-off-by: Adrian Palacios <accorell@amazon.com>
@adpaco-aws
Copy link
Contributor Author

Thanks for the reviews! Ci was failing with errors like this:

error: ././././UnitTest/Datetime.lean:170:59: failed to synthesize
  Neg Duration
Additional diagnostic information may be available using the `set_option diagnostics true` command.
error: ././././UnitTest/Datetime.lean:171:62: failed to synthesize
  OfNat Duration 60000
numerals are polymorphic in Lean, but the numeral `60000` cannot be used in a context where the expected type is
  Duration
due to the absence of the instance above
Additional diagnostic information may be available using the `set_option diagnostics true` command.

when running the unit tests, so I added the instances which are infallible and I don't think they should be used for anything else other than the testing code (noted this in a comment).

I'm also planning on ignoring the DCO check because rebasing here could be complicated.

intro h₁
cases h₁
rename_i x h₁
simp [InstanceOfExtType] at h₁
Copy link
Contributor

Choose a reason for hiding this comment

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

If a simp occurs in the middle of a proof, we want to replace with simp only for robustness in the face of updates.

To figure out what needs to go into a simp only call, replace a simp with simp? in the IDE, and the Lean will give you a suggestion for how to replace that simp with simp only.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added only to most of the simp occurring anywhere other than the "tail" of a proof.

There's a couple of simps that cannot be converted to simp only without inducing errors. It's the only simp statement in the type_of_call_offset_is_sound and type_of_call_durationSince_is_sound theorems. I couldn't fix them - I suppose that's because simp? is not working well in those cases?

Copy link
Contributor

Choose a reason for hiding this comment

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

You can leave them for now. Usually, the tricky case for converting to simp only is when you're replacing a simp that is applied to multiple goals simultaneously. In that case, you have to put the union of the suggestions into simp only by manually copying-pasting, which can get annoying.

@emina
Copy link
Contributor

emina commented Feb 28, 2025

when running the unit tests, so I added the instances which are infallible and I don't think they should be used for anything else other than the testing code (noted this in a comment).

Can you point me to where in the code these are?

Copy link
Contributor Author

@adpaco-aws adpaco-aws left a comment

Choose a reason for hiding this comment

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

Can you point me to where in the code these are?

See comment above.

Signed-off-by: Adrian Palacios <accorell@amazon.com>
Signed-off-by: Adrian Palacios <accorell@amazon.com>
Signed-off-by: Adrian Palacios <accorell@amazon.com>
Copy link
Contributor

@emina emina left a comment

Choose a reason for hiding this comment

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

LGTM!

@adpaco-aws adpaco-aws merged commit 83b1c3e into cedar-policy:main Feb 28, 2025
5 of 6 checks passed
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