[cython/cython] Fix crash when function contains fused extension type (PR #6204)
Matus Valo
You can view, comment on, or merge this pull request online at:
https://github.com/cython/cython/pull/6204
Commit Summary
- 3ea182e Fix crash when function contains fused extension type
File Changes
(2 files)
Patch Links:
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
da-woods
@da-woods commented on this pull request.
In Cython/Compiler/PyrexTypes.py:
> @@ -5042,7 +5042,7 @@ def best_match(arg_types, functions, pos=None, env=None, args=None):
if len(candidates) == 1:
return candidates[0][0]
elif len(candidates) == 0:
- if pos is not None:
+ if pos is not None and errors:
You've possibly looked into this more than me, but I wonder if this should end up showing a "no suitable method found" error?
(I'm also not quite sure what the point of [1 for func, e in errors if e == errmsg] below... that looks like it could be simplified).
Otherwise PR looks good
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Matus Valo
@matusvalo commented on this pull request.
In Cython/Compiler/PyrexTypes.py:
> @@ -5042,7 +5042,7 @@ def best_match(arg_types, functions, pos=None, env=None, args=None):
if len(candidates) == 1:
return candidates[0][0]
elif len(candidates) == 0:
- if pos is not None:
+ if pos is not None and errors:
I wonder if this should end up showing a "no suitable method found" error?
No idea. Broken parts of the cythonized code already contains errors - see test part of the PR. Honestly, I tried to grep the code and I was not able to find any test with this error message:
matus@dev:~/dev/cython/tests$ grep -RI 'no suitable method found' *
matus@dev:~/dev/cython/tests$ grep -RI 'suitable method found' *
matus@dev:~/dev/cython/tests$ grep -RI 'suitable method' *
matus@dev:~/dev/cython/tests$ grep -RI 'suitable' *
For me adding additional error is not adding any value to the case I found compiler crashing.
(I'm also not quite sure what the point of
When I reviewed the code, I agree. It does not make sense. the part [1 for func, e in errors if e == errmsg] because at least first item in errors list will match yielding [1] list in worst case. Maybe I am missing something though. It is difficult to say because I didn't found any test covering it...
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
da-woods
@da-woods commented on this pull request.
In Cython/Compiler/PyrexTypes.py:
> @@ -5042,7 +5042,7 @@ def best_match(arg_types, functions, pos=None, env=None, args=None):
if len(candidates) == 1:
return candidates[0][0]
elif len(candidates) == 0:
- if pos is not None:
+ if pos is not None and errors:
Broken parts of the cythonized code already contains errors [...] For me adding additional error is not adding any value to the case I found compiler crashing.
Yes - agree with this.
I think this change is fine then. Avoiding the crash is definitely an improvement
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Matus Valo
Merged #6204 into master.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.