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

Level finder #811

Merged
merged 8 commits into from
May 6, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions UNRELEASED.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

* Docker: Use openjdk-16 for the runtime in the app image, see #807
* Documentation: Introduce TypeAlises operator convention, see #808
* tlair: TlaExLevelFinder and TlaDeclLevelFinder to compute TLA+ levels, see #811

### Bug fixes

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
package at.forsyte.apalache.tla.lir

/**
* This class computes the level of a TLA+ declaration by using TlaExLevelFinder. See Lamport. Specifying Systems, p. 322.
*
* @param module a TLA module that contains the declarations to query
* @author Igor Konnov
*/
class TlaDeclLevelFinder(module: TlaModule) {
private val consts = Set(module.constDeclarations.map(_.name): _*)
private val vars = Set(module.varDeclarations.map(_.name): _*)
private val defs = Map(module.operDeclarations map { d => d.name -> d }: _*)
private var levelCacheMap: Map[String, TlaLevel] = Map()

def apply(decl: TlaDecl): TlaLevel = {
decl match {
case TlaConstDecl(_) =>
TlaLevelConst

case TlaVarDecl(_) =>
TlaLevelState

case TlaAssumeDecl(_) =>
TlaLevelConst

Comment on lines +17 to +25
Copy link
Contributor

Choose a reason for hiding this comment

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

unless I missunderstand, it seems like where checking for this and then determining the level in two parallel ways, Once here by pattern matching, and once when we construct the level finder, when we cache to the various sets. Couldn't we handle all of this logic with the _.contains checks and the caching lookup?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The code in apply is also handling the cases of ASSUME and THEOREM. Also, this code avoids an extra lookup in defs, consts and vars (but that's not the main point).

Copy link
Collaborator

Choose a reason for hiding this comment

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

What happens if I write an assume declaration with primes?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

SANY will tell you that it does not like your spec. I found that out a couple of months ago :D

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It will even complain if you use state variables in ASSUME.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, what about the expression (1 + 1)' outside of assume? Just because it has a prime operator, does not mean its level isn't 0.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Well, syntactically (1 + 1)' is level 2. Semantically, it is level 0. It looks like the book is not precise about whether we should compute the levels semantically or syntactically.

case TlaOperDecl(name, _, _) =>
cacheLevel(Set(name), name)
levelCacheMap(name)
Copy link
Contributor

Choose a reason for hiding this comment

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

Given that we are caching results, why don't we first check whether the name is cached before we try to add it to the cache?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yep, added that


case TlaTheoremDecl(_, _) =>
TlaLevelConst
}
}

