Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Formalize incomplete error exceptions to improve codeop module handling #113744

Closed
pablogsal opened this issue Jan 5, 2024 · 10 comments
Closed
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement

Comments

@pablogsal
Copy link
Member

pablogsal commented Jan 5, 2024

Currently the way we detect incomplete errors in the codeop module is based on the exception text, which is suboptimal at least. To do this properly, add a new SyntaxError subclass called IncompleteInputError and use that in the codeop module to detect errors from the parser.

Linked PRs

pablogsal added a commit to pablogsal/cpython that referenced this issue Jan 5, 2024
…incomplete input detection in the codeop module
pablogsal added a commit to pablogsal/cpython that referenced this issue Jan 8, 2024
…incomplete input detection in the codeop module

Signed-off-by: Pablo Galindo <[email protected]>
pablogsal added a commit to pablogsal/cpython that referenced this issue Jan 29, 2024
…incomplete input detection in the codeop module

Signed-off-by: Pablo Galindo <[email protected]>
pablogsal added a commit to pablogsal/cpython that referenced this issue Jan 30, 2024
…incomplete input detection in the codeop module

Signed-off-by: Pablo Galindo <[email protected]>
pablogsal added a commit to pablogsal/cpython that referenced this issue Jan 30, 2024
…mprove incomplete input detection in the codeop module
pablogsal added a commit to pablogsal/cpython that referenced this issue Jan 30, 2024
…mprove incomplete input detection in the codeop module

Signed-off-by: Pablo Galindo <[email protected]>
pablogsal added a commit that referenced this issue Jan 30, 2024
…lete input detection in the codeop module (#113745)

Signed-off-by: Pablo Galindo <[email protected]>
@encukou
Copy link
Member

encukou commented Feb 5, 2024

Since PyExc_IncompleteInputError was added to the stable API, it needs to be documented. Usually the C names for exceptions just link to the Python docs, so the Python docs are needed as well.

@encukou
Copy link
Member

encukou commented Feb 5, 2024

(If you want to delegate this to me, let me know. Currently I'd lean toward not including it in builtins and C-API.)

@pablogsal
Copy link
Member Author

(If you want to delegate this to me, let me know. Currently I'd lean toward not including it in builtins and C-API.)

I think is fine to have it in builtins given that we have the other subclasses of SyntaxError, even if this is currently only used in codeop but if you feel strongly about this I am fine to have it removed.

I am fine removing it from the C-API 👍

WDYT?

@encukou
Copy link
Member

encukou commented Feb 6, 2024

Sounds good. It can always be added :)

@pablogsal
Copy link
Member Author

pablogsal commented Feb 6, 2024

@encukou can you refresh my memory on how can I make the API check not complain? Marking the exception as extern? Or there is a better way?

aisk pushed a commit to aisk/cpython that referenced this issue Feb 11, 2024
…incomplete input detection in the codeop module (python#113745)

Signed-off-by: Pablo Galindo <[email protected]>
@pablogsal
Copy link
Member Author

@encukou gentle ping

@encukou
Copy link
Member

encukou commented Feb 19, 2024

Sorry, I missed this notification.

Not sure which check you have in mind. If you have a PR where you just need to silence a check, could you send it?

I assume it's the Docs check? The exception either needs to be documented, or removed from limited API like this:

--- a/Include/pyerrors.h
+++ b/Include/pyerrors.h
@@ -108,7 +108,9 @@ PyAPI_DATA(PyObject *) PyExc_NotImplementedError;
 PyAPI_DATA(PyObject *) PyExc_SyntaxError;
 PyAPI_DATA(PyObject *) PyExc_IndentationError;
 PyAPI_DATA(PyObject *) PyExc_TabError;
+#if !defined(Py_LIMITED_API)
 PyAPI_DATA(PyObject *) PyExc_IncompleteInputError;
+#endif
 PyAPI_DATA(PyObject *) PyExc_ReferenceError;
 PyAPI_DATA(PyObject *) PyExc_SystemError;
 PyAPI_DATA(PyObject *) PyExc_SystemExit;
diff --git a/Misc/stable_abi.toml b/Misc/stable_abi.toml
index ca7cf029615..a7fb8e3c327 100644
--- a/Misc/stable_abi.toml
+++ b/Misc/stable_abi.toml
@@ -2485,8 +2485,6 @@
 [function._Py_SetRefcnt]
     added = '3.13'
     abi_only = true
-[data.PyExc_IncompleteInputError]
-    added = '3.13'
 [function.PyList_GetItemRef]
     added = '3.13'
 [typedef.PyCFunctionFast]
diff --git a/Include/pyerrors.h b/Include/pyerrors.h
index 68d7985dac8..9b7321e5ad5 100644

and run make regen-limited-abi, which will remove it from the Docs data (among other things).

Or to make it fully internal, move the declaration to internal/pycore_excetpions.h instead of using #if !defined(Py_LIMITED_API).

@pablogsal
Copy link
Member Author

@encukou I am getting this:


Checked 110 modules (33 built-in, 76 shared, 0 n/a on macosx-14.3-arm64, 0 disabled, 1 missing, 0 failed on import)
./python.exe ./Tools/build/stable_abi.py --all ./Misc/stable_abi.toml
Some extra declarations were found in "Include/Python.h" with Py_LIMITED_API:
 - PyExc_IncompleteInputError
Traceback (most recent call last):
  File "/Users/pgalindo3/github/python/main/./Tools/build/stable_abi.py", line 758, in <module>
    main()
    ~~~~^^
  File "/Users/pgalindo3/github/python/main/./Tools/build/stable_abi.py", line 734, in main
    raise Exception(f"""
    ...<19 lines>...
    """)
Exception:
        These checks related to the stable ABI did not succeed:
            unixy_check

        If you see diffs in the output, files derived from the stable
        ABI manifest the were not regenerated.
        Run `make regen-limited-abi` to fix this.

        Otherwise, see the error(s) above.

        The stable ABI manifest is at: Misc/stable_abi.toml
        Note that there is a process to follow when modifying it.

        You can read more about the limited API and its contracts at:

        https://docs.python.org/3/c-api/stable.html

        And in PEP 384:

        https://peps.python.org/pep-0384/

make: *** [check-limited-abi] Error 1

and this doesn't go away when running make regen-limited-abi. This is after removing the exception from Misc/stable_abi.toml

@encukou
Copy link
Member

encukou commented Feb 19, 2024

Use the #if !defined(Py_LIMITED_API) guard from the comment above -- the first part of the diff.

@JacobCoffee JacobCoffee added type-feature A feature request or enhancement interpreter-core (Objects, Python, Grammar, and Parser dirs) labels Sep 27, 2024
@JacobCoffee
Copy link
Member

Hi @pablogsal, is this good to close since #113745 is merged?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

3 participants