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 = None
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.
[clinic start generated code]*/

static PyObject *
_decimal_Decimal_exp_impl(PyObject *self, PyObject *context)
/*[clinic end generated code: output=c0833b6e9b8c836f input=274784af925e60c9]*/
{
    return Dec_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]*/
{
    return Dec_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.