private def cacheLevel(callers: Set[String], name: String): TlaLevel = {
def levelOfName(name: String): TlaLevel = {
if (consts.contains(name)) {
TlaLevelConst
} else if (vars.contains(name)) {
TlaLevelState
} else {
if (!callers.contains(name) && defs.contains(name)) {
// as the module comes from the parser, we assume that defs contains a definition for the name `name`
levelCacheMap.get(name) match {
case Some(level) => level

case None => cacheLevel(callers + name, name)
}
} else {
TlaLevelConst
}
}
}

val level = new TlaExLevelFinder(levelOfName)(defs(name).body)
levelCacheMap += name -> level
level
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package at.forsyte.apalache.tla.lir

import at.forsyte.apalache.tla.lir.oper.{TlaActionOper, TlaTempOper}

/**
* This class computes the level of a TLA+ expression. See Lamport. Specifying Systems, p. 322.
*
* @param nameLevel a function that returns the level of a name.
* @author Igor Konnov
*/
class TlaExLevelFinder(nameLevel: String => TlaLevel) {
def apply(ex: TlaEx): TlaLevel = find(ex)

private def find: TlaEx => TlaLevel = {
case NameEx(name) =>
nameLevel(name)

case OperEx(op, args @ _*) =>
val level = TlaExLevelFinder.levelOfOper.getOrElse(op, TlaLevelConst)
level.join(args.map(find))

case LetInEx(body, defs @ _*) =>
find(body).join(defs.map(d => find(d.body)))

case _ =>
TlaLevelConst
Comment on lines +25 to +26
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this catchall come back to bite us if new expressions are introduced? Would it be worth while to explicitly enumerate the other constructors here instead of having a wildcard?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I doubt that, as the types of expressions were stable for long time.

}
}

object TlaExLevelFinder {
private val levelOfOper = Map(
TlaActionOper.prime -> TlaLevelAction,
TlaActionOper.enabled -> TlaLevelAction,
TlaActionOper.unchanged -> TlaLevelAction,
TlaActionOper.stutter -> TlaLevelAction,
TlaActionOper.nostutter -> TlaLevelAction,
TlaActionOper.composition -> TlaLevelAction,
TlaTempOper.box -> TlaLevelTemporal,
TlaTempOper.diamond -> TlaLevelTemporal,
TlaTempOper.leadsTo -> TlaLevelTemporal,
TlaTempOper.weakFairness -> TlaLevelTemporal,
TlaTempOper.strongFairness -> TlaLevelTemporal,
TlaTempOper.guarantees -> TlaLevelTemporal,
TlaTempOper.AA -> TlaLevelTemporal,
TlaTempOper.EE -> TlaLevelTemporal
)
}
93 changes: 93 additions & 0 deletions tlair/src/main/scala/at/forsyte/apalache/tla/lir/TlaLevel.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
package at.forsyte.apalache.tla.lir

/**
* The level of a TLA+ expression, as explained in: Lamport. Specifying Systems, p. 322.
*/
sealed trait TlaLevel extends Ordered[TlaLevel] {

/**
* The level number as assigned in the book.
*/
val level: Int

/**
* A textual representation of the level.
*/
val name: String

/**
* Compare this level to another level.
*
* @param that the level to compare
* @return 0, if the levels are equal; a negative number if `this` is smaller than `that`, and a positive number
* if `this` is larger than `that`.
*/
override def compare(that: TlaLevel): Int = {
level - that.level
}

/**
* Join the levels by computing the maximum of the levels of `this` and `that`.
*
* @param that the level to join with
* @return the minimal level j that satisfies: `j <= this` and `j <= that`.
*/
def join(that: TlaLevel): TlaLevel = {
TlaLevel.fromInt(Math.max(level, that.level))
}

/**
* Join the level with a sequence of level.
*
* @param otherLevels a sequence of levels
* @return the join of `this` and otherLevels
*/
def join(otherLevels: Seq[TlaLevel]): TlaLevel = {
otherLevels.foldLeft(this) { case (l, r) => l.join(r) }
}
}

object TlaLevel {
private val INT_TO_LEVEL = Seq(TlaLevelConst, TlaLevelState, TlaLevelAction, TlaLevelTemporal)

def fromInt(levelNo: Int): TlaLevel = {
INT_TO_LEVEL.find(_.level == levelNo) match {
Comment on lines +51 to +54
Copy link
Collaborator

Choose a reason for hiding this comment

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

Minor gripe,I would use an indexed collection and do boundary tests here, instead of find.

Copy link
Collaborator Author

@konnov konnov May 6, 2021

Choose a reason for hiding this comment

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

I've replaced it with find, which should be ok for a list of 4 elements

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ha-ha. My solution was exactly as you suggest, and @shonfeder did not like it :P

case Some(level) => level

case None =>
throw new IllegalArgumentException(s"Unexpected level: $levelNo")
}
}
}

/**
* Constant level: constants and constant-level expressions.
*/
case object TlaLevelConst extends TlaLevel {
override val level: Int = 0
override val name: String = "Constant"
}

/**
* State level: the constant-level, state variables, and expressions over them that are not actions or temporal formulas.
*/
case object TlaLevelState extends TlaLevel {
override val level: Int = 1
override val name: String = "State"
}

/**
* Action level: the state level, state variables, primed state variables, and action operators.
*/
case object TlaLevelAction extends TlaLevel {
override val level: Int = 2
override val name: String = "Action"
}

/**
* Temporal level: the action level and temporal formulas.
*/
case object TlaLevelTemporal extends TlaLevel {
override val level: Int = 3
override val name: String = "Temporal"
}
Loading