Skip to content

gh-127341: Argument Clinic: fix compiler warnings for getters with docstrings #127310

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

Merged

Conversation

ZeroIntensity
Copy link
Member

@ZeroIntensity ZeroIntensity commented Nov 26, 2024

Finally got enough connection to submit a PR 😄

Co-authored-by: Erlend E. Aasland erlend.aasland@protonmail.com

@skirpichev
Copy link
Member

It seems you forgot make clinic, or it was intentional, to reduce patch size? Change looks fine.

@ZeroIntensity
Copy link
Member Author

It seems you forgot make clinic, or it was intentional, to reduce patch size? Change looks fine.

Yeah, that would be it. Fixed!

@skirpichev skirpichev self-requested a review November 27, 2024 02:24
@erlend-aasland
Copy link
Contributor

I don't like the original approach that was taken when the GETSET_ clinic templates were added. How about we simplify it to this:

diff --git a/Tools/clinic/libclinic/parse_args.py b/Tools/clinic/libclinic/parse_args.py
index fc2d9fe9870..a57d729bec5 100644
--- a/Tools/clinic/libclinic/parse_args.py
+++ b/Tools/clinic/libclinic/parse_args.py
@@ -146,7 +146,7 @@ def declare_parser(
 GETSET_DOCSTRING_PROTOTYPE_STRVAR: Final[str] = libclinic.normalize_snippet("""
     PyDoc_STRVAR({getset_basename}__doc__,
     {docstring});
-    #define {getset_basename}_HAS_DOCSTR
+    #define {getset_basename}_DOCSTR {getset_basename}__doc__
 """)
 IMPL_DEFINITION_PROTOTYPE: Final[str] = libclinic.normalize_snippet("""
     static {impl_return_type}
@@ -157,9 +157,7 @@ def declare_parser(
         {{"{name}", {methoddef_cast}{c_basename}{methoddef_cast_end}, {methoddef_flags}, {c_basename}__doc__}},
 """)
 GETTERDEF_PROTOTYPE_DEFINE: Final[str] = libclinic.normalize_snippet(r"""
-    #if defined({getset_basename}_HAS_DOCSTR)
-    #  define {getset_basename}_DOCSTR {getset_basename}__doc__
-    #else
+    #if !defined({getset_basename}_DOCSTR)
     #  define {getset_basename}_DOCSTR NULL
     #endif
     #if defined({getset_name}_GETSETDEF)
@@ -170,9 +168,7 @@ def declare_parser(
     #endif
 """)
 SETTERDEF_PROTOTYPE_DEFINE: Final[str] = libclinic.normalize_snippet(r"""
-    #if defined({getset_name}_HAS_DOCSTR)
-    #  define {getset_basename}_DOCSTR {getset_basename}__doc__
-    #else
+    #if !defined({getset_basename}_DOCSTR)
     #  define {getset_basename}_DOCSTR NULL
     #endif
     #if defined({getset_name}_GETSETDEF)

@erlend-aasland
Copy link
Contributor

BTW, I think we should record why this was a bug in an issue, and then link this PR to that issue.

@ZeroIntensity ZeroIntensity changed the title Clinic: Fix compiler warnings when using a docstring with a getter gh-127341: Clinic: Fix compiler warnings when using a docstring with a getter Nov 27, 2024
@erlend-aasland erlend-aasland changed the title gh-127341: Clinic: Fix compiler warnings when using a docstring with a getter gh-127341: Argument Clinic: fix compiler warnings for getters with docstrings Nov 29, 2024
Copy link
Contributor

@erlend-aasland erlend-aasland left a comment

Choose a reason for hiding this comment

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

Thanks for identifying and fixing this!

@erlend-aasland erlend-aasland merged commit 9949091 into python:main Nov 29, 2024
54 checks passed
@erlend-aasland erlend-aasland added the needs backport to 3.13 bugs and security fixes label Nov 29, 2024
@miss-islington-app

This comment was marked as outdated.

@miss-islington-app
Copy link

Sorry, @ZeroIntensity and @erlend-aasland, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 99490913a08adcf2fe5e69b82772a829ec462275 3.13

@ZeroIntensity
Copy link
Member Author

I'll deal with backporting.

ZeroIntensity added a commit to ZeroIntensity/cpython that referenced this pull request Nov 29, 2024
…tters with docstrings (pythonGH-127310)

(cherry picked from commit 9949091)

Co-authored-by: Peter Bierma <zintensitydev@gmail.com>
Co-authored-by: Erlend E. Aasland <erlend@python.org>
@bedevere-app
Copy link

bedevere-app bot commented Nov 29, 2024

GH-127431 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Nov 29, 2024
erlend-aasland added a commit that referenced this pull request Nov 30, 2024
…with docstrings (GH-127310) (#127431)

(cherry picked from commit 9949091)

Co-authored-by: Erlend E. Aasland <erlend@python.org>
@ZeroIntensity ZeroIntensity deleted the fix-clinic-getsetdef-docstrings branch December 14, 2024 16:41
srinivasreddy pushed a commit to srinivasreddy/cpython that referenced this pull request Jan 8, 2025
…ith docstrings (python#127310)

Co-authored-by: Erlend E. Aasland <erlend@python.org>
ebonnal pushed a commit to ebonnal/cpython that referenced this pull request Jan 12, 2025
…ith docstrings (python#127310)

Co-authored-by: Erlend E. Aasland <erlend@python.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clinic causes compiler warnings when using a getter and setter with a docstring
3 participants