Report overflow error #4959
Report overflow error #4959
Conversation
There was a problem hiding this comment.
By looking the test result, that function looks to have both types of errors.
Could you distinguish those errors?
year=10000 seems to expect ValueError
test test_time crashed -- Traceback (most recent call last):
File "/home/runner/work/RustPython/RustPython/pylib/Lib/test/libregrtest/runtest.py", line 256, in _runtest_inner
refleak = _runtest_inner2(ns, test_name)
File "/home/runner/work/RustPython/RustPython/pylib/Lib/test/libregrtest/runtest.py", line 207, in _runtest_inner2
the_module = importlib.import_module(abstest)
File "/home/runner/work/RustPython/RustPython/pylib/Lib/importlib/__init__.py", line 126, in import_module
return _bootstrap._gcd_import(name[level:], package, level)
File "_frozen_importlib", line 1207, in _gcd_import
File "_frozen_importlib", line 1176, in _find_and_load
File "_frozen_importlib", line 1179, in _find_and_load
File "_frozen_importlib", line 1153, in _find_and_load_unlocked
File "_frozen_importlib", line 1150, in _find_and_load_unlocked
File "_frozen_importlib", line 1150, in _find_and_load_unlocked
File "_frozen_importlib", line 706, in _load_unlocked
File "_frozen_importlib", line 704, in _load_unlocked
File "_frozen_importlib", line 697, in _load_unlocked
File "_frozen_importlib", line 691, in _load_unlocked
File "_frozen_importlib_external", line 940, in exec_module
File "_frozen_importlib", line 241, in _call_with_frames_removed
File "/home/runner/work/RustPython/RustPython/pylib/Lib/test/test_time.py", line 674, in <module>
del skip_if_not_supported
File "/home/runner/work/RustPython/RustPython/pylib/Lib/test/test_time.py", line 666, in _TestStrftimeYear
@skip_if_not_supported(10000)
File "/home/runner/work/RustPython/RustPython/pylib/Lib/test/test_time.py", line 661, in skip_if_not_supported
cond = False
File "/home/runner/work/RustPython/RustPython/pylib/Lib/test/test_time.py", line 659, in skip_if_not_supported
time.strftime('%Y', (y,) + (0,) * 8)
OverflowError: mktime argument out of range
|
I sure can. |
…flow for hour, min and sec - test now passes
|
I pushed new changes, is that what you had in mind? The test that failed now is green. Let me know if you meant something different and I can fix it. |
There was a problem hiding this comment.
I am sorry to be late. I was occupied by other major changes recently
By looking timemodule.c, mktime always return OverflowError and ValueError seems to came from somewhere else related to failing function.
So your original fix was more close to actual mktime fix, but it is good for now. I will create an issue for remaining issue.
Thank you for contributing!
|
No problem. |
|
I would love to contribute more, so ill look for other things I can do to help. |
| macro_rules! field { | ||
| ($field:ident) => { | ||
| self.$field.clone().try_into_value(vm)? | ||
| }; | ||
| } | ||
| let dt = NaiveDateTime::new( | ||
| NaiveDate::from_ymd_opt(field!(tm_year), field!(tm_mon), field!(tm_mday)) | ||
| .ok_or_else(invalid)?, | ||
| .ok_or_else(invalid_value)?, |
There was a problem hiding this comment.
I think this should raise an Overflow error too. The documentation for CPython states:
If the input value cannot be represented as a valid time, either OverflowError or ValueError will be raised (which depends on whether the invalid value is caught by Python or the underlying C libraries).
The value being caught by Python probably means the conversion from a CPython object into a CPython longobject. I.e, if you pass in a list, this will obviously fail with a ValueError because it will be expecting an integer.
The underlying C library is basically equivalent to the NaiveDate calls here. I'll try and look into it further on the weekend to see where the value error YunWon mentions happens.
There was a problem hiding this comment.
If you want, I can look into where the error is coming from. I would really like to get into contributing to this project, and it might be an excellent way to get to know the code base. If I can't figure it out, ill let you know.
There was a problem hiding this comment.
That sounds great to me! Do reply in the issue so we can keep as much of the context in one place.
#4938
It now creates an overflow error with the message described in the issue.
Output: