Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Cleaning up generating macros in_cursesmodule.c#125354

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed

Conversation

picnixz
Copy link
Member

This is mostly a cosmetic but useful PR. Why? because it's for future maintenance. For now, all generated methods have the following signature (modulo thePy_UNUSED):

staticPyObject*NAME(PyCursesWindowObject*self,PyObject*Py_UNUSED(args));

and they are stored inPyCursesWindow_methods using(PyCFunction) casts. I'm not sure whether we want to keep an explicit cast here or not but since we are also fixing UBSan failures, I think this one would count as one.

Instead of changing it now, I first suggest cleaning up the macros and then, once we also fixed Argument Clinic generation, we may then fix the UBSan failures here as well (or I can patch half of the Window's object methods since the other half is AC-generated).

I don't see when we'll need to call a method directly using its implementation and not via thePyObject_Call* API so we can also patch the macros directly in this PR.

@vstinner please tell me whether you want me to include the UBSan failure fix in this PR or not (for that reason, I will not put an issue number yet since I don't know whether it should be part of#123961 or#111178).

@picnixzpicnixzforce-pushed thecurses/macro-cleanup-123961 branch fromadb6a33 to6f84b18CompareOctober 13, 2024 12:40
@picnixz
Copy link
MemberAuthor

The commit6f84b18 is only for removing thestateful part in the names of the functions, making most of the functions named with a shorter name. There are only 3-4 occurrences and usage of a stateless check but many occurrences of a stateful one.

If you want a separate PR, I can just cherry-pick that commit though it's easier to include it in this one to reduce the number of whitespace changes.

@vstinner
Copy link
Member

I don't see the point of these changes, they don't solve any problem. They only look like coding style changes: -1 for me.

@picnixz
Copy link
MemberAuthor

picnixz commentedOct 14, 2024
edited
Loading

Oh I should have given more context. I wanted to add the possibility to include the Python function name and the curses function name that were involved when we raise an exception. For now, it's a generic message saying that function X raised something, but the names may not necessarily match the C function that was actually called.

What I wanted to do is essentially mimic OSError and its filename/filename2 attributes (that could help instead of parsing the message that we could change in the future). So I wanted first to cleanup the base code before continuing the work.


If you think that the error feature is not needed, I'll just close this PR and the refactorization issue as well.

@picnixz
Copy link
MemberAuthor

Closing in favor of#125844.

@picnixzpicnixz deleted the curses/macro-cleanup-123961 branchOctober 22, 2024 14:32
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@vstinnervstinnerAwaiting requested review from vstinner

Assignees
No one assigned
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

2 participants
@picnixz@vstinner

[8]ページ先頭

©2009-2025 Movatter.jp