Skip to content

gh-153785: Generate AttributeError messages from context#153786

Open
johnslavik wants to merge 37 commits into
python:mainfrom
johnslavik:gh-153785
Open

gh-153785: Generate AttributeError messages from context#153786
johnslavik wants to merge 37 commits into
python:mainfrom
johnslavik:gh-153785

Conversation

@johnslavik

@johnslavik johnslavik commented Jul 16, 2026

Copy link
Copy Markdown
Member

I think this is the best place to do this. We do the same trick in KeyError.

This also fixed issue #143811 that motivated the current one.

@python-cla-bot

This comment was marked as resolved.

@johnslavik johnslavik changed the title Generate AttributeError messages from context gh-153785: Generate AttributeError messages from context Jul 16, 2026
@read-the-docs-community

read-the-docs-community Bot commented Jul 16, 2026

Copy link
Copy Markdown

Documentation build overview

📚 cpython-previews | 🛠️ Build #33636506 | 📁 Comparing faf6e53 against main (b4662e8)

  🔍 Preview build  

5 files changed · ± 5 modified

± Modified

@johnslavik
johnslavik requested a review from DinoV July 16, 2026 01:23
@johnslavik
johnslavik marked this pull request as draft July 16, 2026 02:11
@johnslavik
johnslavik marked this pull request as ready for review July 16, 2026 02:21
@johnslavik
johnslavik marked this pull request as ready for review July 16, 2026 03:51

@DinoV DinoV left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

Comment thread Objects/exceptions.c Outdated
Py_END_CRITICAL_SECTION();

if (!obj) {
return BaseException_str(op);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be worth a comment here that BaseException_str also locks and that's why this is outside of the lock.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to get Claude to look for whether similar situations have a comment convention, but couldn't find anything. I added a small "re-acquires lock" comment. Does it read useful?

@johnslavik johnslavik Jul 17, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @kumaraditya303 as the person who made BaseException_str thread-safe

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that looks good, it's enough of a breadcrumb for someone looking at it to not have to goto the other function.

Comment thread Lib/test/test_exceptions.py Outdated
Comment thread Objects/exceptions.c Outdated
Comment thread Objects/exceptions.c
Comment thread Lib/test/test_exceptions.py Outdated
Comment thread Objects/exceptions.c Outdated
Comment thread Doc/library/exceptions.rst Outdated
Comment thread Objects/exceptions.c Outdated
Comment thread Objects/exceptions.c Outdated
@johnslavik
johnslavik requested a review from pablogsal July 17, 2026 10:15
Comment thread Objects/exceptions.c
result = PyUnicode_FromFormat("module has no attribute '%U'", name);
}
} else {
result = PyUnicode_FromFormat("'%.200s' object has no attribute '%U'",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use %T.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really want the whole qualname? Other AttributeError messages use %.200s or %.100s on tp_name.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please refrain from pushing so often intermediate commits. Only push when you are ready, preferably consolidated commit.

@vstinner, is there a format for short name?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't find any myself looking at unicode_fromformat_arg.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants