Why the entire line? Would the location of the type would be better?

Is this legal syntax?

except*\ValueError:

Would the location of the type would be better?

The type has the location of the expression that defines it. This is for exceptions raised from theexcept* itself. If you look at the test - the exception is fromsplit() on the exception group, which is called byexcept*.

Why the entire line?

We don't have accurate information in the AST about the location of theexcept*, so this is next best. If it is multiline, the first line is where theexcept* is, which is what we want.

This is for theCHECK_EG_MATCH instruction, IIUC. Since it is matching the type, wouldn't the location of the type be a reasonable location?

If the AST had all the location information for everything, what location would you give theCHECK_EG_MATCH instruction?

I thinkCHECK_EG_MATCH should have the location ofexcept*, which is applying the match check. The location of calculating the type should be the location of the type expression.

We don't generally attach locations to syntax likeexcept *.
For example, the location of the calls to__enter__ and__exit__ in awith statement is the context expression, not thewith keyword.

Maybe we should attach the locations to the keywords? Though, as you say, it will need changes to the AST.

I made this issue to add locations to the AST#129157.

But we need to do something here that we can backport.

This is for theCHECK_EG_MATCH instruction, IIUC. Since it is matching the type, wouldn't the location of the type be a reasonable location?

If we go with the type, then we have the analogous situation withCHECK_EXC_MATCH for old-styleexcept, where there may not be a type. We probably can't get an exception raised there, but do we mind if the location info is different in the code object is different forexcept andexcept*?

This is also valid (PEP 8 notwithstanding :-):

>>> try:...     foo... except\... *\... ValueError\... :...     pass...

I agree with Irit that the location we want to show in the traceback is theexcept keyword, whichin practice will be on the same line as the* and most likely also on the type.

(Sorry for the delay Irit. Today I am attempting to catch up with a lot of email I've been procrastinating on.)

Following the discussion with@pablogsal on#129162 I think there are two different things here: (1) the location we put for the instruction in the code object and (2) what we display in the traceback.

For the traceback, I now think we should ideally hilight the wholeexcept orexcept* statement, but place carets under the specific type (if there is one and it's relevant). If there is more than one type, we want to point to the one that cause the exception. This is similar to the case of awith statement with multiple context managers - we want to point to thewith keyword, but also indicate which of the context managers was involved.

I think Pablo's PR should just add the missing location info to the AST, and then we can take it from there.

I did find an example for plainexcept where putting the traceback on the expression might make more sense -- the CHECK_EXC_MATCH instruction has only one possible failure, which is when the expression returns something that's not an exception, and then we get

TypeError: catching classes that do not inherit from BaseException is not allowed

I'd like to see that attached to the expression that gave a non-exception rather than onexcept (if they are on different lines).

Forexcept* it is more complicated (many failure points, not all related to the given type) and I think attaching to theexcept is more acceptable there.

This blank line is weird. I'd expect either an arrow and/or squiggles pointing atexcept or at the first character of the line or at the whole line, OR no arrows/squiggles and no blank line.

The old way showed the entire except block, but no extra blank line.

I guess the extra blank line I noticed in the test output is because we use zeros for the column positions?

So maybe this needs to be more careful and figure out the position of the last character on the line,or just put it only on theexcept keyword? (Which would require figuring out the position of theexcept keyword, which we might not have.

I guess if it's hard to do that right, I'm okay with the blank line.

I did find an example for plainexcept where putting the traceback on the expression might make more sense -- the CHECK_EXC_MATCH instruction has only one possible failure, which is when the expression returns something that's not an exception, and then we get

TypeError: catching classes that do not inherit from BaseException is not allowed

I'd like to see that attached to the expression that gave a non-exception rather than onexcept (if they are on different lines).

Forexcept* it is more complicated (many failure points, not all related to the given type) and I think attaching to theexcept is more acceptable there.

My suggestion here:

This highlights theexcept keyword.

For plainexcept you could do the same, or dig the coordinates out of the type expression (a bit harder because there are a lot of possibilities there).