Bytecode documentation issues for Python 3.12

[MatthieuDartiailh]

As part of adding support for Python 3.12 to https://github.com/MatthieuDartiailh/bytecode I have identified a number of documentation issues both in the dismodule documentation in the bytecode section of the What's new in 3.12. Opening a separate issue for each point would just clutter the issue tracker so I will try to summarize all my findings here.

Issue in dis documentation

• COMPARE_OP oparg now uses the 4 lower bits as a cache. As a consequence:
  • the statement The operation name can be found in cmp_op[opname] is not true anymore.
  • the meaning of the cache is not documented but it can be != 0 even for a freshly compiled bytecode and as a consequence the difference is visible in dis.dis output
• MIN_INSTRUMENTED_OPCODE is not documented but needed to determine what are the "normal" opcodes
• LOAD_SUPER_ATTR description could use a code block to describe its stack effect IMO
• POP_JUMP_IF_NOT_NONE and POP_JUMP_IF_NONE are still described as pseudo-instructions even though there are not anymore in 3.12
• CALL_INSTRINSIC_2 stack manipulation description looks wrong.
  The description reads Passes STACK[-2], STACK[-1] as the arguments and sets STACK[-1] to the result, but the implementation pops 2 values from the stack
• END_SEND is not documented
• how to account for the presence of CACHE instructions following jumping instructions is not described (FOR_ITER and SEND for the time being)

Issue in What's new

• BINARY_SLICE is not mentioned
• STORE_SLICE is not mentioned
• CLEANUP_THROW is not mentioned
• RETURN_CONST is not mentioned
• LOAD_FAST_CHECK is not mentioned
• END_SEND is not mentioned
• CALL_INSTRINSIC_1/2 are not mentioned
• FOR_ITER new behavior is not mentioned
• The fact that POP_JUMP_IF_* family of instructions are now real instructions is not mentioned
• YIELD_VALUE need for an argument is not mentioned
• The addition of dis.hasexc is not mentioned

Linked PRs

• gh-107457: update dis documentation with changes in 3.12 #108900
• [3.12] gh-107457: update dis documentation with changes in 3.12 (GH-108900) #110985

[iritkatriel]

@MatthieuDartiailh Thanks for this. Are you working on a PR?

[MatthieuDartiailh]

Not at the moment. If I manage to finish my work on bytecode, I will make a PR, but feel free to beat me to it.

[MatthieuDartiailh]

Also note that while I could address many of those, I did not figure out yet what the lower 4 bits of COMPARE_OP are for.

[gvanrossum]

Also note that while I could address many of those, I did not figure out yet what the lower 4 bits of COMPARE_OP are for.

See this fragment from Include/internal/pycore_code.h:

/* Comparison bit masks. */

/* Note this evaluates its arguments twice each */
#define COMPARISON_BIT(x, y) (1 << (2 * ((x) >= (y)) + ((x) <= (y))))

/*
 * The following bits are chosen so that the value of
 * COMPARSION_BIT(left, right)
 * masked by the values below will be non-zero if the
 * comparison is true, and zero if it is false */

/* This is for values that are unordered, ie. NaN, not types that are unordered, e.g. sets */
#define COMPARISON_UNORDERED 1

#define COMPARISON_LESS_THAN 2
#define COMPARISON_GREATER_THAN 4
#define COMPARISON_EQUALS 8

#define COMPARISON_NOT_EQUALS (COMPARISON_UNORDERED | COMPARISON_LESS_THAN | COMPARISON_GREATER_THAN)

[MatthieuDartiailh]

Thanks !

I also found this fragment in compile.c that completes the picture:

static const int compare_masks[] = {
    [Py_LT] = COMPARISON_LESS_THAN,
    [Py_LE] = COMPARISON_LESS_THAN | COMPARISON_EQUALS,
    [Py_EQ] = COMPARISON_EQUALS,
    [Py_NE] = COMPARISON_NOT_EQUALS,
    [Py_GT] = COMPARISON_GREATER_THAN,
    [Py_GE] = COMPARISON_GREATER_THAN | COMPARISON_EQUALS,
};

