Skip to content

Conversation

@Jefffrey
Copy link
Contributor

Which issue does this PR close?

N/A

Rationale for this change

Whilst reviewing some recent PRs (#18839 & #18768) I noticed we have quite a few inner implementation functions that are public for some reason, which give the false impression these are meant to be public APIs (and thus any changes to their signature needs to be restricted). Went through and limited the functions to private where possible to try reduce our public API footprint.

What changes are included in this PR?

Change inner functions in functions & nested-functions crates to be private, away from public.

  • There are still some that are left public such as some regex ones, because they are used directly in benches

Are these changes tested?

Compiler itself.

Are there any user-facing changes?

Yes, quite a few functions are now private, but I don't think they were meant to be public in the first place.

@github-actions github-actions bot added the functions Changes to functions implementation label Nov 22, 2025
}
}

#[expect(clippy::needless_pass_by_value)]
Copy link
Contributor Author

@Jefffrey Jefffrey Nov 22, 2025

Choose a reason for hiding this comment

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

I wanted to fix this lint, then realised could do a wider refactor to eliminate this function and simplify the code

@Jefffrey Jefffrey marked this pull request as ready for review November 22, 2025 03:25
Copy link
Contributor

@2010YOUY01 2010YOUY01 left a comment

Choose a reason for hiding this comment

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

LGTM, thank you.

I also found those unnecessary pubs annoying, perhaps https://doc.rust-lang.org/stable/nightly-rustc/rustc_lint/builtin/static.UNREACHABLE_PUB.html can help, I will experiment later.

@alamb alamb added the api change Changes the API exposed to users of the crate label Nov 23, 2025
Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Makes sense to me too -- if downstream users want to use this code I think it is ok if they simply copy it into their projects ('vendor' it)

@Jefffrey Jefffrey added this pull request to the merge queue Nov 23, 2025
Merged via the queue into apache:main with commit 76b9e12 Nov 23, 2025
28 checks passed
@Jefffrey Jefffrey deleted the fix-functions-pub-types branch November 23, 2025 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api change Changes the API exposed to users of the crate functions Changes to functions implementation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants