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

Mechanize interface generation, part 2 #43

Closed
wants to merge 25 commits into from

Conversation

kulp
Copy link
Collaborator

@kulp kulp commented Jul 15, 2020

Goals

  1. Demonstrate improved predictability of the Rust interface (how closely the Rust interface follows the C interface)
  2. Reduce the risk of failing to implement an entry point from the C library
  3. Increase the likelihood that any API changes or additions in future GNU lightning versions will either be supported automatically, or manifested as build failures in lightning-sys, as opposed to being hidden

Means

Using the foundation laid in #32, the current PR implements a pattern-matching paradigm to generate public entry point wrappers.

Gains

  1. The Rust interface hews more closely to the C interface, reducing surprise for the user.
  2. The risk of some kinds of bugs may be reduced, by decreasing the number of failure modes.
  3. Self-checking is improved, since the new mechanism generates simple test cases to demonstrate that that all jit_-macro entry points advertised by GNU lightning's lightning.h exist, and (in most cases) that the entry points can be called with zeroed parameters without crashing.
  4. The net volume of code to maintain (even including what was introduced in Mechanize interface generation, part 1 #32) is decreased somewhat, after accounting for the generated tests that did not previously exist.

Costs

The new implementation (including work done in #32) carries a significant burden of complexity, especially in macros. The use of tt-call makes the macros more composable and understandable individually, but contributes to the verbosity of the macro implementation.

@kulp kulp mentioned this pull request Jul 15, 2020
@codecov
Copy link

codecov bot commented Jul 15, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@6271686). Click here to learn what that means.
The diff coverage is n/a.

@@ -1,7 +1,7 @@
[package]
name = "lightning-sys"
version = "0.2.1"
authors = ["Peter Elliott <pelliott@ualberta.ca>"]
authors = ["Peter Elliott <pelliott@ualberta.ca>", "Darren Kulp <darren@kulp.ch>"]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I hope adding my name is all right here. I do not care so much, but I thought it would make sense to match the crates.io ownership list.

Copy link
Owner

Choose a reason for hiding this comment

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

About time :^)

@kulp kulp mentioned this pull request Jul 16, 2020
@kulp kulp added this to the v0.3 milestone Jul 20, 2020
@kulp
Copy link
Collaborator Author

kulp commented Jul 29, 2020

I am planning to get back to this soon (at worst, by early August), with robust justification; I just ended up working on other things for a while.

kulp added 20 commits August 28, 2020 16:43
`jit_entry`-style support for aliases of these functions will not be
ready right away. For convenience, collect these aliases for specific
entry points that are not provided by `jit_entry` yet.
This stops jit_register_p from being split into `jit_registe`, `r`,
 `_p`.
Check for entry point existence

Introduce infrastructure for top-level wrappers

Add explicit ignores for many instructions

Convert compilation check to run check
Prevent testing for jit_destroy_state

Avoid testing disassembly when not present
`cargo doc` shows that entry points like `JitState::jit_absr_d` were
being exposed publicly outside the crate. This seems to be because,
despite being in a non-`pub` `mod` named `raw`, they are associated with
a `pub` `type` named `JitState`. Hide them internal to the crate for
now.
@kulp
Copy link
Collaborator Author

kulp commented Sep 5, 2020

I am splitting the current PR into several new PRs.

  1. Update build.rs with collected mechanization fixes #50 - collected improvements to the build.rs mechanization to support future work
  2. Collect various cleanup changes #51 - general cleanup
  3. Test entry points using mechanization #54 - a part that uses the jit_entry! macros introduced in Mechanize interface generation, part 1 #32 to test for entry point existence
  4. Generate entry points via mechanization and pattern matching #55 - a part that uses the jit_entry! macros to match and implement entry points

This splitting-up should make it easier to evaluate the costs and benefits of the new implementation. Much of the benefit is gained from parts 1-3.

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.

None yet

2 participants