-
-
Notifications
You must be signed in to change notification settings - Fork 31.1k
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
Add docstring
field to AST nodes
#73649
Comments
spin off of bpo-11549. http://bugs.python.org/issue11549#msg130955
|
I like the change because (IMHO) it makes the code simpler, and becase it also changes the first line of code object. I reviewed the patch: need basic unit tests. |
I like this change. Added comments on Rietveld. Are changes in importlib.h only due to changing first line numbers? |
lnotab is changed too. - 0,0,0,115,12,0,0,0,8,4,4,2,8,8,8,12,
- 8,25,8,13,114,18,0,0,0,99,0,0,0,0,0,0,
+ 0,0,0,115,10,0,0,0,12,6,8,8,8,12,8,25,
+ 8,13,114,18,0,0,0,99,0,0,0,0,0,0,0,0, 115 is header for bytes type. |
docstring
attribute to AST nodes
def func(): "doc" + "string" Currently (Python 2.7-3.6), func.__doc__ is None. I suggest to add an unit test for this corner case, even if the result is going to change in a near future. We need to "specify" the expected behaviour, and make sure that we get the same result if optimizations are enabled or not. |
Support adding tests. Tests should cover all cases: module, class, function, coroutine and check also the first line number. What is the value of co_firstlineno if the function doesn't have any statements? def f():
'''docstring''' |
Oh, I misunderstood something. inada-n@x250 ~/w/p/ast-docstring> cat -n x.py |
2017-02-08 10:08 GMT+01:00 INADA Naoki <report@bugs.python.org>:
1+1 is replaced with 2 and lone integer literals are removed by the |
Oops, I spoke too fast :-) "1+1" is not removed. "1+1" is replaced with "2" by the peephole optimizer, whereas the compiler ignoring constants comes before the peephole optimizer. One more time, it would be better to implement constant folding at the AST level ;-) $ python3
Python 3.5.2 (default, Sep 14 2016, 11:28:32)
>>> def func():
... "docstring"
... 1+1
...
>>> import dis
>>> dis.dis(func)
3 0 LOAD_CONST 3 (2)
3 POP_TOP
4 LOAD_CONST 2 (None)
7 RETURN_VALUE |
This patch affects firstlineno and lnotab of module and class, but not functions. module:
-<code object <module> at 0x7f053a8f8b70, file "Lib/importlib/_bootstrap.py", line 8>
+<code object <module> at 0x7fdefbf10340, file "Lib/importlib/_bootstrap.py", line 25>
filename Lib/importlib/_bootstrap.py
-firstlineno: 8
+firstlineno: 25
-lnotab: b'\x04\x11\x04\x02\x08\x08\x08\x07\x04\x02\x04\x03\x10\x04\x0eD\x0e\x15\x0e\x13\x08\x13\x08\x13\x08\x0b\x0e\x08\x08\x0b\x08\x0c\x08\x10\x08$\x0e\x1b\x0ee\x10\x1a\x06\x03\n-\x0e<\x08\x11\x08\x11\x08\x19\x08\x1d\x08\x17\x08\x10\x0eI\x0eM\x0e\r\x08\t\x08\t\n/\x08\x14\x04\x01\x08\x02\x08\x1b\x08\x06\n\x19\x08\x1f\x08\x1b\x12#\x08\x07\x08/'
- 8 0 LOAD_CONST 0 ('Core implementation of import.\n\nThis module is NOT meant to be directly imported! It has been designed such\nthat it can be bootstrapped into Python as the implementation of import. As\nsuch it requires the injection of specific modules and attributes in order to\nwork. One should use importlib as the public-facing version of this module.\n\n')
+lnotab: b'\x04\x00\x04\x02\x08\x08\x08\x07\x04\x02\x04\x03\x10\x04\x0eD\x0e\x15\x0e\x13\x08\x13\x08\x13\x08\x0b\x0e\x08\x08\x0b\x08\x0c\x08\x10\x08$\x0e\x1b\x0ee\x10\x1a\x06\x03\n-\x0e<\x08\x11\x08\x11\x08\x19\x08\x1d\x08\x17\x08\x10\x0eI\x0eM\x0e\r\x08\t\x08\t\n/\x08\x14\x04\x01\x08\x02\x08\x1b\x08\x06\n\x19\x08\x1f\x08\x1b\x12#\x08\x07\x08/'
+ 25 0 LOAD_CONST 0 ('Core implementation of import.\n\nThis module is NOT meant to be directly imported! It has been designed such\nthat it can be bootstrapped into Python as the implementation of import. As\nsuch it requires the injection of specific modules and attributes in order to\nwork. One should use importlib as the public-facing version of this module.\n\n')
2 STORE_NAME 0 (__doc__)
-
- 25 4 LOAD_CONST 1 (None)
+ 4 LOAD_CONST 1 (None)
6 STORE_GLOBAL 1 (_bootstrap_external)
class:
-<code object _ModuleLock at 0x7f053ab61db0, file "Lib/importlib/_bootstrap.py", line 51>
+<code object _ModuleLock at 0x7fdefc61c580, file "Lib/importlib/_bootstrap.py", line 51>
filename Lib/importlib/_bootstrap.py
firstlineno: 51
-lnotab: b'\x08\x04\x04\x02\x08\x08\x08\x0c\x08\x19\x08\r'
+lnotab: b'\x0c\x06\x08\x08\x08\x0c\x08\x19\x08\r'
51 0 LOAD_NAME 0 (__name__)
2 STORE_NAME 1 (__module__)
4 LOAD_CONST 0 ('_ModuleLock')
6 STORE_NAME 2 (__qualname__)
-
- 55 8 LOAD_CONST 1 ('A recursive lock implementation which is able to detect deadlocks\n (e.g. thread 1 trying to take locks A then B, and thread 2 trying to\n take locks B then A).\n ')
+ 8 LOAD_CONST 1 ('A recursive lock implementation which is able to detect deadlocks\n (e.g. thread 1 trying to take locks A then B, and thread 2 trying to\n take locks B then A).\n ')
10 STORE_NAME 3 (__doc__) |
So what's new entry may be: +* ``Module``, ``FunctionDef``, ``AsyncFunctionDef``, and |
Now I doubt about this patch is really good. docstring of Module and Class generates two bytecode. LOAD_CONST 0 ("docstring") |
Therefore we loss the possibility to set a breakpoint on the docstring? It doesn't look a great lost. |
Just terminology ... strictly speaking what you've done here is "add a *field* to the nodes Module, FunctionDef and ClassDef", rather than add an *attribute* -- that is, when one is consistent with the terms used in the ast module (https://docs.python.org/3/library/ast.html#node-classes) or Wang (https://docs.python.org/devguide/compiler.html#wang97). |
Thanks. I don't familiar with language frontend. |
docstring
attribute to AST nodesdocstring
field to AST nodes
This issue broke the ast API. Copy of the following comment on the PR: Would it have been possible to make the docstring optional ? (It's already breaking things, like IPython). Should I comment on upstream bpo ? I created the issue bpo-29622 to fix this. |
thank you for your work on the AST, I know many developers are looking forward to improvement and stabilisation with the hope of having it stable (and documented) in the stdlib at some point. The recent change in PR 46 now change (at least) the constructor of I know the ast is autogenerated and "use at your own risk". But IPython for example, use I would be happy if it raise a deprecation warning that it will be required in the future. I'm of course speaking about Thanks. |
OK, let's continue on bpo-29522, and close this issue. |
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:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: