GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-115119: remove TEST_COVERAGE private macro (_decimal module) by skirpichev · Pull Request #149756 · python/cpython

Conversation

@skirpichev

@skirpichev skirpichev commented May 13, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

This amends 9eb3b14. Use unstead Py_DEBUG.

  • Issue: Unvendor libmpdec sources #115119

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@skirpichev
pythongh-115119: amend 9eb3b14, remove TEST_COVERAGE private macro
f9ad2e3
@skirpichev
skirpichev requested a review from zware May 13, 2026 04:06
@skirpichev
skirpichev requested review from AA-Turner, corona10, emmatyping and erlend-aasland as code owners May 13, 2026 04:07
@skirpichev skirpichev added the skip news label May 13, 2026
@bedevere-app bedevere-app Bot mentioned this pull request May 13, 2026
Unvendor libmpdec sources #115119
Closed
15 tasks done
@bedevere-app bedevere-app Bot added the awaiting review label May 13, 2026
@skirpichev

skirpichev commented May 31, 2026

Copy link
Copy Markdown
Member Author

CC @zware, @vstinner

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

vstinner
vstinner reviewed Jun 1, 2026
View reviewed changes

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Please change PR title to explain the change rather than referring to a commit sha1. Please elaborate also on why you made this change and what is its purpose.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@skirpichev skirpichev changed the title gh-115119: amend 9eb3b146686, remove TEST_COVERAGE private macro gh-115119: TEST_COVERAGE private macro (_decimal module) Jun 1, 2026
@skirpichev skirpichev changed the title gh-115119: TEST_COVERAGE private macro (_decimal module) gh-115119: remove TEST_COVERAGE private macro (_decimal module) Jun 1, 2026
@skirpichev
skirpichev requested a review from vstinner June 1, 2026 13:35
@vstinner

vstinner commented Jun 1, 2026

Copy link
Copy Markdown
Member

So, I had to reverse engineer your change...

This change no longer defines TEST_COVERAGE macro when building the _decimal extension with a Python debug build (--with-pydebug).

It seems like the TEST_COVERAGE macro was introduced by the commit 97b4121 (issue gh-94847 and PR gh-94848) to fix building the _decimal with --with-pydebug --with-lto (debug mode with LTO).

In the libmpdec-2.5.1 copy embedded in Python 3.15, I can see that the TEST_COVERAGE macro has an effect on inlining:

#if defined(_MSC_VER)
  #define ALWAYS_INLINE __forceinline
#elif defined (__IBMC__) || defined(LEGACY_COMPILER)
  #define ALWAYS_INLINE
  #undef inline
  #define inline
#else
  #ifdef TEST_COVERAGE
    #define ALWAYS_INLINE
  #else
    #define ALWAYS_INLINE inline __attribute__ ((always_inline))
  #endif
#endif

The bundled copy of the libmpdec decimal library has been removed in the main branch. So it's no longer needed to define the TEST_COVERAGE variable.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

vstinner
vstinner approved these changes Jun 1, 2026
View reviewed changes

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting review labels Jun 1, 2026
zware
zware approved these changes Jun 1, 2026
View reviewed changes
Comment thread Modules/_decimal/_decimal.c
*/

#ifdef TEST_COVERAGE
#ifdef Py_DEBUG

@zware zware Jun 1, 2026

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I wonder if we should go so far as to do this in Include/pyport.h, but that's not a decision I will be making.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@zware
zware merged commit 59abdf8 into python:main Jun 1, 2026
61 checks passed

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app bedevere-app Bot removed the awaiting merge label Jun 1, 2026
@skirpichev
skirpichev deleted the amend-pr133964 branch June 1, 2026 21:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vstinner vstinner vstinner approved these changes
@zware zware zware approved these changes
@erlend-aasland erlend-aasland Awaiting requested review from erlend-aasland erlend-aasland is a code owner
@corona10 corona10 Awaiting requested review from corona10 corona10 is a code owner
@AA-Turner AA-Turner Awaiting requested review from AA-Turner AA-Turner is a code owner
@emmatyping emmatyping Awaiting requested review from emmatyping emmatyping is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@skirpichev @vstinner @zware
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.