Skip to content

bpo-35059, libmpdec: Add missing EXTINLINE in mpdecimal.h#10128

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
vstinner merged 1 commit into from
Oct 26, 2018
Merged

bpo-35059, libmpdec: Add missing EXTINLINE in mpdecimal.h #10128

vstinner merged 1 commit into from
Oct 26, 2018

Conversation

@vstinner
Copy link
Member

@vstinnervstinner commented Oct 26, 2018•
edited by bedevere-bot
Loading

Declare functions with EXTINLINE:

  • mpd_del()
  • mpd_uint_zero()
  • mpd_qresize()
  • mpd_qresize_zero()
  • mpd_minalloc()

These functions are implemented with "inline" or "ALWAYS_INLINE", but
declared without inline which cause linker error on Visual Studio in
Debug mode when using /Ob1.

https://bugs.python.org/issue35059

@vstinner
bpo-35059, libmpdec: Add missing EXTINLINE in mpdecimal.h
afb16a8
Declare functions with EXTINLINE: * mpd_del() * mpd_uint_zero() * mpd_qresize() * mpd_qresize_zero() * mpd_minalloc() These functions are implemented with "inline" or "ALWAYS_INLINE", but declared without inline which cause linker error on Visual Studio in Debug mode when using /Ob1.
@vstinnervstinner mentioned this pull request Oct 26, 2018
Closed
@vstinner
Copy link
MemberAuthor

vstinner commented Oct 26, 2018

"Build AMD64 Windows7 SP1 custom #3 is complete: Success"

Good! That's the one buildbot (AMD64 Windows7 SP1 3.x) where the compilation failed!

@vstinnervstinner merged commit 3b1cba3 into python:masterOct 26, 2018
@vstinnervstinner deleted the decimal_inline branch October 26, 2018 15:07
@vstinner
Copy link
MemberAuthor

vstinner commented Oct 26, 2018

@skrah: I'm sorry, I couldn't wait for your review since I broke most Windows buildbots and I wanted to fix the CI as soon as possible, since I have more PRs to push in https://bugs.python.org/issue35059 and I would like to make sure that they don't break anything!

I tried to find how to propose this change upstream, but I failed to find a (git, hg, svn, ...) repository, not a document explaining how to contribute to libmpdec.

If you dislike this change, maybe I can revert it and I can change the configuration of the "_decimal" project in the Visual Studio solution to not compile it with /Ob1 in Debug mode (only this project).

But it looks like a bug in libmpdec.h, since all other functions implemented with inline or ALWAYS_INLINE are already declared with EXTINLINE.

Maybe the compiler is smarter and doesn't cause the linker bug in Release mode. No idea why the bug only occurs on specific Windows buildbots (not on AppVeyor, not on my Windows VM, not on Linux, etc.)

Linker bug fixed by this PR:
#10094 (comment)

@vstinner
Copy link
MemberAuthor

vstinner commented Oct 26, 2018

I will wait to double check that this change really fix all Windows buildbots.

Then the question is if the change should be backported to 3.6 and 3.7.

@vstinnervstinner mentioned this pull request Oct 26, 2018
Merged
@vstinner
Copy link
MemberAuthor

vstinner commented Oct 26, 2018

I checked buildbots: my change fixed all Windows buildbots, good.

@miss-islington
Copy link
Contributor

miss-islington commented Oct 26, 2018

Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.6.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

miss-islington commented Oct 26, 2018

Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖

@bedevere-bot
Copy link

bedevere-bot commented Oct 26, 2018

GH-10134 is a backport of this pull request to the 3.6 branch.

@bedevere-bot
Copy link

bedevere-bot commented Oct 26, 2018

GH-10135 is a backport of this pull request to the 3.7 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 26, 2018
@vstinner@miss-islington
bpo-35059, libmpdec: Add missing EXTINLINE in mpdecimal.h (pythonGH-1…
1805816
…0128) Declare functions with EXTINLINE: * mpd_del() * mpd_uint_zero() * mpd_qresize() * mpd_qresize_zero() * mpd_minalloc() These functions are implemented with "inline" or "ALWAYS_INLINE", but declared without inline which cause linker error on Visual Studio in Debug mode when using /Ob1. (cherry picked from commit 3b1cba3) Co-authored-by: Victor Stinner <[email protected]>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 26, 2018
@vstinner@miss-islington
bpo-35059, libmpdec: Add missing EXTINLINE in mpdecimal.h (pythonGH-1…
722c363
…0128) Declare functions with EXTINLINE: * mpd_del() * mpd_uint_zero() * mpd_qresize() * mpd_qresize_zero() * mpd_minalloc() These functions are implemented with "inline" or "ALWAYS_INLINE", but declared without inline which cause linker error on Visual Studio in Debug mode when using /Ob1. (cherry picked from commit 3b1cba3) Co-authored-by: Victor Stinner <[email protected]>
miss-islington added a commit that referenced this pull request Oct 26, 2018
@miss-islington@vstinner
bpo-35059, libmpdec: Add missing EXTINLINE in mpdecimal.h (GH-10128)
95cfb81
Declare functions with EXTINLINE: * mpd_del() * mpd_uint_zero() * mpd_qresize() * mpd_qresize_zero() * mpd_minalloc() These functions are implemented with "inline" or "ALWAYS_INLINE", but declared without inline which cause linker error on Visual Studio in Debug mode when using /Ob1. (cherry picked from commit 3b1cba3) Co-authored-by: Victor Stinner <[email protected]>
miss-islington added a commit that referenced this pull request Oct 26, 2018
@miss-islington@vstinner
bpo-35059, libmpdec: Add missing EXTINLINE in mpdecimal.h (GH-10128)
7eac88a
Declare functions with EXTINLINE: * mpd_del() * mpd_uint_zero() * mpd_qresize() * mpd_qresize_zero() * mpd_minalloc() These functions are implemented with "inline" or "ALWAYS_INLINE", but declared without inline which cause linker error on Visual Studio in Debug mode when using /Ob1. (cherry picked from commit 3b1cba3) Co-authored-by: Victor Stinner <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rhettingerrhettingerAwaiting requested review from rhettinger

@skrahskrahAwaiting requested review from skrah

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@vstinner@miss-islington@bedevere-bot@the-knights-who-say-ni