gh-95778: Use a note for the max digits error message by vstinner · Pull Request #96878 · python/cpython · GitHub
Skip to content

gh-95778: Use a note for the max digits error message - #96878

Closed
vstinner wants to merge 1 commit into
python:mainfrom
vstinner:maxdigits_note
Closed

vstinner wants to merge 1 commit into
python:mainfrom
vstinner:maxdigits_note

Conversation

@vstinner

@vstinner vstinner commented Sep 16, 2022

Copy link
Copy Markdown
Member

Add _PyErr_AddNote() function to the internal C API.

Add _PyErr_AddNote() function to the internal C API.
@vstinner

Copy link
Copy Markdown
Member Author

@vstinner

Copy link
Copy Markdown
Member Author

I'm looking at which existing code might benefit from _PyErr_Note(). "Suggestions" added by PyErr_Display() with _Py_Offer_Suggestions() (issue #82711) is not a good fit: this code doesn't modify the exception on purpose. Or do you want to add suggestions in notes? If yes, we should maybe make sure that the note is not added multiple times if PyErr_Display() is called multiple times on the same exception ;-)

@vstinner

Copy link
Copy Markdown
Member Author

Maybe _PyPegen_number_token() can use _PyErr_AddNote() for this code?

            // This acts as PyErr_Clear() as we're replacing curexc.
            PyErr_Fetch(&type, &value, &tb);
            Py_XDECREF(tb);
            Py_DECREF(type);
            /* Intentionally omitting columns to avoid a wall of 1000s of '^'s
             * on the error message. Nobody is going to overlook their huge
             * numeric literal once given the line. */
            RAISE_ERROR_KNOWN_LOCATION(
                p, PyExc_SyntaxError,
                t->lineno, -1 /* col_offset */,
                t->end_lineno, -1 /* end_col_offset */,
                "%S - Consider hexadecimal for huge integer literals "
                "to avoid decimal conversion limits.",
                value);
            Py_DECREF(value);


// Call add_note(note) on the current exception.
// Return -1 on error, or 0 on success.
// Save and restore the current exception.

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.

Might be good to mention in the comment that there must be a current exception. I think otherwise it crashes or some assertion fails, not sure.

@iritkatriel

Copy link
Copy Markdown
Member

The intention with exception notes was that you use it to add information that is not available at the time the exception is created and raised, but only after you caught it. This doesn't follow that pattern.

Why not just add one or two newlines to the msg?

@vstinner

Copy link
Copy Markdown
Member Author

The intention with exception notes was that you use it to add information that is not available at the time the exception is created and raised, but only after you caught it. This doesn't follow that pattern. Why not just add one or two newlines to the msg?

Oh, that's far from issue #95778 scope, so I created the new issue #96958 to discuss that instead. I propose to continue the discussion there.

@vstinner

Copy link
Copy Markdown
Member Author

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