◐ Shell
clean mode source ↗

bpo-40366: Remove support for passing obsolete flags into compile by isidentical · Pull Request #19660 · python/cpython

vstinner

Choose a reason for hiding this comment

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

If CO_NESTED is no longer useful, why not removing the flag rather than adding a complicated deprecated code path? Passing flags to compile() is uncommon.

@serhiy-storchaka: Do you think that a deprecation period is needed?

Since how long CO_NESTED does nothing?

@isidentical

Since how long CO_NESTED does nothing?

2.2

@vstinner

Oh. If CO_NESTED is useless since Python 2.2 which was released 19 years ago, it's maybe time to remove it.

@isidentical

Does this removal will cover disallowing from __future__ import nested_functions (and probably generators, which was from 2.1)? I can convert this patch to that way

@vstinner

Does this removal will cover disallowing from future import nested_functions

I don't see any benefit from removing this import: it doesn't reduce the Python maintenance burden, it just breaks existing applications for free.

Most __future__ imports simply do nothing. Extract of Python/future.c:

        } else if (strcmp(feature, FUTURE_DIVISION) == 0) {
            continue;
        } else if (strcmp(feature, FUTURE_ABSOLUTE_IMPORT) == 0) {
            continue;
        } else if (strcmp(feature, FUTURE_WITH_STATEMENT) == 0) {
            continue;
        } else if (strcmp(feature, FUTURE_PRINT_FUNCTION) == 0) {
            continue;
        } else if (strcmp(feature, FUTURE_UNICODE_LITERALS) == 0) {
            continue;

@isidentical isidentical changed the title bpo-40366: Deprecate passing obsoleted flags to compile bpo-40366: Remove support for passing obsolete flags into compile

Apr 22, 2020

@furkanonder

@carljm

FWIW, removal of CO_NESTED was discussed in #96811 and rejected, since it is still useful (to detect nested functions) and used in third-party code, and setting it doesn't cause any problems.

@furkanonder

FWIW, removal of CO_NESTED was discussed in #96811 and rejected, since it is still useful (to detect nested functions) and used in third-party code, and setting it doesn't cause any problems.

In that case, I think we can close the PR. @isidentical What are your thoughts on this?

johnslavik

compile('print("\xe5")\n', '', 'exec')
self.assertRaises(ValueError, compile, chr(0), 'f', 'exec')
self.assertRaises(ValueError, compile, str('a = 1'), 'f', 'bad')
self.assertRaises(ValueError, compile, 'obsolote_flag', 'f', 'exec', CO_NESTED)

Choose a reason for hiding this comment

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

self.assertRaises(ValueError, compile, 'obsolote_flag', 'f', 'exec', CO_NESTED)
self.assertRaises(ValueError, compile, 'obsolete_flag', 'f', 'exec', CO_NESTED)

@vstinner

FWIW, removal of CO_NESTED was discussed in #96811 and rejected, since it is still useful (to detect nested functions) and used in third-party code, and setting it doesn't cause any problems.

Let met close this old inactive PR.