gh-73487: Convert _decimal to use Argument Clinic (part 2)

[skirpichev]

• Issue: decimal: Use Argument Clinic #73487

[AA-Turner]

@skirpichev This PR is +7000/-2000, it's too big. Do you have any suggestions to split it up into parts?

A

[skirpichev]

How about the first commit?

[vstinner]

Same here. If you want me to review your PR, please split it into smaller PRs.

[skirpichev]

Ok, I left only first commit. Is this still too much?

[vstinner]

I would prefer a PR which would be 1/3 or 1/4 of that.

[serhiy-storchaka]

I would prefer a single PR containing all changes of the same kind than 10 different PRs.

Different kinds of changes can be presented in different PRs.

[skirpichev]

Patch updated. Now it's roughly 1/3 of the previous version:

$ git diff --stat master..origin/ac-decimal/73487-pt2 Modules/_decimal/_decimal.c Modules/_decimal/_decimal.c | 400 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------------- 1 file changed, 320 insertions(+), 80 deletions(-) 

I did no renaming of arguments. If this going to be in a dozen of prs - lets do this last. This pr includes all updates in interned strings (Include/ stuff). But in general split is more or less arbitrary.

Please review.

Documentation diff:

--- ref.txt 2025-08-16 07:41:45.006614985 +0300+++ patch.txt 2025-08-16 07:22:18.425426268 +0300@@ -291,14 +291,16 @@ | the context to the result. | | power(self, /, a, b, modulo=None) - | Compute a**b. If 'a' is negative, then 'b' must be integral. The result- | will be inexact unless 'a' is integral and the result is finite and can- | be expressed exactly in 'precision' digits. In the Python version the- | result is always correctly rounded, in the C version the result is almost- | always correctly rounded.+ | Compute a**b. | - | If modulo is given, compute (a**b) % modulo. The following restrictions- | hold:+ | If 'a' is negative, then 'b' must be integral. The result will be+ | inexact unless 'a' is integral and the result is finite and can be+ | expressed exactly in 'precision' digits. In the Python version the+ | result is always correctly rounded, in the C version the result is+ | almost always correctly rounded.+ |+ | If modulo is given, compute (a**b) % modulo. The following+ | restrictions hold: | | * all three arguments must be integral | * 'b' must be nonnegative @@ -473,10 +475,8 @@ | __floordiv__(self, value, /) | Return self//value. | - | __format__(...)- | Default object formatter.- |- | Return str(self) if format_spec is empty. Raise TypeError otherwise.+ | __format__(self, /, fmtarg, override=None)+ | Formats the Decimal according to fmtarg. | | __ge__(self, value, /) | Return self>=value. @@ -651,13 +651,16 @@ | exactly. | | exp(self, /, context=None) - | Return the value of the (natural) exponential function e**x at the given- | number. The function always uses the ROUND_HALF_EVEN mode and the result- | is correctly rounded.+ | Return the value of the (natural) exponential function e**x.+ |+ | The function always uses the ROUND_HALF_EVEN mode and the result is+ | correctly rounded. | | fma(self, /, other, third, context=None) - | Fused multiply-add. Return self*other+third with no rounding of the- | intermediate product self*other.+ | Fused multiply-add.+ |+ | Return self*other+third with no rounding of the intermediate product+ | self*other. | | >>> Decimal(2).fma(3, 5) | Decimal('11') @@ -704,18 +707,23 @@ | otherwise. | | ln(self, /, context=None) - | Return the natural (base e) logarithm of the operand. The function always- | uses the ROUND_HALF_EVEN mode and the result is correctly rounded.+ | Return the natural (base e) logarithm of the operand.+ |+ | The function always uses the ROUND_HALF_EVEN mode and the result is+ | correctly rounded. | | log10(self, /, context=None) - | Return the base ten logarithm of the operand. The function always uses the- | ROUND_HALF_EVEN mode and the result is correctly rounded.+ | Return the base ten logarithm of the operand.+ |+ | The function always uses the ROUND_HALF_EVEN mode and the result is+ | correctly rounded. | | logb(self, /, context=None) - | For a non-zero number, return the adjusted exponent of the operand as a- | Decimal instance. If the operand is a zero, then Decimal('-Infinity') is- | returned and the DivisionByZero condition is raised. If the operand is- | an infinity then Decimal('Infinity') is returned.+ | Return the adjusted exponent of the operand as a Decimal instance.+ |+ | If the operand is a zero, then Decimal('-Infinity') is returned and the+ | DivisionByZero condition is raised. If the operand is an infinity then+ | Decimal('Infinity') is returned. | | logical_and(self, /, other, context=None) | Return the digit-wise 'and' of the two (logical) operands. @@ -746,14 +754,10 @@ | values of the operands. | | next_minus(self, /, context=None) - | Return the largest number representable in the given context (or in the- | current default context if no context is given) that is smaller than the- | given operand.+ | Returns the largest representable number smaller than itself. | | next_plus(self, /, context=None) - | Return the smallest number representable in the given context (or in the- | current default context if no context is given) that is larger than the- | given operand.+ | Returns the smallest representable number larger than itself. | | next_toward(self, /, other, context=None) | If the two operands are unequal, return the number closest to the first @@ -762,11 +766,12 @@ | to be the same as the sign of the second operand. | | normalize(self, /, context=None) - | Normalize the number by stripping the rightmost trailing zeros and- | converting any result equal to Decimal('0') to Decimal('0e0'). Used- | for producing canonical values for members of an equivalence class.- | For example, Decimal('32.100') and Decimal('0.321000e+2') both normalize- | to the equivalent value Decimal('32.1').+ | Normalize the number by stripping trailing 0s+ |+ | This also change anything equal to 0 to 0e0. Used for producing+ | canonical values for members of an equivalence class. For example,+ | Decimal('32.100') and Decimal('0.321000e+2') both normalize to+ | the equivalent value Decimal('32.1'). | | number_class(self, /, context=None) | Return a string describing the class of the operand. @@ -860,8 +865,9 @@ | of the first operand are unchanged. | | sqrt(self, /, context=None) - | Return the square root of the argument to full precision. The result is- | correctly rounded using the ROUND_HALF_EVEN rounding mode.+ | Return the square root of the argument to full precision.+ |+ | The result is correctly rounded using the ROUND_HALF_EVEN rounding mode. | | to_eng_string(self, /, context=None) | Convert to an engineering-type string. @@ -1762,17 +1768,20 @@ FUNCTIONS IEEEContext(bits, /) - Return a context object initialized to the proper values for one of the- IEEE interchange formats. The argument must be a multiple of 32 and less- than IEEE_CONTEXT_MAX_BITS.+ Return a context, initialized as one of the IEEE interchange formats.++ The argument must be a multiple of 32 and less than+ IEEE_CONTEXT_MAX_BITS. getcontext() Get the current default context. localcontext(ctx=None, **kwargs) - Return a context manager that will set the default context to a copy of ctx- on entry to the with-statement and restore the previous default context when- exiting the with-statement. If no context is specified, a copy of the current+ Return a context manager for a copy of the supplied context.++ That will set the default context to a copy of ctx on entry to the+ with-statement and restore the previous default context when exiting+ the with-statement. If no context is specified, a copy of the current default context is used. setcontext(context, /)

