Skip to content

Conversation

@ana-pantilie
Copy link
Contributor

Part of #1278

After #2007


Reviewer checklist
  • Test coverage: stack test --coverage
  • Public API documentation: stack haddock

ana-pantilie and others added 30 commits July 24, 2020 13:53
instance From OnePathRule Attribute.Trusted where
from = Attribute.trusted . attributes . getOnePathRule

instance From OnePathRule (TermLike VariableName) where
Copy link
Contributor

Choose a reason for hiding this comment

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

What term is this giving?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's the entire claim as a term-like. This is related to the From rule (TermLike VariableName) constraint used when logging (we need this otherwise we wouldn't be able to print the rule in the log). I think that discussion got lost amidst everything else. The first thing that came to mind was to use a newtype over TermLike VariableName so that it's clearer this is the whole rule represented as a term. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, we could have something like

newtype AxiomPattern = AxiomPattern (TermLike VariableName)

{ left =
Pattern.mapVariables resetConfigVariable left
, right =
Pattern.mapVariables resetConfigVariable <$> right
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be MultiOr.map instead of fmap?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's right. Thanks for spotting this!

Copy link
Contributor

Choose a reason for hiding this comment

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

Is it safe to remove the Functor instance of that type yet?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are still places where it's used. I was thinking it would be cleaner to do this as another task.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good! I can take care of that; there were several clean-up items that I wanted to do, that should be postponed until #2080 is merged.

@rv-jenkins rv-jenkins merged commit 06be983 into runtimeverification:master Aug 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants