gh-115419: Change default sym to not_null #116562
Conversation
Fidget-Spinner
changed the title
Fidget-Spinner
changed the title
Lib/test/test_capi/test_opt.py
Outdated
| """Note: this function must be executed in a subprocess, because when other tests are run, | ||
| the globals of this module might get affected, causing globals to constant promotion | ||
| to fail. | ||
| """ |
There was a problem hiding this comment.
I betcha you can do without a subprocess. Instead, just create a namespace and use that, like several other tests do. The namespace directory acts as the globals for the function defined in it. Try it!
There was a problem hiding this comment.
Sorry my Python is actually somewhat rusty -- which namespace are you referring to? I don't remember types.SimpleNamespace being that powerful.
There was a problem hiding this comment.
Just a dict -- look at test_type_inconsistency() in the same file for a complete example. Because it says exec(src, ns, ns) the ns dict is used as the globals for the function, and you can add/update its "globals" using ns["some_name"] = some_value.
|
There are two parts to this, AFAICT.
1 seems like a worthwhile change, but 2 is incorrect as functions are mutable. Could you move 1 to its own PR? |
Fidget-Spinner
changed the title
|
FWIW This may be off-topic and a pipe dream, but I have a feeling that we ought to consider deprecating certain rare mutable attributes. E.g. assignments to |
| (void)owner; | ||
| OUT_OF_SPACE_IF_NULL(attr = sym_new_not_null(ctx)); | ||
| if (oparg & 1) { | ||
| OUT_OF_SPACE_IF_NULL(self_or_null = sym_new_unknown(ctx)); |
There was a problem hiding this comment.
At some point in the future we will be able to tell whether the attribute is a method or not, and hence we will be able to set it to either null or not_null -- but not today, right?
There was a problem hiding this comment.
No not for _LOAD_ATTR - this opcode is special, it means we dont know anything and it's only decideable at runtime.
|
The test suite crashed without _LOAD_ATTR manual definition |
Changes the default symbol to
not-nullbecause in reality, most of the stuff in CPython pushes non-null to the stack.