[issue36347] Renaming the constants for the .flags of PyMemberDef

[miss-islington]

miss-islington  added the comment:


New changeset 24bba8cf5b8db25c19bcd1d94e8e356874d1c723 by Jeroen Demeyer in 
branch 'master':
bpo-36347: stop using RESTRICTED constants (GH-12684)
https://github.com/python/cpython/commit/24bba8cf5b8db25c19bcd1d94e8e356874d1c723


--
nosy: +miss-islington

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36347] Renaming the constants for the .flags of PyMemberDef

[Stéphane Wirtel]

Stéphane Wirtel  added the comment:

For that, where did you see the macro was not used? maybe by an external
library.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36347] Renaming the constants for the .flags of PyMemberDef

[Jeroen Demeyer]

Change by Jeroen Demeyer :


--
pull_requests: +12612

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36347] Renaming the constants for the .flags of PyMemberDef

[Ronald Oussoren]

Ronald Oussoren  added the comment:

A discussion on the use of enum and deprecation warnings might be useful, 
although there is a significant risk on bike shedding.

I agree that formally deprecating READONLY can lead to uglier code in extension 
that use the value and need to support anything beyond the bleeding edge 
(although the complication isn't that bad: just add a conditional definition of 
PY_READONLY).  I'm personally not to worried about this, and generally prefer 
being more aggressive with adding deprecation warnings.


W.r.t. C++ and enums: that should be ok, especially when using anonymous enums 
that are basically only used to name constants.  

Bitmask with enums can be more problematic when using named enums that are also 
used for variable definitions because C++ compilers are allowed to use the 
minimal variable size that can represent all defined values (that's also a 
problem for the ABI), but it is also possible to use scoped enums to specify 
the size of values.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36347] Renaming the constants for the .flags of PyMemberDef

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

I am against deprecating READONLY. This will break virtually every third-party 
extension that use it. Many projects are strong about compiler warning and will 
need to rewrite the code that worked for years.

I think that we can add the deprecation warning only after the last version 
that do not have PY_READONLY (3.7) will be no longer supported. I.e. in 3.11 or 
something around.

Also I am not sure about using enums for flags. Would not this cause problems 
on C++?

Since this is an extending of the C API, it would be better to discuss the 
necessary of adding PY_READWRITE on Python-Dev.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36347] Renaming the constants for the .flags of PyMemberDef

[Stéphane Wirtel]

Stéphane Wirtel  added the comment:

With the help of Victor, I don't need to implement __attribute__((deprecated)), 
this one is already defined in pyport.h

So, I have updated my PR.

Next step, update all the references of READONLY, ..., by the new constants.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36347] Renaming the constants for the .flags of PyMemberDef

[Stéphane Wirtel]

Stéphane Wirtel  added the comment:

or with this feature of MSVS 
https://docs.microsoft.com/en-us/cpp/preprocessor/deprecated-c-cpp?view=vs-2017

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36347] Renaming the constants for the .flags of PyMemberDef

[Stéphane Wirtel]

Stéphane Wirtel  added the comment:

@steve.dower

Can I use this 
https://docs.microsoft.com/en-us/cpp/cpp/deprecated-cpp?view=vs-2017 for the 
deprecation?

I am going to try with my PR but I would like to have your approbation if it's 
the right solution or not.

Thank you

--
nosy: +steve.dower

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36347] Renaming the constants for the .flags of PyMemberDef

[Stéphane Wirtel]

Stéphane Wirtel  added the comment:

Ronald,

Please, could you check this PR and give me your advice,
For the moment, it's just re-declaration of the current constants with
the PY_ prefix and with an enumeration and the __attribute__(deprecated)

The PR is in Draft mode and I will update it with your remarks and the
feedback from the Python-dev ML

PR: https://github.com/python/cpython/pull/12527

There is a check list for me.

https://github.com/python/cpython/pull/12527#issuecomment-476002321

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36347] Renaming the constants for the .flags of PyMemberDef

[Stéphane Wirtel]

Change by Stéphane Wirtel :


--
pull_requests: +12478

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36347] Renaming the constants for the .flags of PyMemberDef

[Stéphane Wirtel]

Stéphane Wirtel  added the comment:

Hi Ronald,

I have checked with gcc and clang on my computer (fedora 29)

This flag is defined in CLANG for the GNU and C++11 standard

See this table:

https://clang.llvm.org/docs/AttributeReference.html#deprecated

We could integrate it because the unused flag is also defined in GNU and
C++11 standard.

