Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
TL; DR
As talked about offline with @bclement-ocp (cc Issue #209 ), there is small bug in the current handling of attributes in nested binders, where some attributes were duplicated before PR #207, and are now instead ignored/dropped in some cases.
For instance, if one has explicit nested forall quantifiers, and one of the intermediates quantifiers (which is therefore the body of another quantifier) has some pattern/trigger annotations, those were duplicated before (and therefore in released versions of dolmen) and are dropped on the current
master
branch. This PR fixes all of that.More details
The bug is due to the typechecker of Dolmen trying to collapse together nested quantifiers. For instance if one writes
forall x : Int. forall y : Int. x = y
, then dolmen tries to treat that the same as it was written asforall x : Int, y : int. x = y
. This is marginally useful in the quantifier case, but can save a lot of memory when dealing with let-bindings, since this allows to allocate a single expression node to bindn
variables, instead ofn
expression nodes.The functions that perform this collapsing have had rougly two behaviours:
While the dropping of attributes is undeniably worse that duplicating them, this PR solves the two problems and makes it so that attributes are only typed once and attached to the same formula as in the source. This is done relatively easy by simply not doing the collapsing if there are any attributes on the intermediate bindings formulas.
To help in debugging situations such as this, the first commit of this PR introduces a way to print the tags of typed expressions (which was not done/easy to do before). This required to change the implementation of heterogeneous maps, which is actually not bad since we now depend on dbunzli's hmap package instead of copying the container implementation.