GeezerVison

For the traceback.py changes (and tests) I guess we should wait for @ammaraskar's changes on the traceback.py module.

@isidentical this may need rebasing now that the other PR is merged

Let's focus on land this one next @ammaraskar @isidentical

@ammaraskar after your _format_traceback (or even before) can you try to port this to the traceback.py? I think it is going to look something like this but haven't integrated printing logic / tests yet.

+def _extract_anchors_from_segment(segment):
+    import ast
+
+    tree = ast.parse(segment)
+    if len(tree.body) == 1:
+        statement = tree.body[0]
+        match statement:
+            case ast.Expr(expr):
+                match expr:
+                    case ast.BinOp():
+                        operator_str = segment[expr.left.end_col_offset:expr.right.col_offset]
+                        operator_offset = len(operator_str) - len(operator_str.lstrip())
+                        return operator_offset, operator_offset + 1
+                    case ast.Subscript():
+                        return expr.value.end_col_offset, expr.slice.col_offset
+    return -1, -1

🤖 New build scheduled with the buildbot fleet by @isidentical for commit 26430d4 🤖

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

Ping @pablogsal

Apparently ASAN is failing, though not sure if it is relevant.

@isidentical I have pushed 081738d to greatly simplify the code structure. I think it reads better now. Please, check it out

Thanks @pablogsal, it LGTM. Feel free to merge!

@isidentical Please, check out #27037 (comment)

Ah, I think that is not relevant. I just checked again, and it seems like source_line should be always non-NULL, though since we are dealing with value propagation over functions it might be still wiser to just replace the if (source_line) { ... } with assert(source_line); ....

Also I am not really sure if we get any cases where primary/secondary will be different, but if so I think it would be nicer to make amendments to the traceback.py logic to do the same.

Also I am not really sure if we get any cases where primary/secondary will be different, but if so I think it would be nicer to make amendments to the traceback.py logic to do the same.

In this PR or in a different one?

Ah, I think that is not relevant. I just checked again, and it seems like source_line should be always non-NULL, though since we are dealing with value propagation over functions it might be still wiser to just replace the if (source_line) { ... } with assert(source_line); ....

We should also add some tests where we fail to parse the source. Is this covered currently?

In this PR or in a different one?

This PR.

Also I am not really sure if we get any cases where primary/secondary will be different, but if so I think it would be nicer to make amendments to the traceback.py logic to do the same.

Also, I am not very sure what you refer to with "will be different". Can you elaborate?

We should also add some tests where we fail to parse the source. Is this covered currently?

I don't see this covered. Could you add a test for this?

We now have a test for when parsing fails and the source_line conditional is gone. Anything else to do here?

We now have a test for when parsing fails and the source_line conditional is gone. Anything else to do here?

Yes, the most important step: celebrate 🎉

(also pray for the buildbots to not fail 😉 )