GeezerVison

🤖 New build scheduled with the buildbot fleet by @brandtbucher for commit 33dc4d8 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

wasm32 buildbot failures are unrelated: #94549 (comment)

Is a bit unfortunate that we cannot just propagate correctly the offsets from the AST nodes. The disadvantage of a method relying on inspecting the source (here through the method name) is that ASTs created from abstract sources (or no source at all) may fall through the cracks in these kind of checks.

What's preventing us from correctly grab the original column offsets from the multi line method call? (I know this has more to be with the original issue, but I would like to fully understand this before adding more band aids :) )

The issue here is that we are flattening a multi line location into a single line. So we are trying to construct the location info for the attribute name only (which is not available directly in the AST, it seems).

The issue here is that we are flattening a multi line location into a single line. So we are trying to construct the location info for the attribute name only (which is not available directly in the AST, it seems).

For our own nodes, I can fix that in the parser so our nodes make those available. For random AST nodes it may still be a problem.

The issue here is that we are flattening a multi line location into a single line. So we are trying to construct the location info for the attribute name only (which is not available directly in the AST, it seems).

For our own nodes, I can fix that in the parser so our nodes make those available. For random AST nodes it may still be a problem.

I think we almost have it. The location of meth->v.Attribute seems to be the dot plus the attr name. Is that right? Can you exclude the dot?

For random AST nodes it may still be a problem.

The pytest nodes are not completely random, if we copy to them the location of the attr name from the source code the traceback will probably look right - the source line from the file will appear with the right thing hilighted.

What's preventing us from correctly grab the original column offsets from the multi line method call?

The root of the issue is that attr is stored as an identifier (string) in the AST. If it was a full Name node, this whole hack could just be replaced with a single SET_LOC.

I think we almost have it. The location of meth->v.Attribute seems to be the dot plus the attr name. Is that right? Can you exclude the dot?

Unfortunately, Attribute nodes include the entire span of the attribute access, including everything to the left of the dot.

This also creates a weird (but probably fine) inconsistency. If the attribute access is all on one line, the whole thing is underlined in the traceback. If it spans multiple lines, only the attribute name is underlined.

All this just comes from our need to trace the line that the attribute name is on, and trying (after the fact) to figure out what columns that should include. :(

The root of the issue is that attr is stored as an identifier (string) in the AST. If it was a full Name node, this whole hack could just be replaced with a single SET_LOC.

This is exactly what I'm referring to. We could modify the AST to add this information so we don't need to guess. This way, it should also have work with arbitrary AST nodes.

The price is that although this is something we can do, it will force everyone to adapt.

Yeah. I think it would be an acceptable change for 3.12. While some consumers of the AST would need to change their code slightly, it wouldn't be removing any functionality that's currently present. If anything, it's adding functionality.

Thanks @brandtbucher for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

Sorry, @brandtbucher, I could not cleanly backport this to 3.11 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 264b3ddfd561d97204ffb30be6a7d1fb0555e560 3.11

I think this one's gonna need a manual backport.

GH-94721 is a backport of this pull request to the 3.11 branch.