[carljm]

Thanks for the report!

LOAD_SUPER_ATTR description could use a code block to describe its stack effect IMO

Re-reading this doc entry, I think it could be clearer that it pops three elements from the stack and pushes either one or two, depending on low bit of oparg (just like LOAD_ATTR). Would this prose clarification address your issue?

I'm not convinced that a code block would be better than a small addition to the prose. A code block is more verbose and introduces more distracting details, which would either be confusing because they differ from the actual implementation, or confusing because they match the actual implementation and thus add complexity that isn't needed in the documentation. But if you have an example of what a code block clarification would look like that you think is better than prose, I'm open to considering it.

[gvanrossum]

Not to mention this magical code in bytecodes.c:

            double dleft = PyFloat_AS_DOUBLE(left);
            double dright = PyFloat_AS_DOUBLE(right);
            // 1 if NaN, 2 if <, 4 if >, 8 if ==; this matches low four bits of the oparg
            int sign_ish = COMPARISON_BIT(dleft, dright);
            _Py_DECREF_SPECIALIZED(left, _PyFloat_ExactDealloc);
            _Py_DECREF_SPECIALIZED(right, _PyFloat_ExactDealloc);
            res = (sign_ish & oparg) ? Py_True : Py_False;

and

            Py_ssize_t ileft = _PyLong_CompactValue((PyLongObject *)left);
            Py_ssize_t iright = _PyLong_CompactValue((PyLongObject *)right);
            // 2 if <, 4 if >, 8 if ==; this matches the low 4 bits of the oparg
            int sign_ish = COMPARISON_BIT(ileft, iright);
            _Py_DECREF_SPECIALIZED(left, (destructor)PyObject_Free);
            _Py_DECREF_SPECIALIZED(right, (destructor)PyObject_Free);
            res = (sign_ish & oparg) ? Py_True : Py_False;

That completes the picture; these are the only two uses (but they are in very performance-critical code, hence all these shenanigans).

[MatthieuDartiailh]

I just noticed that END_SEND does not appear in the dis doc, I will edit my first message.

[MatthieuDartiailh]

If anybody could enlighten me regarding the use of END_SEND and CLEANUP_THROW I would greatly appreciate it. I am currently having some issues when trying to compute the stack size for async function using async with in https://github.com/MatthieuDartiailh/bytecode.

[gvanrossum]

For END_SEND, all I know is that it was added to support PEP 669 (instrumentation) in gh-103083.

For CLEANUP_THROW, look at gh-98001; that seems to indicate it has to do with line number tables.

[MatthieuDartiailh]

It seems to me that SEND documentation is either wrong or poorly phrased. It says:

If the call raises StopIteration, pop both items, push the exception’s value attribute, and increment the bytecode counter by delta.

Which to me sounds like two values are removed from the stack and one is pushed giving a stack of -1 but SEND stack effect is 0

Another undocumented feature if that when jumping the offset is augmented by 1 compared to the oparg !!! which is rather surprising.

EDIT: same issue for FOR_ITER

[gvanrossum]

For SEND, comparing the description to the actual code in the main branch, it looks like the docs are just wrong -- it leaves STACK[-2] (the generator or generator-ish object) in place. According to blame, that sentence was originally written by @brandtbucher about a year ago -- maybe it was always wrong, or maybe the implementation of SEND has changed since then? (The new sentence doesn't appear in the 3.11 docs, only in 3.12 and 3.13.)

I'm not sure the FOR_ITER docs are incorrect -- it doesn't have the same sentence in its docs, nor are the semantics the same. It's clear in main that it pushes an extra value if iter.__next__() produces one, and jumps by oparg if it raises StopIteration. The implementation pops the iterator and jumps one instruction more, but that's considered an optimization -- the instruction it skips is END_FOR, which pops two values off the stack, following the fiction that FOR_ITER always pushes a value. It is a requirement that the instruction pointed at by oparg is always END_FOR.