From e9753e5d7be18541e8408d48ba8d94fe8776b2fb Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Sat, 26 Aug 2023 02:31:46 +0200 Subject: [PATCH 1/5] gh-108494: AC: change parse_file() API Revert my change adding 'ns' parameter, add back 'verify' parameter, and add also 'limited_capi' parameter. --- Lib/test/test_clinic.py | 10 +--------- Tools/clinic/clinic.py | 11 ++++++----- 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index e7039d59070597..5fb2ceecf09a27 100644 --- a/Lib/test/test_clinic.py +++ b/Lib/test/test_clinic.py @@ -22,13 +22,6 @@ from clinic import DSLParser -def default_namespace(): - ns = types.SimpleNamespace() - ns.force = False - ns.limited_capi = clinic.DEFAULT_LIMITED_CAPI - return ns - - def _make_clinic(*, filename='clinic_tests'): clang = clinic.CLanguage(None) c = clinic.Clinic(clang, filename=filename) @@ -704,9 +697,8 @@ def expect_parsing_failure( self, *, filename, expected_error, verify=True, output=None ): errmsg = re.escape(dedent(expected_error).strip()) - ns = default_namespace() with self.assertRaisesRegex(clinic.ClinicError, errmsg): - clinic.parse_file(filename, ns=ns) + clinic.parse_file(filename) def test_parse_file_no_extension(self) -> None: self.expect_parsing_failure( diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index c4304bb5b1270d..96ae1e5ad667a7 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -2593,11 +2593,10 @@ def __repr__(self) -> str: def parse_file( filename: str, *, - ns: argparse.Namespace, output: str | None = None, + verify: bool = True, + limited_capi: bool = DEFAULT_LIMITED_CAPI, ) -> None: - verify = not ns.force - limited_capi = ns.limited_capi if not output: output = filename @@ -6158,7 +6157,8 @@ def run_clinic(parser: argparse.ArgumentParser, ns: argparse.Namespace) -> None: continue if ns.verbose: print(path) - parse_file(path, ns=ns) + parse_file(path, + verify=not ns.force, limited_capi=ns.limited_capi) return if not ns.filename: @@ -6170,7 +6170,8 @@ def run_clinic(parser: argparse.ArgumentParser, ns: argparse.Namespace) -> None: for filename in ns.filename: if ns.verbose: print(filename) - parse_file(filename, output=ns.output, ns=ns) + parse_file(filename, output=ns.output, + verify=not ns.force, limited_capi=ns.limited_capi) def main(argv: list[str] | None = None) -> NoReturn: From 0b1ae979c055b2daa414421bc8b1210e94de1600 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Sat, 26 Aug 2023 02:31:46 +0200 Subject: [PATCH 2/5] gh-108494: AC: change parse_file() API Revert my change adding 'ns' parameter, add back 'verify' parameter, and add also 'limited_capi' parameter. --- Lib/test/test_clinic.py | 10 +--------- Tools/clinic/clinic.py | 11 ++++++----- 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index fcccf988d3a7dd..6ac29ba6aa5305 100644 --- a/Lib/test/test_clinic.py +++ b/Lib/test/test_clinic.py @@ -22,13 +22,6 @@ from clinic import DSLParser -def default_namespace(): - ns = types.SimpleNamespace() - ns.force = False - ns.limited_capi = clinic.DEFAULT_LIMITED_CAPI - return ns - - def _make_clinic(*, filename='clinic_tests'): clang = clinic.CLanguage(None) c = clinic.Clinic(clang, filename=filename) @@ -704,9 +697,8 @@ def expect_parsing_failure( self, *, filename, expected_error, verify=True, output=None ): errmsg = re.escape(dedent(expected_error).strip()) - ns = default_namespace() with self.assertRaisesRegex(clinic.ClinicError, errmsg): - clinic.parse_file(filename, ns=ns) + clinic.parse_file(filename) def test_parse_file_no_extension(self) -> None: self.expect_parsing_failure( diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 723592fa266c4c..f280138ad7cf65 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -2612,11 +2612,10 @@ def __repr__(self) -> str: def parse_file( filename: str, *, - ns: argparse.Namespace, output: str | None = None, + verify: bool = True, + limited_capi: bool = DEFAULT_LIMITED_CAPI, ) -> None: - verify = not ns.force - limited_capi = ns.limited_capi if not output: output = filename @@ -6190,7 +6189,8 @@ def run_clinic(parser: argparse.ArgumentParser, ns: argparse.Namespace) -> None: continue if ns.verbose: print(path) - parse_file(path, ns=ns) + parse_file(path, + verify=not ns.force, limited_capi=ns.limited_capi) return if not ns.filename: @@ -6202,7 +6202,8 @@ def run_clinic(parser: argparse.ArgumentParser, ns: argparse.Namespace) -> None: for filename in ns.filename: if ns.verbose: print(filename) - parse_file(filename, output=ns.output, ns=ns) + parse_file(filename, output=ns.output, + verify=not ns.force, limited_capi=ns.limited_capi) def main(argv: list[str] | None = None) -> NoReturn: From e9f70f2e2fedd9d0b6ab5b53101dbd3e411c5377 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Mon, 28 Aug 2023 18:16:19 +0200 Subject: [PATCH 3/5] Clinic constructor uses DEFAULT_LIMITED_CAPI --- Tools/clinic/clinic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index f280138ad7cf65..3ba6dd3ef3eb1f 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -2415,7 +2415,7 @@ def __init__( *, filename: str, verify: bool = True, - limited_capi: bool = False, + limited_capi: bool = DEFAULT_LIMITED_CAPI, ) -> None: # maps strings to Parser objects. # (instantiated from the "parsers" global.) From 7a1767850a2ca3ac6106936d6ad958b2347eb157 Mon Sep 17 00:00:00 2001 From: AlexWaygood Date: Mon, 28 Aug 2023 17:51:30 +0100 Subject: [PATCH 4/5] gh-108494: Argument clinic: Improve the API of `parse_file()` --- Lib/test/test_clinic.py | 20 ++++++++------------ Tools/clinic/clinic.py | 5 ++--- 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index 6ac29ba6aa5305..efc6a922c59a14 100644 --- a/Lib/test/test_clinic.py +++ b/Lib/test/test_clinic.py @@ -13,7 +13,6 @@ import os.path import re import sys -import types import unittest test_tools.skip_if_missing('clinic') @@ -21,10 +20,12 @@ import clinic from clinic import DSLParser +DEFAULT_LIMITED_C_API = False + def _make_clinic(*, filename='clinic_tests'): clang = clinic.CLanguage(None) - c = clinic.Clinic(clang, filename=filename) + c = clinic.Clinic(clang, filename=filename, limited_capi=DEFAULT_LIMITED_C_API) c.block_parser = clinic.BlockParser('', clang) return c @@ -53,11 +54,6 @@ def _expect_failure(tc, parser, code, errmsg, *, filename=None, lineno=None, return cm.exception -class MockClinic: - def __init__(self): - self.limited_capi = clinic.DEFAULT_LIMITED_CAPI - - class ClinicWholeFileTest(TestCase): maxDiff = None @@ -131,7 +127,7 @@ def test_parse_with_body_prefix(self): clang.body_prefix = "//" clang.start_line = "//[{dsl_name} start]" clang.stop_line = "//[{dsl_name} stop]" - cl = clinic.Clinic(clang, filename="test.c") + cl = clinic.Clinic(clang, filename="test.c", limited_capi=DEFAULT_LIMITED_C_API) raw = dedent(""" //[clinic start] //module test @@ -698,7 +694,7 @@ def expect_parsing_failure( ): errmsg = re.escape(dedent(expected_error).strip()) with self.assertRaisesRegex(clinic.ClinicError, errmsg): - clinic.parse_file(filename) + clinic.parse_file(filename, limited_capi=DEFAULT_LIMITED_C_API) def test_parse_file_no_extension(self) -> None: self.expect_parsing_failure( @@ -838,9 +834,9 @@ def _test(self, input, output): blocks = list(clinic.BlockParser(input, language)) writer = clinic.BlockPrinter(language) - mock_clinic = MockClinic() + c = _make_clinic() for block in blocks: - writer.print_block(block, clinic=mock_clinic) + writer.print_block(block, clinic=c) output = writer.f.getvalue() assert output == input, "output != input!\n\noutput " + repr(output) + "\n\n input " + repr(input) @@ -866,7 +862,7 @@ def test_round_trip_2(self): def _test_clinic(self, input, output): language = clinic.CLanguage(None) - c = clinic.Clinic(language, filename="file") + c = clinic.Clinic(language, filename="file", limited_capi=DEFAULT_LIMITED_C_API) c.parsers['inert'] = InertParser(c) c.parsers['copy'] = CopyParser(c) computed = c.parse(input) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 3ba6dd3ef3eb1f..2a4b6ff3b776eb 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -63,7 +63,6 @@ version = '1' -DEFAULT_LIMITED_CAPI = False NO_VARARG = "PY_SSIZE_T_MAX" CLINIC_PREFIX = "__clinic_" CLINIC_PREFIXED_ARGS = { @@ -2415,7 +2414,7 @@ def __init__( *, filename: str, verify: bool = True, - limited_capi: bool = DEFAULT_LIMITED_CAPI, + limited_capi: bool ) -> None: # maps strings to Parser objects. # (instantiated from the "parsers" global.) @@ -2614,7 +2613,7 @@ def parse_file( *, output: str | None = None, verify: bool = True, - limited_capi: bool = DEFAULT_LIMITED_CAPI, + limited_capi: bool, ) -> None: if not output: output = filename From 88626ce751ff2dd7a66fa80914d5b071614d1ba2 Mon Sep 17 00:00:00 2001 From: AlexWaygood Date: Mon, 28 Aug 2023 18:39:07 +0100 Subject: [PATCH 5/5] Address Victor's review --- Lib/test/test_clinic.py | 10 ++++------ Tools/clinic/clinic.py | 4 ++-- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index efc6a922c59a14..0592c2e3a2b291 100644 --- a/Lib/test/test_clinic.py +++ b/Lib/test/test_clinic.py @@ -20,12 +20,10 @@ import clinic from clinic import DSLParser -DEFAULT_LIMITED_C_API = False - def _make_clinic(*, filename='clinic_tests'): clang = clinic.CLanguage(None) - c = clinic.Clinic(clang, filename=filename, limited_capi=DEFAULT_LIMITED_C_API) + c = clinic.Clinic(clang, filename=filename, limited_capi=False) c.block_parser = clinic.BlockParser('', clang) return c @@ -127,7 +125,7 @@ def test_parse_with_body_prefix(self): clang.body_prefix = "//" clang.start_line = "//[{dsl_name} start]" clang.stop_line = "//[{dsl_name} stop]" - cl = clinic.Clinic(clang, filename="test.c", limited_capi=DEFAULT_LIMITED_C_API) + cl = clinic.Clinic(clang, filename="test.c", limited_capi=False) raw = dedent(""" //[clinic start] //module test @@ -694,7 +692,7 @@ def expect_parsing_failure( ): errmsg = re.escape(dedent(expected_error).strip()) with self.assertRaisesRegex(clinic.ClinicError, errmsg): - clinic.parse_file(filename, limited_capi=DEFAULT_LIMITED_C_API) + clinic.parse_file(filename, limited_capi=False) def test_parse_file_no_extension(self) -> None: self.expect_parsing_failure( @@ -862,7 +860,7 @@ def test_round_trip_2(self): def _test_clinic(self, input, output): language = clinic.CLanguage(None) - c = clinic.Clinic(language, filename="file", limited_capi=DEFAULT_LIMITED_C_API) + c = clinic.Clinic(language, filename="file", limited_capi=False) c.parsers['inert'] = InertParser(c) c.parsers['copy'] = CopyParser(c) computed = c.parse(input) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 2a4b6ff3b776eb..39a776f02b5bf5 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -2413,8 +2413,8 @@ def __init__( printer: BlockPrinter | None = None, *, filename: str, + limited_capi: bool, verify: bool = True, - limited_capi: bool ) -> None: # maps strings to Parser objects. # (instantiated from the "parsers" global.) @@ -2611,9 +2611,9 @@ def __repr__(self) -> str: def parse_file( filename: str, *, + limited_capi: bool, output: str | None = None, verify: bool = True, - limited_capi: bool, ) -> None: if not output: output = filename