Implementing slice() and slices#345
Conversation
why do you need it? |
That might be a holdover from an earlier implementation detail; I'm not sure it's still relevant. |
Add parsing of slice nodes Make slices into calls to slice() in astframe; implement list.__getitem__(slice) Add slice (with hack) to make_fqn_const Fix mypy issue in fmt_expr_Slice Dump constant structs to c Implement slice indices and tests Implement slicing of lists, so long as step == 1
Merge from main More improvements to list and slices; some additional tests (some failing) Working slicing except for imports in _list in tests? Avoid circular imports in stdlib Suppress NoneType params and args in c backend Experiments with doppler
Merge branch 'main' into slice_impl Doppler working with a janky fqn hack that breaks c =O Much less janky implementation of eval_Slice in astframe Got opspec with fast_metacall... not right but closer Remove typo in shift_sice_expr Cleanup and clarify shift_expr_Slice
Simplify ASTFrame and Doppler for slice Remove unnecessary backend changes
JeffersGlass
force-pushed
the
slice_impl
branch
from
f71e5a6 to
574588e
Compare
Merge branch 'main' into slice_impl Cleanup types and tests Cleanup tests and old dead code
JeffersGlass
force-pushed
the
slice_impl
branch
from
574588e to
0ffd3e3
Compare
|
This I think is ready for review! One discussed feature was using a bitmask instead of individual |
antocuni
left a comment
antocuni
left a comment
There was a problem hiding this comment.
very good job @JeffersGlass 👏
I left some comments here and there; they might look a lot but it's just because this PR is pretty big, but overall it looks good.
Another note: this PR implements slice support only for __getitem__, correcT? It might be worth raising a WIP in the __setitem__ case (and leave the actual implementation for another PR)
| paramlist = [f"{p.c_type} {p.name}" for p in self.params] | ||
| paramlist = [ | ||
| f"{p.c_type} {p.name}" for p in self.params if p.c_type.name != "void" | ||
| ] |
There was a problem hiding this comment.
interesting. I suppose this is needed because "somewhere" you get a function with a void parameter?
Which function is that?
There was a problem hiding this comment.
This code suppresses any None parameters of functions - in particular opimpls returned by slice.__new__.
Consider the following spy code:
def main() -> None:
l = [1,2,3][:1]The metafunc that is slice.__new__ ends up selecting impl6 to us. For typechecking reasons, impl6 takes parameters of the form (None, int, None). But in the C code, we don't need (or want) to pass a void parameteter - we know we only need a single int to specify, in this case, the end of the slice. Otherwise, we end up with code like:
spy__slice$Slice spy__slice$Slice$__new__$impl6(void _start, int32_t _stop, void _step) { //<<< errror
return (spy__slice$Slice){ 0, 1, _stop, 0, 1, 1 };
}There was a problem hiding this comment.
ah I see, basically you want to support this scenario:
def foo(x: None) -> None:
passI never thought about it but I suppose it makes sense to allow it. It should have a test in test_basic.py though (like test_None_arguments)
There was a problem hiding this comment.
This appears to have been committed in error in an earlier PR
Added a |
antocuni
left a comment
antocuni
left a comment
There was a problem hiding this comment.
thank you @JeffersGlass!
There is a conflict in doppler.py, but feel free to merge it once the conflict is resolved and you add test_None_arguments in test_basic.py
2 participants
Implements slice objects by (1) writing a Slice class in spy and (2) turning Slice ast nodes into calls to the Slice constructor at frame eval time.