Skip to content

Conversation

@AlexWaygood
Copy link
Member

@AlexWaygoodAlexWaygood commented Aug 28, 2023

  • Get rid of the ns argument to parse_file(); restore the verify argument that was removed in 1dd9510.
  • Add a limited_capi argument to parse_file(). Make the limited_capi argument to both Clinic.__init__ and parse_filerequired, so that callers always have to be explicit about whether the limited C API is desired or not. This means that there is a "single source of truth" about what the default is, rather than this being duplicated between the global constant and the logic in the create_cli() function.
  • Get rid of MockClinic from the test file, which was added in 1dd9510: just use our existing _make_clinic() helper function instead.

With this PR, we would need to make only these changes if we wanted to change it tomorrow so that the limited C API is the default for AC-generated code (plus a few fixes to some tests that currently depend on being able to use our internal C API):

diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index efc6a922c5..f4f8b2dd20 100644 --- a/Lib/test/test_clinic.py+++ b/Lib/test/test_clinic.py@@ -20,7 +20,7 @@ import clinic from clinic import DSLParser -DEFAULT_LIMITED_C_API = False+DEFAULT_LIMITED_C_API = True def _make_clinic(*, filename='clinic_tests'): diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 2a4b6ff3b7..1a82ff7e57 100755 --- a/Tools/clinic/clinic.py+++ b/Tools/clinic/clinic.py@@ -6096,8 +6096,8 @@ def create_cli() -> argparse.ArgumentParser: cmdline.add_argument("--exclude", type=str, action="append", help=("a file to exclude in --make mode; " "can be given multiple times")) - cmdline.add_argument("--limited", dest="limited_capi", action='store_true',- help="use the Limited C API")+ cmdline.add_argument("--no-limited", dest="limited_capi", action='store_false',+ help="Don't use the Limited C API") cmdline.add_argument("filename", metavar="FILE", type=str, nargs="*", help="the list of files to process") return cmdline

Closes#108504

vstinnerand others added 5 commits August 26, 2023 02:34
Revert my change adding 'ns' parameter, add back 'verify' parameter, and add also 'limited_capi' parameter.
Revert my change adding 'ns' parameter, add back 'verify' parameter, and add also 'limited_capi' parameter.
Copy link
Member

@vstinnervstinner left a comment

Choose a reason for hiding this comment

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

LGTM, whatever works :-) I'm not going to nitpick on the coding style ;-)

@AlexWaygoodAlexWaygood enabled auto-merge (squash) August 28, 2023 17:39
@AlexWaygood
Copy link
MemberAuthor

Thanks for the reviews!

@bedevere-bot

This comment was marked as spam.

@AlexWaygoodAlexWaygood deleted the improve-parse-file branch August 28, 2023 18:26
Sign up for freeto 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.

4 participants

@AlexWaygood@bedevere-bot@vstinner@erlend-aasland