put opcode information in one place #62061

benjaminp · 2013-04-28T15:14:50Z

BPO	17861
Nosy	@loewis, @Yhg1s, @db3l, @vstinner, @benjaminp, @ned-deily, @ezio-melotti, @merwok, @serhiy-storchaka, @kushaldas
Files	generate_opcode_h.py: opcode.h generator generate_opcode_h.py: second version of the patch generate_opcode_h.py: Third revision of the patch. issue17861_v1.patch: patch with code and makefile update issue17861_v2.patch: version two of the patch issue17861_v3.patch: New patch with .hgtouch file details. issue17861_v4.patch: New patch with proper changesets. issue17861_v5.patch: New patch from a clean repo, with changed opcode.h issue17861_v6.patch: New patch with changes as suggested by Brett. generate_opcode_h.diff generate_opcode_h.diff: Support running under Python 2.5

^{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 = 'https://github.com/kushaldas'
closed_at = <Date 2018-04-06.23:51:26.582>
created_at = <Date 2013-04-28.15:14:49.574>
labels = ['interpreter-core', 'easy', 'type-feature']
title = 'put opcode information in one place'
updated_at = <Date 2018-04-06.23:51:26.582>
user = 'https://github.com/benjaminp'

bugs.python.org fields:

activity = <Date 2018-04-06.23:51:26.582>
actor = 'ned.deily'
assignee = 'kushal.das'
closed = True
closed_date = <Date 2018-04-06.23:51:26.582>
closer = 'ned.deily'
components = ['Interpreter Core']
creation = <Date 2013-04-28.15:14:49.574>
creator = 'benjamin.peterson'
dependencies = []
files = ['30045', '30046', '30048', '30064', '30066', '34842', '34855', '34872', '34883', '34930', '35078']
hgrepos = []
issue_num = 17861
keywords = ['patch', 'easy']
message_count = 22.0
messages = ['187986', '187998', '188000', '188005', '188074', '188079', '215915', '216004', '216179', '216233', '216294', '216348', '216358', '216591', '216599', '216620', '217409', '217415', '217417', '217469', '217500', '314258']
nosy_count = 11.0
nosy_names = ['loewis', 'twouters', 'db3l', 'vstinner', 'benjamin.peterson', 'ned.deily', 'ezio.melotti', 'eric.araujo', 'python-dev', 'serhiy.storchaka', 'kushal.das']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue17861'
versions = ['Python 3.4']

benjaminp · 2013-04-28T15:14:49Z

Right now, opcode information is duplicated in Include/opcode.h and Lib/opcode.py. I should only have to update one manually and have the other one generated automatically. opcode.h should probably be generated by a script from opcode.py.

kushaldas · 2013-04-28T16:25:06Z

Here is a simple script which prints the opcode.h in console. We can redirect it as required.

kushaldas · 2013-04-28T16:51:21Z

Second version of the script with the fix for HAVE_ARGUMENT

kushaldas · 2013-04-28T17:05:34Z

Third revision with fixed empty spaces at the end of the lines.

kushaldas · 2013-04-29T16:43:33Z

As we discussed on #python-dev channel this new patch includes the script in Tools/scripts/generate_opcode_h.py and it also contains the required Makefile.pre.in change so that it gets auto(re)generated at compile time if required.

kushaldas · 2013-04-29T17:34:12Z

Version 2 of the patchset edited as per review.

merwok · 2014-04-11T06:15:14Z

Patch looks good to me.

loewis · 2014-04-13T09:03:11Z

Please add the file to .hgtouch as well, and verify that "make touch" works correctly.

kushaldas · 2014-04-14T19:09:33Z

New patch with .hgtouch file details.

kushaldas · 2014-04-14T22:09:59Z

New patch with proper changesets.

kushaldas · 2014-04-15T14:24:00Z

New patch from a clean repo, with changed opcode.h

kushaldas · 2014-04-15T17:53:23Z

New patch with changes as suggested by Brett.

python-dev · 2014-04-15T18:22:27Z

New changeset 56e7fa27f979 by Kushal Das in branch 'default':
Closes bpo-17861: Autogenerate Include/opcode.h from opcode.py.
http://hg.python.org/cpython/rev/56e7fa27f979

Yhg1s · 2014-04-16T21:06:23Z

FYI, this broke building in a separate object directory (again!) for multiple reasons: it's running the script without specifying $(srcdir), and it's writing to $(srcdir)/Include/opcode.h (where $(srcdir) may be unwritable), and it's picking up the wrong opcode module: the sys.path mucking in Tools/generate_opcode_h.py isn't inserting the right directory for 'import opcode' to pick up the target-Python's opcode.py. It should probably be using execfile() and have the name of the file passed in, instead.

Even if it were importing the right opcode.py, relying on 'import' or 'execfile' here sounds like a bad idea: it means the opcode module of the *target* Python needs to be compatible with the Python that the generate_opcode_h.py script runs with. Since the syntax of opcode.h is much more constrained, a more flexible approach, in my opinion, would be to parse opcode.h to produce opcode.py (or an _opcode.py with just the definitions, that opcode.py would then import.)

Yhg1s · 2014-04-16T21:34:29Z

Here's a minimal patch to at least make the current mechanism work when using a separate build directory.

I still don't like the fact that this is importing opcode.py in a different Python than the target Python. Nor do I like that the script hardcodes information about the opcodes (specifically, the first opcode to HAVE_ARGUMENT.) This feels like it's not actually better for maintenance than the previous solution.

python-dev · 2014-04-16T23:15:22Z

New changeset 2b187c9e3e92 by Thomas Wouters in branch 'default':
Fix Tools/scripts/generate_opcode_h.py from issue bpo-17861 to work correctly
http://hg.python.org/cpython/rev/2b187c9e3e92

db3l · 2014-04-28T19:54:55Z

This generator script breaks the daily OSX DMG builds on my bolen-dmg buildslave for the 3.x branch.

The issue is with the use of "with" as the slave uses Python 2.5 to build the installer. Now, that's old, and I'm not even sure how necessary the daily builds are to keep running, but I believe the Mac build-installer script still tries to target 2.5 as a minimum - though I'm not sure if that's true for all build tools. But the fix is fairly simple (a __future__ import).

See attached patch.

Alternatively, if the minimum Python to build the installer should be bumped, I can work on that for the slave.

python-dev · 2014-04-28T20:47:25Z

New changeset 1fd9c3f6cf68 by Ned Deily in branch 'default':
Issue bpo-17861: Allow generate_opcode_h to run with a system Python 2.5.
http://hg.python.org/cpython/rev/1fd9c3f6cf68

ned-deily · 2014-04-28T20:53:46Z

Thanks, David. Ideally, the generator script shouldn't run during an installer build since presumably the generated file should be up-to-date in the repo. "make touch" could handle that but the installer build does use a separate build/object directory and doesn't currently use "make touch". For Python 3.5, the installer build now does need at least a Python 2.6 to build the documentation but there's no reason to unnecessarily break the main build with such a simple fix available.

loewis · 2014-04-29T05:36:21Z

It would be possible to have the daily DMG builder run "make touch".

ned-deily · 2014-04-29T08:46:51Z

Martin, it could if "make touch" worked when building outside of the source directory (bpo-21383).

serhiy-storchaka · 2018-03-22T14:08:50Z

Currently an explicit make regen-opcode and make regen-opcode-targets (or just make regen-all) are needed for regenerating C files from opcode.py.

Is anything left to do in this issue?

benjaminp added interpreter-core (Objects, Python, Grammar, and Parser dirs) easy type-feature A feature request or enhancement labels Apr 28, 2013

python-dev mannequin closed this as completed Apr 15, 2014

Yhg1s reopened this Apr 16, 2014

Yhg1s assigned kushaldas Apr 16, 2014

ned-deily closed this as completed Apr 6, 2018

ezio-melotti transferred this issue from another repository Apr 10, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

put opcode information in one place #62061

put opcode information in one place #62061

benjaminp commented Apr 28, 2013

benjaminp commented Apr 28, 2013

kushaldas commented Apr 28, 2013

kushaldas commented Apr 28, 2013

kushaldas commented Apr 28, 2013

kushaldas commented Apr 29, 2013

kushaldas commented Apr 29, 2013

merwok commented Apr 11, 2014

loewis mannequin commented Apr 13, 2014

kushaldas commented Apr 14, 2014

kushaldas commented Apr 14, 2014

kushaldas commented Apr 15, 2014

kushaldas commented Apr 15, 2014

python-dev mannequin commented Apr 15, 2014

Yhg1s commented Apr 16, 2014

Yhg1s commented Apr 16, 2014

python-dev mannequin commented Apr 16, 2014

db3l commented Apr 28, 2014

python-dev mannequin commented Apr 28, 2014

ned-deily commented Apr 28, 2014

loewis mannequin commented Apr 29, 2014

ned-deily commented Apr 29, 2014

serhiy-storchaka commented Mar 22, 2018

put opcode information in one place #62061

put opcode information in one place #62061

Comments

benjaminp commented Apr 28, 2013

benjaminp commented Apr 28, 2013

kushaldas commented Apr 28, 2013

kushaldas commented Apr 28, 2013

kushaldas commented Apr 28, 2013

kushaldas commented Apr 29, 2013

kushaldas commented Apr 29, 2013

merwok commented Apr 11, 2014

loewis mannequin commented Apr 13, 2014

kushaldas commented Apr 14, 2014

kushaldas commented Apr 14, 2014

kushaldas commented Apr 15, 2014

kushaldas commented Apr 15, 2014

python-dev mannequin commented Apr 15, 2014

Yhg1s commented Apr 16, 2014

Yhg1s commented Apr 16, 2014

python-dev mannequin commented Apr 16, 2014

db3l commented Apr 28, 2014

python-dev mannequin commented Apr 28, 2014

ned-deily commented Apr 28, 2014

loewis mannequin commented Apr 29, 2014

ned-deily commented Apr 29, 2014

serhiy-storchaka commented Mar 22, 2018