From 19c720e8fa448e373610c23bb1be5e8ef82d0777 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Mon, 25 Dec 2023 22:52:56 +0100 Subject: [PATCH 1/3] gh-113317: Rework Argument Clinic cpp.py error handling Rework error handling in the C preprocessor helper. Instead of monkey- patching the cpp.Monitor.fail() method from within clinic.py, rewrite cpp.py to use a subclass of the ClinicError exception. Yak-shaving in preparation for putting cpp.py into libclinic. --- Lib/test/test_clinic.py | 4 ++-- Tools/clinic/clinic.py | 23 +---------------------- Tools/clinic/cpp.py | 21 +++++++++------------ Tools/clinic/libclinic/__init__.py | 6 ++++++ Tools/clinic/libclinic/error.py | 26 ++++++++++++++++++++++++++ 5 files changed, 44 insertions(+), 36 deletions(-) create mode 100644 Tools/clinic/libclinic/error.py diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index 21f56fe0195e69..3d6816d73d45bc 100644 --- a/Lib/test/test_clinic.py +++ b/Lib/test/test_clinic.py @@ -22,7 +22,7 @@ def _make_clinic(*, filename='clinic_tests'): - clang = clinic.CLanguage(None) + clang = clinic.CLanguage(filename) c = clinic.Clinic(clang, filename=filename, limited_capi=False) c.block_parser = clinic.BlockParser('', clang) return c @@ -3920,7 +3920,7 @@ def test_Function_and_Parameter_reprs(self): self.assertEqual(repr(parameter), "") def test_Monitor_repr(self): - monitor = clinic.cpp.Monitor() + monitor = clinic.cpp.Monitor("test.c") self.assertRegex(repr(monitor), r"") monitor.line_number = 42 diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index f004bec3cce8f6..82efff56eda756 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -53,6 +53,7 @@ # Local imports. import libclinic +from libclinic import ClinicError # TODO: @@ -94,27 +95,6 @@ def __repr__(self) -> str: TemplateDict = dict[str, str] -@dc.dataclass -class ClinicError(Exception): - message: str - _: dc.KW_ONLY - lineno: int | None = None - filename: str | None = None - - def __post_init__(self) -> None: - super().__init__(self.message) - - def report(self, *, warn_only: bool = False) -> str: - msg = "Warning" if warn_only else "Error" - if self.filename is not None: - msg += f" in file {self.filename!r}" - if self.lineno is not None: - msg += f" on line {self.lineno}" - msg += ":\n" - msg += f"{self.message}\n" - return msg - - @overload def warn_or_fail( *args: object, @@ -669,7 +649,6 @@ class CLanguage(Language): def __init__(self, filename: str) -> None: super().__init__(filename) self.cpp = cpp.Monitor(filename) - self.cpp.fail = fail # type: ignore[method-assign] def parse_line(self, line: str) -> None: self.cpp.writeline(line) diff --git a/Tools/clinic/cpp.py b/Tools/clinic/cpp.py index 16eee6fc399491..87d21bb8adceb8 100644 --- a/Tools/clinic/cpp.py +++ b/Tools/clinic/cpp.py @@ -3,6 +3,8 @@ import sys from typing import NoReturn +from libclinic.error import ParseError + TokenAndCondition = tuple[str, str] TokenStack = list[TokenAndCondition] @@ -32,7 +34,7 @@ class Monitor: Anyway this implementation seems to work well enough for the CPython sources. """ - filename: str | None = None + filename: str _: dc.KW_ONLY verbose: bool = False @@ -59,14 +61,8 @@ def condition(self) -> str: """ return " && ".join(condition for token, condition in self.stack) - def fail(self, *a: object) -> NoReturn: - if self.filename: - filename = " " + self.filename - else: - filename = '' - print("Error at" + filename, "line", self.line_number, ":") - print(" ", ' '.join(str(x) for x in a)) - sys.exit(-1) + def fail(self, msg: str) -> NoReturn: + raise ParseError(msg, filename=self.filename, lineno=self.line_number) def writeline(self, line: str) -> None: self.line_number += 1 @@ -74,7 +70,7 @@ def writeline(self, line: str) -> None: def pop_stack() -> TokenAndCondition: if not self.stack: - self.fail("#" + token + " without matching #if / #ifdef / #ifndef!") + self.fail(f"#{token} without matching #if / #ifdef / #ifndef!") return self.stack.pop() if self.continuation: @@ -145,7 +141,7 @@ def pop_stack() -> TokenAndCondition: if token in {'if', 'ifdef', 'ifndef', 'elif'}: if not condition: - self.fail("Invalid format for #" + token + " line: no argument!") + self.fail(f"Invalid format for #{token} line: no argument!") if token in {'if', 'elif'}: if not is_a_simple_defined(condition): condition = "(" + condition + ")" @@ -155,7 +151,8 @@ def pop_stack() -> TokenAndCondition: else: fields = condition.split() if len(fields) != 1: - self.fail("Invalid format for #" + token + " line: should be exactly one argument!") + self.fail(f"Invalid format for #{token} line: " + "should be exactly one argument!") symbol = fields[0] condition = 'defined(' + symbol + ')' if token == 'ifndef': diff --git a/Tools/clinic/libclinic/__init__.py b/Tools/clinic/libclinic/__init__.py index 0c3c6840901a42..b0a40be9622c66 100644 --- a/Tools/clinic/libclinic/__init__.py +++ b/Tools/clinic/libclinic/__init__.py @@ -1,5 +1,8 @@ from typing import Final +from .error import ( + ClinicError, +) from .formatting import ( SIG_END_MARKER, c_repr, @@ -15,6 +18,9 @@ __all__ = [ + # Error handling + "ClinicError", + # Formatting helpers "SIG_END_MARKER", "c_repr", diff --git a/Tools/clinic/libclinic/error.py b/Tools/clinic/libclinic/error.py new file mode 100644 index 00000000000000..afb21b02386fe7 --- /dev/null +++ b/Tools/clinic/libclinic/error.py @@ -0,0 +1,26 @@ +import dataclasses as dc + + +@dc.dataclass +class ClinicError(Exception): + message: str + _: dc.KW_ONLY + lineno: int | None = None + filename: str | None = None + + def __post_init__(self) -> None: + super().__init__(self.message) + + def report(self, *, warn_only: bool = False) -> str: + msg = "Warning" if warn_only else "Error" + if self.filename is not None: + msg += f" in file {self.filename!r}" + if self.lineno is not None: + msg += f" on line {self.lineno}" + msg += ":\n" + msg += f"{self.message}\n" + return msg + + +class ParseError(ClinicError): + pass From ed150ca6dd0cf25da65262850d633c640e42aa1e Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Wed, 27 Dec 2023 22:20:29 +0100 Subject: [PATCH 2/3] Address review: rename error.py as errors.py --- Tools/clinic/libclinic/{error.py => errors.py} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename Tools/clinic/libclinic/{error.py => errors.py} (100%) diff --git a/Tools/clinic/libclinic/error.py b/Tools/clinic/libclinic/errors.py similarity index 100% rename from Tools/clinic/libclinic/error.py rename to Tools/clinic/libclinic/errors.py From c1be56e9bcd3004d3e2a2ddec48af601154554eb Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Wed, 27 Dec 2023 22:23:21 +0100 Subject: [PATCH 3/3] Fix --- Tools/clinic/cpp.py | 2 +- Tools/clinic/libclinic/__init__.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Tools/clinic/cpp.py b/Tools/clinic/cpp.py index 87d21bb8adceb8..659099056cd46c 100644 --- a/Tools/clinic/cpp.py +++ b/Tools/clinic/cpp.py @@ -3,7 +3,7 @@ import sys from typing import NoReturn -from libclinic.error import ParseError +from libclinic.errors import ParseError TokenAndCondition = tuple[str, str] diff --git a/Tools/clinic/libclinic/__init__.py b/Tools/clinic/libclinic/__init__.py index b0a40be9622c66..d4e7a0c5cf7b76 100644 --- a/Tools/clinic/libclinic/__init__.py +++ b/Tools/clinic/libclinic/__init__.py @@ -1,6 +1,6 @@ from typing import Final -from .error import ( +from .errors import ( ClinicError, ) from .formatting import (