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

Error checking for JVMSetup commands #948

Merged
merged 12 commits into from
Dec 4, 2020
Merged

Error checking for JVMSetup commands #948

merged 12 commits into from
Dec 4, 2020

Conversation

brianhuffman
Copy link
Contributor

This PR includes a few things related to error checking for JVMSetup blocks:

  • Add custom exception datatype for JVMSetup errors
  • Use throwM from exceptions package instead of fail to report errors
  • Add more well-formedness checks to various JVMSetup commands
  • Refactor some method-spec datatypes to enforce invariants
  • Update regression tests to eliminate known false positives

Copy link
Contributor

@atomb atomb left a comment

Choose a reason for hiding this comment

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

This is a great improvement. There are still a couple of fail calls in nearby code, and I commented on those. I was just looking at the context provided in the diffs, though, and it may be that those are still fail because they're executing in a context that isn't set up for throwM yet.

pos <- getPosition
-- cls' is either cls or a (transitive) superclass of cls
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice to see this corrected!

Brian Huffman added 12 commits December 4, 2020 11:04
Support for static fields is planned using a different mechanism.
(See #908.)
This partially addresses #928. This patch only covers the uses of
`fail` that were used to implement the JVMSetup commands. There are
still a lot of uses of `fail` in other parts of the SAW/JVM code.
Previously it represented the left-hand side with an arbitrary
`SetupValue`; however, the only valid `SetupValue`s for this purpose
are `SetupVar` applied to an `AllocIndex`. This change lets us
simplify code for the simulation phase, in exchange for doing more
error checking during the setup phase (which is also a good thing).

The early checking eliminates a couple of the known false positives
from #938.
This fixes one of the known false positives from #938.
This fixes several more of the known false positives from #938.
This fixes several more of the known false positives from #938.
This fixes more remaining known false positives of #938.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Multiple jvm_field_is declarations on same field are not detected
2 participants