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

AddKoreAttribute as a Frontend Kompiler Pass #3522

Merged
merged 19 commits into from
Jul 26, 2023
Merged

Conversation

Robertorosmaninho
Copy link
Collaborator

@Robertorosmaninho Robertorosmaninho commented Jul 18, 2023

Part of #3444

This PR creates a new compiler pass that adds the Kore attributes to productions before ModuleToKore. This is part of the effort to refactor the ModuleToKore infrastructure to simplify it by decoupling functions and actions that can be done before the translation and that aren't related to (kore) code emission.

Besides extracting the AddKoreAttrubutes function from ModuleToKore.java, this PR also simplifies function calls such as isFunctional and isConstructor only to test if the attribute is present on the given production.

@Robertorosmaninho
Copy link
Collaborator Author

I'm currently blocked on the two pending issues mentioned above.
I'm testing it with this test as it was one of the first from regression-new to fail within this implementation.

The definition.kore reflect the second issue, as the symbols don't contain the appropriate attributes inserted by the AddKoreAttributes pass.

@dwightguth
Copy link
Collaborator

As far as the issue with the module transformer is concerned, it is a known issue that passes that only edit the attributes of sentences in a module get cached incorrectly by the code that helps cache module transformations, as a result of which the changes to the attribute get discarded.

You have two options:

  1. In a separate PR, try to fix the issue with module attribute caching. I don't remember exactly why I had trouble doing this when I tried, but I remember it wasn't as easy as I hoped it would be.
  2. Bundle this pass together in the same module transformer as another change that edits something other than the attributes of every module it edits attributes in. This will bypass the issue with caching and correctly give back the right info.

@Robertorosmaninho
Copy link
Collaborator Author

As far as the issue with the module transformer is concerned, it is a known issue that passes that only edit the attributes of sentences in a module get cached incorrectly by the code that helps cache module transformations, as a result of which the changes to the attribute get discarded.

You have two options:

  1. In a separate PR, try to fix the issue with module attribute caching. I don't remember exactly why I had trouble doing this when I tried, but I remember it wasn't as easy as I hoped it would be.

That's interesting, could you provide me with more details about it?

  • When/how did you face this issue before?
  • What have you tried?
  • How do we cache these module transformations?

@Robertorosmaninho
Copy link
Collaborator Author

Blocked on #3523.
We need to fix the Transformation caching to update the modules when we only modify the sentence attributes!

@Robertorosmaninho Robertorosmaninho added refactor java Pull requests that update Java code labels Jul 21, 2023
@Robertorosmaninho Robertorosmaninho marked this pull request as ready for review July 24, 2023 19:03
Copy link
Contributor

@radumereuta radumereuta left a comment

Choose a reason for hiding this comment

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

lgtm, but I would like it if someone else could have a look.

@rv-jenkins rv-jenkins merged commit 4db2ccb into develop Jul 26, 2023
@rv-jenkins rv-jenkins deleted the AddKoreAttributesPass branch July 26, 2023 14:02
Robertorosmaninho added a commit that referenced this pull request Aug 1, 2023
dwightguth pushed a commit that referenced this pull request Aug 2, 2023
* Revert "AddKoreAttribute as a Frontend Kompiler Pass (#3522)"

This reverts commit 4db2ccb.

* Update test-pr.yml

Testing public runners

* Update test-pr.yml
Baltoli pushed a commit that referenced this pull request Nov 1, 2023
…3187)

* haskell-backend/src/main/native/haskell-backend: 2c1ff15bd - Export z3 overlay alone in flake (#3519)

* Sync flake inputs to submodules

* haskell-backend/src/main/native/haskell-backend: 93a705112 - [#3493] Add total attribute (#3505)

* Sync flake inputs to submodules

* haskell-backend/src/main/native/haskell-backend: bd61a0565 - kore-rpc Catch any runtime exception and send as error (#3522)

* Sync flake inputs to submodules

* haskell-backend/src/main/native/haskell-backend: 2020ceb1b - Fixing new nix update breaking cachix installation (#3526)

* Sync flake inputs to submodules

* haskell-backend/src/main/native/haskell-backend: 37f122964 - [#3493] Rename `Fl` (functional) by `T` (total) (#3521)

* Sync flake inputs to submodules

* haskell-backend/src/main/native/haskell-backend: 1c184832b - [#3523] Put unserialized definition.kore in bug report (#3527)

* Sync flake inputs to submodules

* haskell-backend/src/main/native/haskell-backend: 559424aa4 - InfoAttemptUnification to DebugAttemptUnification (#3525)

* Sync flake inputs to submodules

* Fix `Dockerfile`

* Fix another `Dockerfile`

* Update result for failing test

The rename from functional to total changed these outputs

---------

Co-authored-by: rv-jenkins <devops@runtimeverification.com>
Co-authored-by: Tamás Tóth <tothtamas28@users.noreply.github.com>
Co-authored-by: Tamas Toth <tamas.toth@runtimeverification.com>
Co-authored-by: Radu Mereuta <headness13@gmail.com>
@Baltoli Baltoli mentioned this pull request Dec 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants