-
-
Notifications
You must be signed in to change notification settings - Fork 40
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
Quint -> TLA+ transpilation fixes #3041
Changes from 9 commits
b6d8824
6f61e23
65686e1
8056710
cb5ee57
bb746fc
ce5e61e
f28e16e
7b4e7db
9598c3e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Fixed a few problems on parsing Quint and pretty printing TLA, see #3041 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -297,14 +297,6 @@ class PrettyWriter( | |
text("{") <> nest(line <> binding <> ":" <> nest(line <> filter)) <> line <> text("}") | ||
) /// | ||
|
||
// a function of multiple arguments that are packed into a tuple: don't print the angular brackets <<...>> | ||
case OperEx(op @ TlaFunOper.app, funEx, OperEx(TlaFunOper.tuple, args @ _*)) => | ||
val argDocs = args.map(exToDoc(op.precedence, _, nameResolver)) | ||
val commaSeparatedArgs = folddoc(argDocs.toList, _ <> text(",") <@> _) | ||
group( | ||
exToDoc(TlaFunOper.app.precedence, funEx, nameResolver) <> brackets(commaSeparatedArgs) | ||
) /// | ||
|
||
// a function of a single argument | ||
case OperEx(TlaFunOper.app, funEx, argEx) => | ||
group( | ||
|
@@ -473,14 +465,19 @@ class PrettyWriter( | |
wrapWithParen(parentPrecedence, op.precedence, group(doc)) | ||
|
||
case OperEx(op @ TlaOper.apply, NameEx(name), args @ _*) => | ||
val (decls, newArgs) = extractDecls(args) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you have an example of what does not work exactly? For instance, this one works in TLC: https://gist.github.com/konnov/e7df8586a9865027dc4abfc07188e060 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh interesting, I only realize this now, but this problem only happens with high-order operators. Here's an example that doesn't work: --------------------------------- MODULE test ---------------------------------
EXTENDS Integers
VARIABLES z
F(f(_), x) == f(x)
G(x) == F(LET H(y) == y * 2 IN H, x)
Init ==
z = G(3)
Next ==
UNCHANGED z
============================================================================= TLC Errors:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right. This is because operators are not values in TLA+ |
||
|
||
// apply an operator by its name, e.g., F(x) | ||
val argDocs = args.map(exToDoc(op.precedence, _, nameResolver)).toList | ||
val argDocs = newArgs.map(exToDoc(op.precedence, _, nameResolver)).toList | ||
val commaSeparated = ssep(argDocs, "," <> softline) | ||
|
||
val doc = | ||
if (args.isEmpty) { | ||
text(parseableName(name)) | ||
} else { | ||
} else if (decls.isEmpty) { | ||
group(parseableName(name) <> parens(commaSeparated)) | ||
} else { | ||
group(ssep(decls, line) <> line <> parseableName(name) <> parens(commaSeparated)) | ||
} | ||
|
||
wrapWithParen(parentPrecedence, op.precedence, doc) | ||
|
@@ -494,16 +491,20 @@ class PrettyWriter( | |
wrapWithParen(parentPrecedence, op.precedence, doc) | ||
|
||
case OperEx(op, args @ _*) => | ||
val argDocs = args.map(exToDoc(op.precedence, _, nameResolver)).toList | ||
val (decls, newArgs) = extractDecls(args) | ||
val argDocs = newArgs.map(exToDoc(op.precedence, _, nameResolver)).toList | ||
val commaSeparated = ssep(argDocs, "," <> softline) | ||
// The standard operators may contain a '!' that refers to the standard module. Remove it. | ||
val lastIndexOfBang = op.name.lastIndexOf("!") | ||
val unqualifiedName = if (lastIndexOfBang < 0) op.name else (op.name.substring(lastIndexOfBang + 1)) | ||
|
||
val doc = | ||
if (args.isEmpty) { | ||
text(unqualifiedName) | ||
} else { | ||
} else if (decls.isEmpty) { | ||
group(unqualifiedName <> parens(commaSeparated)) | ||
} else { | ||
group(ssep(decls, line) <> line <> unqualifiedName <> parens(commaSeparated)) | ||
} | ||
|
||
wrapWithParen(parentPrecedence, op.precedence, doc) | ||
|
@@ -527,6 +528,25 @@ class PrettyWriter( | |
} | ||
} | ||
|
||
/** | ||
* On TLA+ files, we can't have LET..IN expressions as arguments. This is a helper function to extract the | ||
* declarations so they can be printed before the operator application that would use them as arguments. | ||
Comment on lines
+532
to
+533
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you mean something like this? LET A(x, y) == x + y IN F(A) Actually, this one should work if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure if I understand the question. I'm saying this one works: LET A(x, y) == x + y IN F(A) while this one doesn't: F(LET A(x, y) == x + y IN A) |
||
*/ | ||
def extractDecls(exprs: Seq[TlaEx]): (List[Doc], Seq[TlaEx]) = { | ||
val decls = exprs.collect { | ||
case LetInEx(_, TlaOperDecl("LAMBDA", _, _)) => Nil | ||
case LetInEx(_, decls @ _*) => decls | ||
case _ => Nil | ||
} | ||
val newArgs = exprs.collect { | ||
case expr @ LetInEx(_, TlaOperDecl("LAMBDA", _, _)) => expr | ||
case LetInEx(body, _) => body | ||
case expr => expr | ||
} | ||
|
||
(decls.flatten.map(d => group("LET" <> space <> declToDoc(d) <> line <> "IN")).toList, newArgs) | ||
} | ||
|
||
/** | ||
* Pretty-writes the given decl, while replacing names with the values in the NameReplacementMap. Then, wrap the | ||
* result as a comment, since the substituted names might not be valid TLA. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we have to remove this? In TLA+,
f[x, y]
is a short-form forf[<<x, y>>]
. Look at this example: https://gist.github.com/konnov/ce0ca9071dbe75b936e96dbd99224f0dThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does work like that for tuples, but not for sequences:
TLC fails to check Inv2 here:
I believe there is no way to disambiguate between tuples and sequences at this point, so the only option was to remove it completely.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting. I think the short-form syntax only applies to pairs, triples, etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, these invariants work for your example:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah interesting. Yeah, the problem I hit was on a list of one element (from the hashes you wrote in ICS23 btw).