https://clang.llvm.org/docs/AttributeReference.html#maybe-unused-unused

For gcc, there is this reference

https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#Common-Variable-Attributes

### CLANG
LC_ALL=C clang test.c -o test
test.c:14:13: warning: 'READONLY' is deprecated: use PY_READONLY
[-Wdeprecated-declarations]
int i = READONLY;
^
test.c:8:27: note: 'READONLY' has been explicitly marked deprecated here
READONLY __attribute((deprecated("use PY_READONLY"))) = PY_READONLY,
  ^
1 warning generated.

LC_ALL=C clang --version test.c -o test
clang version 7.0.1 (Fedora 7.0.1-6.fc29)
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

### GCC
LC_ALL=C gcc --version test.c -o test
gcc (GCC) 8.3.1 20190223 (Red Hat 8.3.1-2)
Copyright (C) 2018 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

LC_ALL=C gcc test.c -o test
test.c: In function 'main':
test.c:14:5: warning: 'READONLY' is deprecated: use PY_READONLY
[-Wdeprecated-declarations]
 int i = READONLY;
 ^~~
test.c:8:5: note: declared here
 READONLY __attribute((deprecated("use PY_READONLY"))) = PY_READONLY,
 ^~~~

But I need to know the required version of gcc and clang, in this case,
we need to know if these attributes are supported by the compiler, but
normally this is the case.

Maybe, I should ask on the python-dev mailing list.

Thank you for your advice,

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36347] Renaming the constants for the .flags of PyMemberDef

[Stéphane Wirtel]

Stéphane Wirtel  added the comment:

Pretty good.

In fact I wanted to replace the #define by const int and try to find a solution 
with gcc for a déprécation warning, but I was not sure about a such solution.

Your proposal is really interesting and open new solutions.

Thank you

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36347] Renaming the constants for the .flags of PyMemberDef

[Ronald Oussoren]

Ronald Oussoren  added the comment:

Switches from #define's to an enum would allow explictly deprecating the old 
name (at least with clang and probably with GCC as well):

clang -c -Wall t.c
t.c:12:10: warning: 'READONLY' is deprecated: use PY_READONLY 
[-Wdeprecated-declarations]
int i = READONLY;
^
t.c:7:26: note: 'READONLY' has been explicitly marked deprecated here
READONLY __attribute__((deprecated("use PY_READONLY"))) = PY_READONLY
^
1 warning generated.


For this source code:

#include 

enum {
PY_READWRITE = 0,
PY_READONLY = 1,

READONLY __attribute__((deprecated("use PY_READONLY"))) = PY_READONLY
};

int main(void)
{
int i = READONLY;

printf("%d\n", i);
return 0;
}

I'm not sure if it worthwhile switch to an enum here, the CPython source code 
isn't consistent in using enums for constants like this.

--
nosy: +ronaldoussoren

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36347] Renaming the constants for the .flags of PyMemberDef

[Stéphane Wirtel]

Stéphane Wirtel  added the comment:

I have closed the PR 12410. Feel free to submit another PR.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36347] Renaming the constants for the .flags of PyMemberDef

[Stéphane Wirtel]

Stéphane Wirtel  added the comment:

Hi @Josh,

In the PR, I don't remove READONLY. READONLY becomes an alias of
PY_READONLY.

I think I should split my PR in two PRs

1. Improve the current documentation and the code
2. Change the code in the Modules/ with the new macros.

@Serhyi what is your suggestion?

Thank you

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36347] Renaming the constants for the .flags of PyMemberDef

[Josh Rosenberg]

Josh Rosenberg  added the comment:

Pretty sure you can't actually get rid of READONLY; it's part of the public 
API. Adding PY_READONLY to match PY_READWRITE is fine, but unlike 
WRITE_RESTRICTED/PY_WRITE_RESTRICTED (which isn't used at all in Py3,  has been 
non-functional since 2.3, and caused compilation errors on Visual Studio; more 
details on #36355), READONLY is commonly used by third party C extensions, 
doesn't have any known conflicts with compiler headers, and can't be removed.

Go ahead and add PY_READONLY, and change the documentation to prefer it, but:

#define READONLY 1

needs to stick around in the header indefinitely.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36347] Renaming the constants for the .flags of PyMemberDef

[Stéphane Wirtel]

Change by Stéphane Wirtel :


--
title: Add the constant READWRITE for PyMemberDef -> Renaming the constants for 
the .flags of PyMemberDef

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com