Skip to content

Option to disable pruning of static functions #287

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

Closed
chrysn opened this issue Sep 2, 2020 · 2 comments · Fixed by #291
Closed

Option to disable pruning of static functions #287

chrysn opened this issue Sep 2, 2020 · 2 comments · Fixed by #291

Comments

@chrysn
Copy link
Contributor

chrysn commented Sep 2, 2020

C2Rust, by default and since 8bcb5c9, removes unneeded definitions, which includes static inline functions.

Exporting them is useful when C2Rust is used outside a full transpilation setting -- for example when a C library is accessed by cbindgen, but some functions are declared static inline. (Exporting them anyhow there might be an option; however, as they are static inline, chances are they're used in hot loops, and avoiding the function call would be really nice there. There's some discussion already at rust-lang/rust-bindgen#1344; I'm trying to get that done in a step-by-step fashion).

As a crude proof-of-concept, making TypedAstContext::prune_unused_decls return immediately does the trick; as a next refinement, so does matching on any CDeclKind::Function with Some body by ripping out the conditions -- but that's not really feasible outside experimental setups.

  • Did I miss an easier way to get where I want?
  • Would this be something c2rust could get as an option? Say, c2rust transpile x.json --translate-static-inline.
  • Is ignoring the is_global, is_inline and is_inline_externally_visible attributes a good enough approximation (or even precisely) "including all functions, no matter whether they're static or inline"?
  • (As I'm quite unfamiliar with the code base:) Can you give me some pointers on where I could add that option and pass it on to TypedAstContext? I figure I'd manage to pass on a boolean attribute in it, but there may be something already that carries any settings.
@thedataking
Copy link
Contributor

Thanks for doing some initial research on this @chrysn; I don't know how to preserve these functions without adding a new flag. c2rust transpile already has options that disable internal features that are otherwise on by default (--no-incremental-relooper, --no-simplify-structures) so the matching flag here would be --no-prune-unused-functions IMO.

The transpiler flags are defined in transpile.yaml in the c2rust/src folder.

Re. the attributes you mention (is_global, is_inline and is_inline_externally_visible): why not add a new match case in prune_unused_decls that match any function if the --no-prune-unused-functions flag is set?

@rinon
Copy link
Contributor

rinon commented Sep 2, 2020

I'd call it --preserve-unused-functions or something, but that sounds great. This should probably be added as an option to TranspilerConfig in c2rust-translate/lib.rs.

chrysn added a commit to chrysn-pull-requests/c2rust that referenced this issue Sep 3, 2020
to keep otherwise pruned static / inline functions.

Closes: immunant#287
chrysn added a commit to chrysn-pull-requests/c2rust that referenced this issue Sep 15, 2020
to keep otherwise pruned static / inline functions.

Closes: immunant#287
thedataking pushed a commit that referenced this issue Sep 15, 2020
* Add --preserve-unused-functions

to keep otherwise pruned static / inline functions.

Closes: #287

* transpile: Reduce code duplication

* prune_unused_decls: Rename to unwanted

The criterion for whether it is desired for an item to be transpiled
have been broadened to possibly preserve unused functions as well. Thus,
the "unused" name part has been replaced with "unwanted", which
traditionally means unused but, in other configurations, can mean
"unused but not a function" now as well.
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 a pull request may close this issue.

3 participants