[serhiy-storchaka]

Please extract anything related to macro-generated functions into a separate PR.

Also, instead of generating a function that will be called in single place after Argument Clinic declaration, these macros could generate only the body of the function.

[skirpichev]

Please extract anything related to macro-generated functions into a separate PR.

Last time your preference was a big PR, that does conversion in one shot. This time you suggest a more fine-grained split than even this PR... Ok.

May I at least combine changes, related to different macro in one pr?

Also, instead of generating a function that will be called in single place after Argument Clinic declaration, these macros could generate only the body of the function.

I suspect there will be no difference, if optimization is not turned off.

Are you suggesting something like this:

/*[clinic input]_decimal.Decimal.exp context: object = NoneReturn the value of the (natural) exponential function e**x.The function always uses the ROUND_HALF_EVEN mode and the result iscorrectly rounded.[clinic start generated code]*/staticPyObject*_decimal_Decimal_exp_impl(PyObject*self, PyObject*context) /*[clinic end generated code: output=c0833b6e9b8c836f input=274784af925e60c9]*/{returnDec_UnaryFuncVA(mpd_qexp)(self, context)}

[serhiy-storchaka]

There is a large difference between reviewing changes for functions which are macro-generated, and therefore you can only check the generating macro and how it is called, functions which need individual review. Please split them in separate PRs.

Instead of

/*[clinic end generated code: output=c0833b6e9b8c836f input=274784af925e60c9]*/{returnDec_UnaryFuncVA(mpd_qexp)(self, context)}

you can simply write

/*[clinic end generated code: output=c0833b6e9b8c836f input=274784af925e60c9]*/UNARY_FUNC(mpd_qexp)

[skirpichev]

There is a large difference between reviewing changes for functions which are macro-generated

Sure. Your new suggestion just contradicts to previous proposal, IMO.

Please split them in separate PRs.

Sorry. You miss my question. Should I put every macro (and related changes in AC) to a separate PR?

you can simply write

Got it, thanks. Sorry for a stupid question.

[skirpichev]

I did reversion, please review.

[serhiy-storchaka]

Why there are two definitions of PyDec_GetCurrentContext()? And why not use Argument Clinic right here?

[skirpichev]

They are different with/without WITH_DECIMAL_CONTEXTVAR define.

[serhiy-storchaka]

LGTM. 👍

This PR only converts the global functions. This can be reflected in the commit message.

[skirpichev]

This PR only converts the global functions.

No. It also changes Decimal.__format__ and Context.power.

[AA-Turner]

I've pushed one commit to fix the parameter name used in Decimal.__format__(). One open question but minor.

Thanks!

A

[AA-Turner]

We could use a custom converter in _decimal.c to keep mpd_ssize_t. Do you think it is worth the effort @serhiy-storchaka?

[serhiy-storchaka]

Are there any platforms on which mpd_ssize_t and Py_ssize_t are different? They have different names only because ssize_t is not in the C standard (only in Posix).

[skirpichev]

mpd_ssize_t is here for same reasons as Py_ssize_t.

[AA-Turner]

There is the comment:

/* * Type sizes with assertions in mpdecimal.h and pyport.h: * sizeof(size_t) == sizeof(Py_ssize_t) * sizeof(size_t) == sizeof(mpd_uint_t) == sizeof(mpd_ssize_t) */ 

so I think this is fine.