New submission from Sven Brauch :
Hi,
I'm writing a python language support plugin for an IDE. I'm using the AST
module to get information about the loaded source code, which works pretty
well. However, in some cases, the information provided by the AST is simply not
sufficient to
Sven Brauch added the comment:
Hi,
well, but you have to agree that there is no point in setting a correct column
offset for bar in
foo[bar] (it's correctly set to 4 here!)
and
foo(bar)
but not for
foo.bar (there's no information provided here)
For me, this looks like it was jus
Sven Brauch added the comment:
Hi Terry,
well, the current behaviour is... logical in some way, as it says "the whole
expression which accesses an attribute starts at column 0", i.e. it's easy to
understand why it's done like this. It just turns out that this is pretty
Sven Brauch added the comment:
Hi,
I found the reason for this behavior in the code now, it's in Python/ast.c,
lines 1745 and 1746 in ast_for_power():
tmp->lineno = e->lineno;
tmp->col_offset = e->col_offset;
Here, the range information for the individual
Sven Brauch added the comment:
Hi,
I agree that the current behavior is not wrong or such. It's just entirely
useless.
For all the other types of subscripts, such as a[b][c][d] or a(b)(c)(d), the
ranges of the actual subscript ASTs are also nulled, but there's an additional
Sven Brauch added the comment:
Well, weather it's supposed to or not, it *does* contain the line number
information:
For your example, the AST for "foo" tells you the offset for foo. If you want
to know the offset (well, "offset") for blah, why not look at foo? Curre
Sven Brauch added the comment:
Then you'd get the point where foo starts instead of the location of the
opening brace. Sounds good for me.
Also, I already said that, with the change I proposed, I do not see any case
where relevant information would be lost.
Your argument that it woul
Sven Brauch added the comment:
Hi,
yeah Terry, that's exactly what most people whom I talked about this said (me
too).
Anyway, here's the patch which -- in my opinion -- fixes this behavior:
--- python-orig/Python/ast.c 2010-10-19 03:22:07.0 +0200
+++ python-ast-fix/Py
Sven Brauch added the comment:
Okay, thank you, I'm going to do that. :)
Bye,
Sven
--
___
Python tracker
<http://bugs.python.org/issue10769>
___
___
Pytho
New submission from Sven Brauch:
Here's a patch doing some adjustments to the AST to make it more useful for
static language analysis, as discussed in
http://mail.python.org/pipermail/python-dev/2012-December/123320.html.
Changes done:
* the described fix to attribute ranges
* add loc
Sven Brauch added the comment:
While writing tests, I noticed that the additional fields (lineno, col_offset
for vararg, kwarg, and other arguments) currently are mandatory. Is that a
problem?
It doesn't seem trivial to change that, since apparently only attributes (not
fields) c
Sven Brauch added the comment:
Thanks. I had seen and tried this before, but the "ast" module in python, which
is used in the tests, still requires the additional arguments. Probably this is
only valid for the C API?
--
___
Python trac
Sven Brauch added the comment:
Here's a new proposal, I adjusted the AST tests and fixed some small problems I
encountered during that. It contains all the diffs for generated files, should
I remove those for easier review?
A remaining problem is that AST_Tests::_assertTrueorder now fai
Sven Brauch added the comment:
Here's another version now which I think could be used like this. All tests
have been adjusted. I'll append two patches, one just containing the changes to
the parser for ease of review, and one full diff which also contains changes to
the generated
Sven Brauch added the comment:
Attached is the full diff this time.
--
Added file: http://bugs.python.org/file28680/full.diff
___
Python tracker
<http://bugs.python.org/issue16
Sven Brauch added the comment:
The patch review tool currently throws errors on submitting any form
(http://pastie.org/pastes/5665048/text) so please forgive me for answering here
once more. I'll copy this information (patch + message) to the review as soon
as the website is working
Sven Brauch added the comment:
The above post has an example for trying to add a patch, here's what happens
when I try to post a reply: http://pastie.org/pastes/5665144/text
I also tried with another web browser, so it's unlikely that it's the browser's
fault (
Sven Brauch added the comment:
Ah, whops, I misunderstood that.
The error is rather generic:
Traceback (most recent call last):
File "./Lib/test/test_ast.py", line 796, in test_lambda
self._check_arguments(fac, self.expr)
File "./Lib/test/test_ast.py", line 596,
Sven Brauch added the comment:
Not an issue, having this thing resolved upstream would save a huge lot of pain
elsewhere. ;)
So, to make sure... I'll go to the asdl file, make arguments have two arg
attributes which store the data for the var and kwarg which they can contain,
then I a
Sven Brauch added the comment:
I think I got it mostly working now (it was quite simple in fact), but there's
one issue which I can't seem to solve. This fails:
>>> compile(ast.parse("def fun(): pass"), "", "exec")
Traceback (most recent call
Sven Brauch added the comment:
Here's the next version which I hope to be somewhat complete now.
vararg and kwarg are now of type arg, and I did all the changes which are
required to make this possible. The ast tests pass.
Do you prefer to have this as one large patch all together, or
Sven Brauch added the comment:
Alright, I'll be back with those shortly (as soon as I found out how to do this
best with hg -- I'm used to git ;). I'll also sign the contributor agreement,
that's no problem of course.
--
___
P
Sven Brauch added the comment:
Okay, here they are. I'm not sure how to make hg include a commit message in
the patch...
81299-extend-asdl.diff: Changes required to the ASDL framework, in order to
allow attributes ( ... ) on a product
81300-change-var-kwargs.diff: Makes var/kwarg be inst
Sven Brauch added the comment:
second patch file
--
Added file: http://bugs.python.org/file28788/81300-change-var-kwargs.diff
___
Python tracker
<http://bugs.python.org/issue16
Sven Brauch added the comment:
third patch file
(... is there a better way to upload three files?)
--
Added file: http://bugs.python.org/file28789/81301-change-attr-ranges.diff
___
Python tracker
<http://bugs.python.org/issue16
Sven Brauch added the comment:
I have signed the contributor agreement and sent a scan to the specified mail
address (received no reply so far, but I guess that's okay).
Did anyone happen to find the time to look at the patches yet?
Greetings,
Sven Brauch added the comment:
Hm, I'm still getting the same error messages from the review tool which I
described earlier; I can neither comment nor add patches.
So, I'll have to abuse the bug report again:
Thanks for the review. Is it possible you selected the wrong patch fi
Sven Brauch added the comment:
Any news on this yet? ;)
Unfortunately, I'm still having no luck in adding the patch to the review tool
(same error message).
--
___
Python tracker
<http://bugs.python.org/is
Sven Brauch added the comment:
Attaching files to this bug report here works fine (see corrected patch above),
but when I add the file to http://bugs.python.org/review/16795/ under "Add
another patchset", I get the error message I described. I tried with firefox,
chromium and
Sven Brauch added the comment:
Oh, alright, that explains things. In this case, the file I attached on Jan 29
(http://bugs.python.org/file28905/81300-change-var-kwargs-new.diff) should
contain all the requested changes.
Greetings
--
___
Python
Sven Brauch added the comment:
I don't want to push anything, but did you find time to review this yet? It
would be great to have it in the next release.
--
___
Python tracker
<http://bugs.python.org/is
Sven Brauch added the comment:
Hi Benjamin,
the delay is not a problem -- as long as this is in time for Python 3.4,
everything is fine.
Attached is a patch which adjusts the unparser to the changes. Acoording to the
tests, this is all that needs to be updated.
Cheers,
Sven
Sven Brauch added the comment:
Thanks for reviewing this, and thanks for guiding me through the implementation
process. ;)
--
___
Python tracker
<http://bugs.python.org/issue16
Sven Brauch added the comment:
Hi,
Mailing list thread:
https://mail.python.org/pipermail/python-dev/2012-December/123320.html
Discussion on the patch: http://bugs.python.org/issue16795
Greetings,
Sven
--
___
Python tracker
<h
Sven Brauch added the comment:
Yes, this issue can be closed.
--
resolution: -> fixed
status: open -> closed
___
Python tracker
<http://bugs.python.org/i
Sven Brauch added the comment:
Why did you not CC me in this discussion? It is not very nice to have this
behaviour changed back from what I relied upon in a minor version without
notice.
Which regression was effectively caused by this patch, except for the
documentation being out of date
Sven Brauch added the comment:
Hmm, strange, I did not receive any emails.
"Incorrect" by what definition of incorrect? The word does not really help to
clarify the issue you see with this change, since the behaviour was changed on
purpose. What is the (preferably real-world) a
Sven Brauch added the comment:
But if you need the start of the full expression, can't you just go up in the
"parent" chain until the parent is not an expression any more?
Could additional API be introduced which provides the value I am looking for as
well as the one you need
38 matches
Mail list logo
