Skip to content

Add smart constructors for terms, and other improvements to fly/src/syntax.rs#131

Merged
odedp merged 17 commits into
mainfrom
more-smart-constructors
Jul 23, 2023
Merged

Add smart constructors for terms, and other improvements to fly/src/syntax.rs#131
odedp merged 17 commits into
mainfrom
more-smart-constructors

Conversation

@odedp
Copy link
Copy Markdown
Contributor

@odedp odedp commented Jul 21, 2023

Add smart constructors for Sort, Binder, and all Term cases. The smart constructors can take either something owned, and then they don't clone, or a reference, and then they clone. This is meant to make code for programmatically constructing Terms nicer, without a bunch of .clone everywhere. In a few (documented) cases, the smart constructors do a bit more normalization (e.g., Term::and([]) returns true and not an actual Term::And).

For now, these smart constructors are only used in code I still haven't pushed (in the liveness to safety), but I think they should be used everywhere. Basically, there's no reason to directly construct a Term.

To convert existing code, change the constructor to a smart constructor, and try to replace arg.clone() by &arg to make calling code look nicer.

Other small changes:

  • Rename Sort::Id to Sort::Uninterpreted
  • Rename UOp::Previously to UOp::Previous, which I think is more standard ("previously" is sometimes used for the past version of eventually)
  • Documentation improvements

Minor issues and future TODOs:

  • Binders passed to forall and exist still need to be owned. We can change this by having impl From<&Binder> for <Binder> similarly to what we do for Sort and Term.
  • Term::and and Term::or take an IntoIterator which forces uniformity. So you cannot do Term::and([&t1, t2]) (where you can do Term::iff(&t1, t2), and must instead do Term::and([&t1, &t2]) which introduces an unnecessary clone. I don't think this clone gets optimized away. If we think this is a concern, we can implement variadic and and or, either with specialized functions (say for 2-9 arguments) or perhaps with a macro.

@odedp odedp force-pushed the more-smart-constructors branch 3 times, most recently from 1a72dff to bfbbaee Compare July 21, 2023 17:13
Comment thread fly/src/syntax.rs Outdated
Comment thread fly/src/syntax.rs Outdated
/// all the functions in the signature at each point in time).
/// A Term is an FO-LTL (first-order linear temporal logic) term or formula. The
/// temporal operators supported are: Prime, Next, Prev, Until, Since, Always,
/// Eventually (see [`UOp`] and [`BinOp`]).
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't like this change to the documentation. I'd like to have some basic explanation of LTL here, and I don't think it's important to list the operators here (leave that for the language reference).

Copy link
Copy Markdown
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 like the previous explanation, and I'm not sure the basic semantics of LTL should be explained in the documentation of Term. At some point, I think the language manual or other documentation should explain the semantics of FO-LTL. And I did find it helpful (even for myself) to list all the temporal operators here.

What do you think about the following compromise:

/// A Term is an FO-LTL (first-order linear temporal logic) term or formula. The
/// temporal operators supported are: Prime, Next, Prev, Until, Since, Always,
/// Eventually (see [`UOp`] and [`BinOp`]).
///
/// FO-LTL is an extension of first-order logic where the semantics is given in
/// terms of infinite sequences of models (over a shared universe). A term is
/// interpreted at a particular point (time) in the sequence, and using
/// temporal operators it can also query the past or future. For example
/// exists x. p(x) & (previous !p(x)) & always r(x) means that there exists
/// some element x for which p now holds, but it didn't hold a moment ago,
/// and from this point onwards r keeps holding for x.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Sure, that looks good to me.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Great. I pushed a slightly edited version of this.

Comment thread fly/src/syntax.rs Outdated
Comment thread fly/src/syntax.rs Outdated
@vmwclabot
Copy link
Copy Markdown

@odedp, you must sign every commit in this pull request acknowledging our Developer Certificate of Origin before your changes are merged. This can be done by adding Signed-off-by: John Doe <john.doe@email.org> to the last line of each Git commit message. The e-mail address used to sign must match the e-mail address of the Git author. Click here to view the Developer Certificate of Origin agreement.

@vmwclabot

This comment was marked as duplicate.

@odedp odedp force-pushed the more-smart-constructors branch from f790c06 to af08d3e Compare July 22, 2023 05:33
@vmwclabot

This comment was marked as duplicate.

@odedp odedp force-pushed the more-smart-constructors branch from af08d3e to b15d890 Compare July 22, 2023 05:39
@odedp odedp force-pushed the more-smart-constructors branch from e18a80b to 69b674f Compare July 22, 2023 05:54
@odedp odedp changed the title Smart constructors and other improvements to fly/src/syntax.rs Add smart constructors for terms, and other improvements to fly/src/syntax.rs Jul 22, 2023
tchajed and others added 15 commits July 22, 2023 22:58
Signed-off-by: Tej Chajed <tchajed@vmware.com>
Signed-off-by: Tej Chajed <tchajed@vmware.com>
TODO: Use the smart constructors everywhere. I think Terms should not
be constructed without them (except for some unusual cases perhaps).

Signed-off-by: Oded Padon <oded.padon@gmail.com>
…e for Term

Signed-off-by: Oded Padon <oded.padon@gmail.com>
Signed-off-by: Oded Padon <oded.padon@gmail.com>
Signed-off-by: Oded Padon <oded.padon@gmail.com>
Signed-off-by: Tej Chajed <tchajed@vmware.com>
Signed-off-by: Tej Chajed <tchajed@vmware.com>
Signed-off-by: Tej Chajed <tchajed@vmware.com>
…rt::Uninterpreted)

Signed-off-by: Oded Padon <oded.padon@gmail.com>
Signed-off-by: Oded Padon <oded.padon@gmail.com>
Co-authored-by: Tej Chajed <tchajed@vmware.com>
Signed-off-by: Oded Padon <oded.padon@gmail.com>
Co-authored-by: Tej Chajed <tchajed@vmware.com>
Signed-off-by: Oded Padon <oded.padon@gmail.com>
…ifier

Signed-off-by: Oded Padon <oded.padon@gmail.com>
Signed-off-by: Oded Padon <oded.padon@gmail.com>
odedp added 2 commits July 22, 2023 22:58
…nknown

Signed-off-by: Oded Padon <oded.padon@gmail.com>
Signed-off-by: Oded Padon <oded.padon@gmail.com>
@odedp odedp force-pushed the more-smart-constructors branch from 7bfb07b to 7873ebb Compare July 23, 2023 05:59
@odedp odedp merged commit 3b59347 into main Jul 23, 2023
@odedp odedp deleted the more-smart-constructors branch July 23, 2023 06:21
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.

3 participants