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

feat: Add pub modifier #2754

Merged
merged 17 commits into from
Sep 21, 2023
Merged

feat: Add pub modifier #2754

merged 17 commits into from
Sep 21, 2023

Conversation

jfecher
Copy link
Contributor

@jfecher jfecher commented Sep 19, 2023

Description

Problem*

Resolves #776

Summary*

Superceeds PR #2513

Adds the pub modifier as in #2513, but refactors the PR to check within name resolution rather than adding a separate pass to check for visibility.

Documentation

  • This PR requires documentation updates when merged.

    • I will submit a noir-lang/docs PR.
    • I will request for and support Dev Rel's help in documenting this PR.
      • We will need to mention functions can be made public now pub fn foo(){}
      • Functions that are not public are now no longer visible from outside the crate they were defined in, or from any parent module within the same crate.

Additional Context

PR Checklist*

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt on default settings.

@kevaundray
Copy link
Contributor

@guipublic assigning you as a reviewer

guipublic
guipublic previously approved these changes Sep 21, 2023
@kevaundray
Copy link
Contributor

Can merge after merge conflicts are resolved!

@kevaundray kevaundray added this pull request to the merge queue Sep 21, 2023
Merged via the queue into master with commit dda964e Sep 21, 2023
14 checks passed
@kevaundray kevaundray deleted the jf/add-pub branch September 21, 2023 14:19
@Savio-Sou
Copy link
Collaborator

Would this be a breaking change?

As in all existing Noir functions before this PR without the pub modifier are treated as public by default, but after this PR they would be treated as private by default?

@guipublic
Copy link
Contributor

Yes, functions are now treated as private by default, you need the pub modifier to make them public.
BUT:

  • it is for now a warning, so I is not a breaking change. You are notified that this will break in the future
  • it is only for librairies. If you use function from your own module, it will not complain.
  • noir std library has the pub modifier.

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.

Introduce the notion of public function
4 participants