Further limited API fixes #5798
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
da-woods
commented
Nov 5, 2023
da-woods
commented
- A bunch of tests enabled (mostly already working)
- (some) fstrings work.
- cython.array works
- assorted other small fixes
1. A bunch of tests enabled (mostly already working) 2. (some) fstrings work. 3. cython.array works 4. assorted other small fixes
da-woods
commented
Nov 5, 2023
| @@ -409,12 +409,12 @@ def make_length_call(): | |||
| PyBytes_AS_STRING_func_type = PyrexTypes.CFuncType( | |||
| PyrexTypes.c_char_ptr_type, [ | |||
| PyrexTypes.CFuncTypeArg("s", Builtin.bytes_type, None) | |||
| ]) | |||
| ], exception_value="NULL") | |||
There was a problem hiding this comment.
I'd quite like some way to enable the exception check only when a macro is defined (to avoid generating the code on non-limited api cases). Probably an improvement for the future rather than this PR though.
and a few other small tweaks
scoder
reviewed
Nov 16, 2023
| #else | ||
| base = PyTuple_GetItem(bases, i); | ||
| if (!base) goto error; | ||
| #endif |
There was a problem hiding this comment.
We should incref base if we "avoid borrowed refs". We do too many operations below to keep it as a borrowed reference for all that time.
Comment on lines
+67
to
+81
| #if CYTHON_ASSUME_SAFE_MACROS | ||
| #define __Pyx_PyBytes_AsWritableString(s) ((char*) PyBytes_AS_STRING(s)) | ||
| #define __Pyx_PyBytes_AsWritableSString(s) ((signed char*) PyBytes_AS_STRING(s)) | ||
| #define __Pyx_PyBytes_AsWritableUString(s) ((unsigned char*) PyBytes_AS_STRING(s)) | ||
| #define __Pyx_PyBytes_AsString(s) ((const char*) PyBytes_AS_STRING(s)) | ||
| #define __Pyx_PyBytes_AsSString(s) ((const signed char*) PyBytes_AS_STRING(s)) | ||
| #define __Pyx_PyBytes_AsUString(s) ((const unsigned char*) PyBytes_AS_STRING(s)) | ||
| #else | ||
| #define __Pyx_PyBytes_AsWritableString(s) ((char*) PyBytes_AsString(s)) | ||
| #define __Pyx_PyBytes_AsWritableSString(s) ((signed char*) PyBytes_AsString(s)) | ||
| #define __Pyx_PyBytes_AsWritableUString(s) ((unsigned char*) PyBytes_AsString(s)) | ||
| #define __Pyx_PyBytes_AsString(s) ((const char*) PyBytes_AsString(s)) | ||
| #define __Pyx_PyBytes_AsSString(s) ((const signed char*) PyBytes_AsString(s)) | ||
| #define __Pyx_PyBytes_AsUString(s) ((const unsigned char*) PyBytes_AsString(s)) | ||
| #endif |
There was a problem hiding this comment.
I do hope that we actually do error checking on these. We didn't need it before, so it seems worth taking a look.
There was a problem hiding this comment.
The two I've used in Optimize.py have error checking (because I added it). For their use in PyrexTypes.py, they have error checking because self.exception_value = NULL (
cython/Cython/Compiler/PyrexTypes.py
Line 2683 in bcc22ca
Co-authored-by: scoder <stefan_ml@behnel.de>
and enable a couple more tests
da-woods
added a commit
to da-woods/cython
that referenced
this pull request
Nov 19, 2023
It isn't actually usable because it cimports other modules but it does build without warning. Changes are mainly to string handling (because that's the nature of parsing). Depends on cython#5841 and cython#5798 to actually build, but the PR itself doesn't depend on those
da-woods
added a commit
to da-woods/cython
that referenced
this pull request
Nov 19, 2023
It isn't actually usable because it cimports other modules but it does build without warning. Changes are mainly to string handling (because that's the nature of parsing). Depends on cython#5841 and cython#5798 to actually build, but the PR itself doesn't depend on those
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.