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

AllBottom: simplify and fix meetWith #36

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

AllBottom: simplify and fix meetWith #36

wants to merge 1 commit into from

Conversation

langston-barrett
Copy link

Previously, AllBottom would throw an exception when meetWith was called with a user-supplied EdgeFunction. The proper behavior is to return AllBottom unconditionally.

@ericbodden
Copy link
Member

I am sorry but the check exists for a reason: when the exception is thrown then you are most likely doing something wrong. IIRC the solver should never get into a situation where it needs to compose AllBottom with a user-supplied function. When does that happen in your case?

@ericbodden
Copy link
Member

I had another look to make sure. I am reasonably sure that AllBottom is only used in the case in which IDE is used to instantiate an IFDS problem - and then the exception actually is a sensible precaution. I am not sure AllBottom ever makes sense to be used within another analysis. So can you please elaborate on your use case? Thanks!

@langston-barrett
Copy link
Author

Ah, I was under the impression that AllBottom was a general purpose utility class, not an internal class to be used only in the solver. The reason I encountered the exception was that I returned an instance of AllBottom from an implementation of EdgeFunctions, which it sounds like I should not have tried.

To avoid such a misuse in the future, would it make sense to make AllBottom (and perhaps AllTop) into private or protected classes?

@ericbodden
Copy link
Member

I can't 100% rule out the fact that AllBottom may be useful

The reason I encountered the exception was that I returned an instance of AllBottom from an implementation of EdgeFunctions, which it sounds like I should not have tried.

@langston-barrett that's interesting. So far I would not have thought this use case to ever arise, but I'd be happy to have you convince me otherwise. Would it be possible for you to paste the relevant part of the edge-function implementation here? If AllBottom seems generally useful then I'd be happy to remove the checks.

@ericbodden ericbodden self-assigned this Dec 17, 2019
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.

2 participants