Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generate pxd #70

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from
Draft

Conversation

0dminnimda
Copy link
Contributor

Closes #69

nice, sorry ;]

@MatthieuDartiailh
Copy link
Collaborator

Could we get some benchmarks ? Given the workflow of pegen it is not obvious to me how much cython will be able to optimize things. In particular since we would have to drop the walrus operator which makes the generated parser more readable IMO.

@MatthieuDartiailh
Copy link
Collaborator

Also if we can wait for Cython 3.0 we will get walrus support (see cython/cython#2636). However I have no idea regarding the timeline for its release.

@0dminnimda
Copy link
Contributor Author

Also if we can wait for Cython 3.0 we will get walrus support (see cython/cython#2636). However I have no idea regarding the timeline for its release.

It's very unlikely that it'll be out in 2022
There are not a lot of people working on cython, so unfortunately 3.0 will not be out soon

@0dminnimda
Copy link
Contributor Author

In particular since we would have to drop the walrus operator which makes the generated parser more readable IMO.

Agree, with walrus operator code looks nicer, but because it's generated the code should not be rated by the readability as it's not supposed to be read

Additionally I tried to make the code readable in the no_walrus

example of the loop

    def _loop0_208(self) -> Optional[Any]:
        # _loop0_208: ',' double_starred_kvpair
        mark = self._mark()
        children = []
        while 1:  # recursive
            literal = self.expect(',')
            if not literal: break
            elem = self.double_starred_kvpair()
            if not elem: break
            children.append(elem)
            mark = self._mark()
        self._reset(mark)
        return children;

or just regular rule

    def double_starred_kvpair(self) -> Optional[Any]:
        # double_starred_kvpair: '**' bitwise_or | kvpair
        mark = self._mark()
        while 1:
            literal = self.expect('**')
            if not literal: break
            a = self.bitwise_or()
            if not a: break
            return ( None , a );
            break
        self._reset(mark)
        while 1:
            kvpair = self.kvpair()
            if not kvpair: break
            return kvpair;
            break
        self._reset(mark)
        return None;

@0dminnimda
Copy link
Contributor Author

Could we get some benchmarks ?

Of course

Also regarding drop of the walrus. I'll merge no_walrus branch here because changes will not work without it.
And if we'll be fine with that change I will open a pr for this branch, so it could be merged first

@jnoortheen
Copy link

fyi, walrus operator support is now released with 3.0a11 version https://github.com/cython/cython/blob/master/CHANGES.rst#300-alpha-11-2022-07-31

@0dminnimda
Copy link
Contributor Author

@jnoortheen oh well, now there are better news, the beta 3.0.0b1 was finally released!

I actually made some good progress on this and stuck trying to make memorization decorators to be optimized as well
if I remember correctly I did not push changes that I was not satisfied with / that didn't really work

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature] Optionally create .pxd files
3 participants