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

Don't allow interfaces to have private methods #3973

Merged
merged 7 commits into from
Jan 29, 2022
Merged

Don't allow interfaces to have private methods #3973

merged 7 commits into from
Jan 29, 2022

Conversation

SeanTAllen
Copy link
Member

@SeanTAllen SeanTAllen commented Jan 29, 2022

When Pony allowed interfaces to have private methods, it was possible to use an interface to break encapsulation for objects and access private methods from outside their enclosing package.

The following code used to be legal Pony code but will now fail with a compiler error:

actor Main
  new create(env: Env) =>
    let x: String ref = "sailor".string()
    let y: Foo = x
    y._set(0, 'f')
    env.out.print("Hello, " + x)

interface Foo
  fun ref _set(i: USize, value: U8): U8

AsioEventNotify in the standard library had to be changed from an interface with a single private method to a trait in order to conform with this change.

Fixes #2772

@SeanTAllen
Copy link
Member Author

SeanTAllen commented Jan 29, 2022

I expect this to pass CI. If it does it needs:

  • release notes
  • comment for this PR clean up
  • Good PR comment
  • bad pony test to verify the error
  • Changelog label
  • Additional manually added changelog entry (for ASIO change)

@SeanTAllen SeanTAllen changed the title WIP for issue 2772 Don't allow interfaces to have private methods Jan 29, 2022
@SeanTAllen SeanTAllen requested a review from a team January 29, 2022 13:26
@SeanTAllen SeanTAllen added changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge changelog - changed Automatically add "Changed" CHANGELOG entry on merge and removed changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge labels Jan 29, 2022
@SeanTAllen SeanTAllen marked this pull request as ready for review January 29, 2022 13:27
@SeanTAllen SeanTAllen merged commit 4f0a2dd into main Jan 29, 2022
@SeanTAllen SeanTAllen deleted the issue-2772 branch January 29, 2022 19:17
github-actions bot pushed a commit that referenced this pull request Jan 29, 2022
github-actions bot pushed a commit that referenced this pull request Jan 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog - changed Automatically add "Changed" CHANGELOG entry on merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Private methods from other packages accessible through interfaces
2 participants