Skip to content
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.

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

Make documentation about macros in C API explicit about rvalue vs statement #61789

Open
larryhastings opened this issue Mar 31, 2013 · 7 comments
Labels
docs Documentation in the Doc dir topic-C-API type-feature A feature request or enhancement

Comments

@larryhastings
Copy link
Contributor

BPO 17589
Nosy @loewis, @birkenfeld, @amauryfa, @mdickinson, @pitrou, @larryhastings, @markshannon

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2013-03-31.19:39:15.734>
labels = ['expert-C-API', 'type-feature', 'docs']
title = 'Make documentation about  macros in C API explicit about rvalue vs statement'
updated_at = <Date 2020-06-25.09:41:19.776>
user = 'https://github.com/larryhastings'

bugs.python.org fields:

activity = <Date 2020-06-25.09:41:19.776>
actor = 'vstinner'
assignee = 'docs@python'
closed = False
closed_date = None
closer = None
components = ['Documentation', 'C API']
creation = <Date 2013-03-31.19:39:15.734>
creator = 'larry'
dependencies = []
files = []
hgrepos = []
issue_num = 17589
keywords = []
message_count = 7.0
messages = ['185646', '185667', '185668', '185711', '186317', '186367', '186369']
nosy_count = 9.0
nosy_names = ['loewis', 'georg.brandl', 'amaury.forgeotdarc', 'mark.dickinson', 'pitrou', 'larry', 'Arfrever', 'docs@python', 'Mark.Shannon']
pr_nums = []
priority = 'normal'
resolution = None
stage = 'needs patch'
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue17589'
versions = ['Python 3.4']

@larryhastings
Copy link
Contributor Author

CPython API "functions" implemented as macros can expand into either
rvalues or statements. Most expand into statements (often using the
do {} while (0) syntactic sugar trick), but occasionally they're legal
as rvalues.

As of this writing Py_INCREF works as an rvalue. But discussion on
another tracker issue (bpo-17206) proposes changing the implementation
in such a way that it will only be usable as a statement. Although
it's mildly unlikely, it's certainly possible that this will break
somebody's code.

I propose that the documentation make an explicit ruling on whether
macros are usable as rvalues or as statements. Perhaps a blanket
statement would suffice, "all macros are only supported for use as
statements except where explicitly noted", then annotate specific
macros that are supported for use as rvalues. Though that raises the
question of acknowledging in the documentation that some things are
macros--I think the documentation glosses over that fact right now.

Note: I added you three (Georg, Mark, Martin) as I thought you might
have an opinion on this one way or the other. If you're not interested,
my apologies.

@larryhastings larryhastings added docs Documentation in the Doc dir type-feature A feature request or enhancement labels Mar 31, 2013
@birkenfeld
Copy link
Member

There are also some macros usable as lvalues, such as Py_REFCNT or Py_SIZE (they aren't documented at all currently).

Anyway, documenting as statement-only unless explicitly stated differently is fine with me.

@pitrou
Copy link
Member

pitrou commented Mar 31, 2013

Py_INCREF usable as an rvalue sounds more like an accident than a deliberate feature, and it would be IMO counter-productive to codify this behaviour in the docs.

As for the lvalue usage of Py_REFCNT and Py_SIZE, I think it would be better if it were limited to CPython core. But arguably writers of extension types may want to mutate the size field of a varsize object.

@larryhastings
Copy link
Contributor Author

Py_INCREF usable as an rvalue sounds more like an accident
than a deliberate feature

I'd go with "misfeature", but in no way is it accidental. The coding deliberately preserves the rvalue-ness of it, c.f. _Py_REF_DEBUG_COMMA.

@amauryfa
Copy link
Member

amauryfa commented Apr 8, 2013

There are some extension modules (pytables) that do
return Py_INREF(x), x;
and Py_RETURN_NONE is also defined with a comma expression.

Oh, and Cython:
#define __Pyx_PyBool_FromLong(b) ((b) ? (Py_INCREF(Py_True), Py_True) : (Py_INCREF(Py_False), Py_False))

@larryhastings
Copy link
Contributor Author

Amaury: I'd appreciate it if you'd bring those examples up on bug 17206. If we're going to change the semantics of Py_INCREF I'd prefer we did it with our eyes wide open.

@larryhastings
Copy link
Contributor Author

Oh, and, yes, it's true that Py_RETURN_NONE currently takes advantage of Py_INCREF being an rvalue, and changing Py_INCREF to a statement would break the existing implementation. But Py_RETURN_NONE itself is of necessity a statement. We would simply change Py_RETURN_NONE's implementation to multiple statements, probably with the do { ... } while(0) trick, so it worked again. I'd be shocked if that change broke any existing code. So that's no big deal.

Having external code that depends on Py_INCREF being an rvalue is my concern, and what I hoped you'd bring up on bug bpo-17206.

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir topic-C-API type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

5 participants