Fix best_base to select proper base class#5324
Conversation
| test(123456, "*=20", '**************123456') | ||
|
|
||
| @unittest.expectedFailure | ||
| @run_with_locale('LC_NUMERIC', 'en_US.UTF8') |
There was a problem hiding this comment.
This line is not expected to be removed.
vm/src/builtins/int.rs
Outdated
| flags(BASETYPE), | ||
| with(PyRef, Comparable, Hashable, Constructor, AsNumber, Representable) | ||
| with(PyRef, Comparable, Hashable, Constructor, AsNumber, Representable), | ||
| itemsize(BigInt) |
There was a problem hiding this comment.
I don't get how this is intended to be used yet. How to handle it if it requires multiple fields?
There was a problem hiding this comment.
It was not intended for multiple types to be declared with itemsize.
In the pyclass code, if the number of types declared with itemsize is not one, it is planned to raise an error in a way that developers can understand. Unlike basicsize, it seems difficult to automatically infer item type for each class delcaration, so I have chosen this way.
There was a problem hiding this comment.
Hmm.. I think itemsize implementation is better to be moved to other PRs. It seems it does not cause wrong behavior in best_base.
0964777 to
d3e7b31
Compare
|
I can't reproduce |
| if sys.platform != "darwin": | ||
| test_int__format__locale = unittest.expectedFailure(test_int__format__locale) |
There was a problem hiding this comment.
Reverting this will fix the failing test
There was a problem hiding this comment.
Oh my.. That code accidentally got included in the commit. I've fixed it!
Signed-off-by: snowapril <sinjihng@gmail.com>
| debug_assert!(base.is_some()); | ||
| Ok(base.unwrap()) |
There was a problem hiding this comment.
unwrap implies panic!. So debug_assert! is duplication.
| debug_assert!(base.is_some()); | |
| Ok(base.unwrap()) | |
| Ok(base.expect("best_base must exist")) |
This revision implements
itemsizeslots and fixbest_baseimplementation to select proper base class when generating class.