Skip to content

Expose a C API for pegen #11

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 11 commits into from
Mar 23, 2020
Merged

Expose a C API for pegen #11

merged 11 commits into from
Mar 23, 2020

Conversation

@lysnikolaou
Copy link
Member

@lysnikolaou lysnikolaou commented Mar 22, 2020 •
edited by gvanrossum
Loading

Expose pegen as a C API and as a Python-leve module:

  • Use trailer meta for a function that calls the start rule. (In the future we could add more parse_* functions that use other rules)
  • Expose four C API functions that return either a mod_ty or a PyCodeObject *
  • Expose two Python-level functions that return an AST object
  • Extension module and C API does not get generated, but is hand-written
  • Rewrite of run_parser* functions

This is getting bigger and bigger, so I'm opening this WIP PR, so that it becomes a bit easier to follow what's going on.

Feedback is needed once more.

Closes #6.

pablogsal and others added 3 commits March 20, 2020 23:56
@pablogsal
WIP: Start the work towards exposing pegen as a C-API
7e13f96 
Check the TODO items!
@lysnikolaou
Expose pegen both as a C API and as a Python-level module:
1d695fa 
    * Use @trailer for the generated C API functions
    * Use EXTENSION_SUFFIX for the generated Python-level module
    * Expose four C API functions that return either a mod_ty or a
      PyCodeObject *
    * Expose two Python-level functions that return an AST object
@lysnikolaou
Remake of the design of the PyPegen C API and how it is generated:
3d20a52 
    * Extension modules does not get generated anymore
    * Complete rewrite of run_parser* functions
    * Use trailer meta for only one function that calls the start_rule
@gvanrossum
Copy link

I'll have a look at the code soon. Right now I notice that (on my Mac) make test inside the Tools/peg_generator directory segfaults. This is using Python 3.8.2.

@lysnikolaou
Copy link
Member Author

lysnikolaou commented Mar 22, 2020 •
edited
Loading

I'll have a look at the code soon. Right now I notice that (on my Mac) make test inside the Tools/peg_generator directory segfaults. This is using Python 3.8.2.

I'm on it.

EDIT: Fixed in a5bb3a2.

@lysnikolaou lysnikolaou mentioned this pull request Mar 23, 2020
Merged
gvanrossum
gvanrossum reviewed Mar 23, 2020
View reviewed changes
Copy link

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great progress. But I think we need some refactoring. (Some you may be able to put off to another PR.)

Also, I'm sorry that there's so much duplicate code between here and Tools/peg_generator -- hopefully this is temporary!

pablogsal
pablogsal reviewed Mar 23, 2020
View reviewed changes
@lysnikolaou lysnikolaou mentioned this pull request Mar 23, 2020
Merged
gvanrossum
gvanrossum reviewed Mar 23, 2020
View reviewed changes
Copy link

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR is getting so large, maybe finish these nits, land, and start another iteration with the API inmprovements?

@lysnikolaou
Copy link
Member Author

lysnikolaou commented Mar 23, 2020 •
edited
Loading

Should we merge this now?

Note that pegen tests pass and all the other will fail one way or the other.

gvanrossum
gvanrossum approved these changes Mar 23, 2020
View reviewed changes
Copy link

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, let's land this one!

@lysnikolaou lysnikolaou changed the title WIP: Work on exposing a C API for pegen Expose a C API for pegen Mar 23, 2020
@lysnikolaou lysnikolaou merged commit 0c7a382 into pegen Mar 23, 2020
@lysnikolaou lysnikolaou deleted the c_api_lys branch March 23, 2020 18:27
@lysnikolaou lysnikolaou mentioned this pull request Mar 23, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pablogsal pablogsal pablogsal left review comments

@gvanrossum gvanrossum gvanrossum approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add an API that invokes the new and the old parsers explicitly

4 participants

@lysnikolaou @gvanrossum @pablogsal