KEMBAR78
gh-118771: Ensure names defined in optimizer.h start with Py/_Py by encukou · Pull Request #118825 · python/cpython · GitHub
Skip to content

gh-118771: Ensure names defined in optimizer.h start with Py/_Py #118825

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
May 10, 2024
Merged

gh-118771: Ensure names defined in optimizer.h start with Py/_Py #118825

merged 4 commits into from
May 10, 2024

Conversation

@encukou
Copy link
Member

@encukou encukou commented May 9, 2024 •
edited by bedevere-app bot
Loading

optimizer.h is included from Python.h, and defines some names that can clash with user code. They should be hidden or get the Py/_Py prefix.

This PR:

  • removes unused struct tags from typedefs:
    • struct _bloom_filter
    • struct _exit_data
  • prefixes the remaining names with _Py_:
    • uop_get_target
    • uop_get_exit_index
    • uop_get_jump_target
    • uop_get_error_target
    • optimize_func typedef
    • BLOOM_FILTER_WORDS
    • UOP_FORMAT_TARGET
    • UOP_FORMAT_EXIT
    • UOP_FORMAT_JUMP
    • UOP_FORMAT_UNUSED

Perhaps some of these should be in a private header instead?
Or perhaps some should be public?
@markshannon, could you clarify the intent here?

@bedevere-app bedevere-app bot mentioned this pull request May 9, 2024
Closed
28 tasks
@gvanrossum gvanrossum removed their request for review May 9, 2024 15:21
@gvanrossum
Copy link
Member

I'll leave this for @markshannon to review.

@brandtbucher
Copy link
Member

brandtbucher commented May 9, 2024 •
edited
Loading

I personally don't think we should be exposing this much of the micro-op format. Can they just be moved to pycore_optimizer.h? I forget if the same naming rules apply there.

@markshannon
Copy link
Member

Yes, let's move as much as we can to pycore_optimizer.h

markshannon
markshannon approved these changes May 10, 2024
View reviewed changes
Copy link
Member

@markshannon markshannon left a comment

Choose a reason for hiding this comment

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

@encukou Thanks for tidying that up

@encukou encukou merged commit 941eea0 into python:main May 10, 2024
@encukou encukou deleted the private branch May 10, 2024 16:20
estyxx pushed a commit to estyxx/cpython that referenced this pull request Jul 17, 2024
@encukou @estyxx
pythongh-118771: Ensure names defined in optimizer.h start with Py/_Py (
08216ae 
pythonGH-118825)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@markshannon markshannon markshannon approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@encukou @gvanrossum @brandtbucher @markshannon