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

Ensure IO is non-null before attempting to autoWrapPorts. #643

Merged
merged 2 commits into from
Jul 7, 2017

Conversation

ucbjrl
Copy link
Contributor

@ucbjrl ucbjrl commented Jul 6, 2017

No description provided.

@ucbjrl ucbjrl requested a review from ducky64 July 6, 2017 19:45
@ducky64
Copy link
Contributor

ducky64 commented Jul 6, 2017

What was this meant to fix? I think it makes sense to have it fail noisily if IO isn't defined?

@ucbjrl
Copy link
Contributor Author

ucbjrl commented Jul 6, 2017

The problem is due to attempting to validate any connections that precede the IO definition in compatibility mode. Failing with a null pointer exception seems wrong. But I suppose so would a reference to an (as yet) undefined IO port.

Perhaps we should require IO ports to be defined before any connections are made?

Copy link
Contributor

@ducky64 ducky64 left a comment

Choose a reason for hiding this comment

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

Looks good.

This looks sketchy, though I think all cases where this wouldn't catch an error would be caught at the use site. This mainly exists to make the binding system happy.

@ucbjrl ucbjrl merged commit 794812f into master Jul 7, 2017
@ucbjrl ucbjrl deleted the checknullio branch July 7, 2017 20:00
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