bpo-33407: Implement Py_DEPRECATED() on Windows by ZackerySpytz · Pull Request #8980 · python/cpython

ZackerySpytz · 2018-08-28T22:09:41Z

https://bugs.python.org/issue33407

vstinner

LGTM.

@serhiy-storchaka, @zooba: Would you mind to double check?

serhiy-storchaka · 2018-09-21T23:26:07Z

This breaks compatibility.

vstinner · 2018-09-22T01:15:20Z

This breaks compatibility.

How? Would you mind to elaborate?

serhiy-storchaka · 2018-09-22T05:59:36Z

The current usage:

extern int foo(int) Py_DEPRECATED(3.2);

This PR changes it to

Py_DEPRECATED(3.2) extern int foo(int);

It requires to rewrite all code that uses Py_DEPRECATED. This is a large breakage. Py_DEPRECATED is public and can be used in user code. We shouldn't break it in such way.

vstinner · 2018-09-22T07:26:41Z

Ok, but we have to make the change to support Visual Studio. Do you think that the macro is used outside Python itself?

vstinner

@ZackerySpytz: Can you please explain that Py_DEPRECATED() macro must now be used at the start of the function definition, rather than at the end, in a new "Changes in the C API" section at https://docs.python.org/dev/whatsnew/3.8.html#porting-to-python-3-8 ?

See https://docs.python.org/dev/whatsnew/3.7.html#changes-in-the-c-api for an example of the Python 3.7 documentation.

@ZackerySpytz: Can you please confirm that using Py_DEPRECATED() at the start of the function (rather than the end) already works in Python 3.7?

bedevere-bot · 2018-10-19T22:03:31Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

…ECATED-win

ZackerySpytz · 2019-05-24T12:59:09Z

@vstinner I have added a What's New entry for this change.

Can you please confirm that using Py_DEPRECATED() at the start of the function (rather than the end) already works in Python 3.7?

Yes, I can confirm using Py_DEPRECATED() at the start of the function already worked before this change.
I have made the requested changes; please review again.

bedevere-bot · 2019-05-24T12:59:13Z

Thanks for making the requested changes!

@vstinner: please review the changes made to this pull request.

Doc/whatsnew/3.8.rst

Include/pyport.h

ZackerySpytz · 2019-05-28T12:33:32Z

@vstinner, I have implemented all of your requested changes, and Serhiy Storchaka has also now approved this PR. Please have a look.

vstinner

I have requests on the documentation part.

Misc/NEWS.d/next/Windows/2018-08-28-17-23-49.bpo-33407.ARG0W_.rst

Doc/whatsnew/3.8.rst

Include/pyport.h

bedevere-bot · 2019-05-28T13:01:04Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

ZackerySpytz · 2019-05-28T13:48:21Z

Two deprecation warnings are emitted on MSVC because of this PR; they can be seen on AppVeyor (e.g. https://ci.appveyor.com/project/python/cpython/builds/24580662#L125):

_winapi.c
c:\projects\cpython\modules\_winapi.c(505): warning C4996: 'PyErr_SetFromWindowsErrWithUnicodeFilename': deprecated in 3.3 [C:\projects\cpython\PCbuild\pythoncore.vcxproj]
  c:\projects\cpython\include\cpython\pyerrors.h(108): note: see declaration of 'PyErr_SetFromWindowsErrWithUnicodeFilename'
c:\projects\cpython\modules\_winapi.c(1382): warning C4996: 'PyErr_SetFromWindowsErrWithUnicodeFilename': deprecated in 3.3 [C:\projects\cpython\PCbuild\pythoncore.vcxproj]
  c:\projects\cpython\include\cpython\pyerrors.h(108): note: see declaration of 'PyErr_SetFromWindowsErrWithUnicodeFilename'

I had opened https://bugs.python.org/issue36935 and GH-13355 to fix these warnings.

ZackerySpytz · 2019-05-28T13:48:36Z

@vstinner I have made the requested changes; please review again.

vstinner

LGTM.

the-knights-who-say-ni added the CLA signed label Aug 28, 2018

bedevere-bot added the awaiting review label Aug 28, 2018

ZackerySpytz force-pushed the bpo-33407-Py_DEPRECATED-win branch from ac3ad87 to 64de78b Compare August 28, 2018 22:23

bpo-33407: Implement Py_DEPRECATED() on Windows

72324ab

ZackerySpytz force-pushed the bpo-33407-Py_DEPRECATED-win branch from 64de78b to 72324ab Compare August 28, 2018 23:25

vstinner approved these changes Sep 21, 2018

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting review labels Sep 21, 2018

vstinner requested changes Oct 19, 2018

View reviewed changes

bedevere-bot added awaiting changes and removed awaiting merge labels Oct 19, 2018

ZackerySpytz added 3 commits May 15, 2019 20:48

Merge remote-tracking branch 'upstream/master' into bpo-33407-Py_DEPR…

ac030bb

…ECATED-win

Add a What's New entry.

f5a331a

Add a missing change in the new Include/cpython/unicodeobject.h file.

2d799b1

bedevere-bot added awaiting change review and removed awaiting changes labels May 24, 2019

serhiy-storchaka reviewed May 24, 2019

View reviewed changes

Doc/whatsnew/3.8.rst Outdated Show resolved Hide resolved

Include/pyport.h Outdated Show resolved Hide resolved

Use PyAPI_FUNC() in the example.

68d60fc

serhiy-storchaka approved these changes May 24, 2019

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting change review labels May 24, 2019

vstinner requested changes May 28, 2019

View reviewed changes

Misc/NEWS.d/next/Windows/2018-08-28-17-23-49.bpo-33407.ARG0W_.rst Outdated Show resolved Hide resolved

Doc/whatsnew/3.8.rst Show resolved Hide resolved

Include/pyport.h Outdated Show resolved Hide resolved

Include/pyport.h Show resolved Hide resolved

bedevere-bot added awaiting changes and removed awaiting merge labels May 28, 2019

Implement the requested changes.

ddec4b9

bedevere-bot added awaiting change review and removed awaiting changes labels May 28, 2019

vstinner approved these changes May 28, 2019

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting change review labels May 28, 2019

vstinner merged commit 3c8724f into python:master May 28, 2019

bedevere-bot removed the awaiting merge label May 28, 2019

DinoV pushed a commit to DinoV/cpython that referenced this pull request Jan 14, 2020

bpo-33407: Implement Py_DEPRECATED() on MSVC (pythonGH-8980)

53312cc

Uh oh!

Conversation

ZackerySpytz commented Aug 28, 2018 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka commented Sep 21, 2018

Uh oh!

vstinner commented Sep 22, 2018

Uh oh!

serhiy-storchaka commented Sep 22, 2018

Uh oh!

vstinner commented Sep 22, 2018 via email

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

bedevere-bot commented Oct 19, 2018

Uh oh!

ZackerySpytz commented May 24, 2019

Uh oh!

bedevere-bot commented May 24, 2019

Uh oh!

Uh oh!

Uh oh!

ZackerySpytz commented May 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bedevere-bot commented May 28, 2019

Uh oh!

ZackerySpytz commented May 28, 2019

Uh oh!

ZackerySpytz commented May 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ZackerySpytz commented Aug 28, 2018 •

edited by bedevere-bot

Loading

ZackerySpytz commented May 28, 2019 •

edited

Loading

ZackerySpytz commented May 28, 2019 •

edited

Loading