Add --preserve-unused-functions

[chrysn]

to keep otherwise pruned static / inline functions.

Closes: #287

---

This is an initial simple approach to #287. It looks like it's generating valid code, but I'm too early in my whole evaluation of c2rust to say whether what comes out of this will work, so please take it with due care. (Also, tests don't pass, but neither do they pass on my machine on current master, so...)

I'm playing fast and lose with API stability here, as I don't have the overview to tell whether prune_unused_decls is part of any practically public API. If it is, I can replace the added parameter with a second function prune_unused_decls_but_not_functions and rename the current implementation to a private impl one.

[thedataking]

Looks good overall. I think it is fine to add a parameter to prune_unused_decls as you've done; the function is internal as far as I'm aware.

The tests should pass before we merge. We're hitting an upstream bug on macOS (see #289). If that is not what you are hitting, what platform are you seeing failures on?

[thedataking]

The used variable no longer tracks what is used because of the option to preserve unused functions. Now it tracks what we want to keep, so we should consider renaming it to keep or similar to reflect that.

[chrysn]

Happy to adjust that, though when I get to that level I'll also want to refactor the whole match.

[chrysn]

Renamed in b19396a now; I'm not sure if you'll be happy with it because doing it all the way through meant renaming the function as well (for otherwise things would be inconsistent there). Just have a look, I'd understand if you rather not have that commit.

[edit: commit ref was broken, fixed now]

[chrysn]

I'm building on Debian GNU/Linux unstable, with Rust provided by rustup. All that follows was done on current master (4062985).

On cargo test in the repository root, I hit

error[E0432]: unresolved import `crate::ast`
 --> c2rust-ast-printer/src/pprust/tests.rs:3:5
  |
3 | use crate::ast;
  |     ^^^^^^^^^^ no `ast` in the root

error[E0432]: unresolved import `crate::source_map`
 --> c2rust-ast-printer/src/pprust/tests.rs:4:5
  |
4 | use crate::source_map;
  |     ^^^^^^^^^^^^^^^^^ no `source_map` in the root

error[E0432]: unresolved import `crate::with_default_globals`
 --> c2rust-ast-printer/src/pprust/tests.rs:5:5
  |
5 | use crate::with_default_globals;
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^ no `with_default_globals` in the root

error: aborting due to 3 previous errors

whereas on nightly, I hit

error[E0462]: found staticlib `rustc` instead of rlib or dylib
 --> c2rust-ast-builder/src/lib.rs:2:1
  |
2 | extern crate rustc;
  | ^^^^^^^^^^^^^^^^^^^
  |
  = help: please recompile that crate using --crate-type lib
  = note: the following crate versions were found:
          crate `rustc`: /home/chrysn-cache/dot-cache/rustup-toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/librustc-nightly_rt.lsan.a
          crate `rustc`: /home/chrysn-cache/dot-cache/rustup-toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/librustc-nightly_rt.tsan.a
          crate `rustc`: /home/chrysn-cache/dot-cache/rustup-toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/librustc-nightly_rt.asan.a
          crate `rustc`: /home/chrysn-cache/dot-cache/rustup-toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/librustc-nightly_rt.msan.a

When running in c2rust-transpile, things compile on stable (nightly has the same error, I may be missing some parts from rustup there), but fails at several places.

`cargo test` in c2rust-transpile

``` running 3 tests test renamer::tests::forgets ... ok test renamer::tests::simple ... ok test renamer::tests::scoped ... ok

test result: ok. 3 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

running 6 tests
test src/translator/structs.rs - translator::structs::Translation::convert_struct_fields (line 264) ... FAILED
test src/translator/structs.rs - translator::structs::Translation::convert_struct_literal (line 367) ... FAILED
test src/rust_ast/comment_store.rs - rust_ast::comment_store (line 20) ... FAILED
test src/cfg/multiples.rs - cfg::multiples (line 40) ... FAILED
test src/cfg/multiples.rs - cfg::multiples (line 18) ... FAILED
test src/rust_ast/comment_store.rs - rust_ast::comment_store (line 11) ... FAILED

failures:

---- src/translator/structs.rs - translator::structs::Translation::convert_struct_fields (line 264) stdout ----
error: cannot find derive macro BitfieldStruct in this scope
--> src/translator/structs.rs:265:10
|
3 | #[derive(BitfieldStruct, Clone, Copy)]
| ^^^^^^^^^^^^^^

error: cannot find attribute bitfield in this scope
--> src/translator/structs.rs:268:7
|
6 | #[bitfield(name = "bf1", ty = "libc::c_char", bits = "0..=9")]
| ^^^^^^^^

error: cannot find attribute bitfield in this scope
--> src/translator/structs.rs:269:7
|
7 | #[bitfield(name = "bf2", ty = "libc::c_uchar",bits = "10..=15")]
| ^^^^^^^^

error: aborting due to 3 previous errors

Couldn't compile the test.
---- src/translator/structs.rs - translator::structs::Translation::convert_struct_literal (line 367) stdout ----
error[E0422]: cannot find struct, variant or union type Foo in this scope
--> src/translator/structs.rs:369:20
|
4 | let mut init = Foo {
| ^^^ not found in this scope

error: aborting due to previous error

For more information about this error, try rustc --explain E0422.
Couldn't compile the test.
---- src/rust_ast/comment_store.rs - rust_ast::comment_store (line 20) stdout ----
error[E0412]: cannot find type CommentTraverser in this scope
--> src/rust_ast/comment_store.rs:21:11
|
3 | let trav: CommentTraverser = cmmt_store.into_comment_traverser();
| ^^^^^^^^^^^^^^^^ not found in this scope
|
help: possible candidate is found in another module, you can import it into scope
|
2 | use c2rust_transpile::rust_ast::comment_store::CommentTraverser;
|

error[E0425]: cannot find value cmmt_store in this scope
--> src/rust_ast/comment_store.rs:21:30
|
3 | let trav: CommentTraverser = cmmt_store.into_comment_traverser();
| ^^^^^^^^^^ not found in this scope

error[E0412]: cannot find type Mod in this scope
--> src/rust_ast/comment_store.rs:22:23
|
4 | let updated_module: Mod = trav.traverse_mod(module);
| ^^^ not found in this scope

error[E0425]: cannot find value module in this scope
--> src/rust_ast/comment_store.rs:22:47
|
4 | let updated_module: Mod = trav.traverse_mod(module);
| ^^^^^^ not found in this scope

error: aborting due to 4 previous errors

Some errors have detailed explanations: E0412, E0425.
For more information about an error, try rustc --explain E0412.
Couldn't compile the test.
---- src/cfg/multiples.rs - cfg::multiples (line 40) stdout ----
error[E0425]: cannot find value i in this scope
--> src/cfg/multiples.rs:41:4
|
3 | if i > 5i32 {
| ^ not found in this scope

error[E0425]: cannot find value i in this scope
--> src/cfg/multiples.rs:42:11
|
4 | while i > 0i32 {
| ^ not found in this scope

error[E0425]: cannot find value i in this scope
--> src/cfg/multiples.rs:43:9
|
5 | i -= 3i32
| ^ not found in this scope

error: aborting due to 3 previous errors

For more information about this error, try rustc --explain E0425.
Couldn't compile the test.
---- src/cfg/multiples.rs - cfg::multiples (line 18) stdout ----
error[E0425]: cannot find value i in this scope
--> src/cfg/multiples.rs:20:4
|
4 | if i > 5i32 { current_block = "s_7"; } else { current_block = "s_14"; }
| ^ not found in this scope

error[E0425]: cannot find value i in this scope
--> src/cfg/multiples.rs:24:18
|
8 | if !(i > 0i32) { current_block = "s_14"; continue ; }
| ^ not found in this scope

error[E0425]: cannot find value i in this scope
--> src/cfg/multiples.rs:25:13
|
9 | i -= 3i32;
| ^ not found in this scope

error: aborting due to 3 previous errors

For more information about this error, try rustc --explain E0425.
Couldn't compile the test.
---- src/rust_ast/comment_store.rs - rust_ast::comment_store (line 11) stdout ----
error: unexpected token: ...
--> src/rust_ast/comment_store.rs:14:3
|
5 | ...
| ^^^
|
help: use .. for an exclusive range
|
5 | ..
|
help: or ..= for an inclusive range
|
5 | ..=
|

error[E0586]: inclusive range with no end
--> src/rust_ast/comment_store.rs:15:1
|
6 | }
| ^
|
= help: inclusive ranges must be bounded at the end (..=b or a..=b)

error[E0412]: cannot find type Span in this scope
--> src/rust_ast/comment_store.rs:12:9
|
3 | let sp: Span = cmmt_store.add_comment_lines(vec!["Some comment on an arm"]);
| ^^^^ not found in this scope

error[E0425]: cannot find value cmmt_store in this scope
--> src/rust_ast/comment_store.rs:12:16
|
3 | let sp: Span = cmmt_store.add_comment_lines(vec!["Some comment on an arm"]);
| ^^^^^^^^^^ not found in this scope

error[E0425]: cannot find function mk in this scope
--> src/rust_ast/comment_store.rs:13:13
|
4 | let arm = mk().span(sp).arm(pats, None, body);
| ^^
|
help: a tuple variant with a similar name exists
|
4 | let arm = Ok().span(sp).arm(pats, None, body);
| ^^
help: possible candidate is found in another module, you can import it into scope
|
2 | use c2rust_ast_builder::mk;
|

error[E0425]: cannot find value pats in this scope
--> src/rust_ast/comment_store.rs:13:31
|
4 | let arm = mk().span(sp).arm(pats, None, body);
| ^^^^ not found in this scope

error[E0425]: cannot find value body in this scope
--> src/rust_ast/comment_store.rs:13:43
|
4 | let arm = mk().span(sp).arm(pats, None, body);
| ^^^^ not found in this scope

error: aborting due to 7 previous errors

Some errors have detailed explanations: E0412, E0425, E0586.
For more information about an error, try rustc --explain E0412.
Couldn't compile the test.

failures:
src/cfg/multiples.rs - cfg::multiples (line 18)
src/cfg/multiples.rs - cfg::multiples (line 40)
src/rust_ast/comment_store.rs - rust_ast::comment_store (line 11)
src/rust_ast/comment_store.rs - rust_ast::comment_store (line 20)
src/translator/structs.rs - translator::structs::Translation::convert_struct_fields (line 264)
src/translator/structs.rs - translator::structs::Translation::convert_struct_literal (line 367)

test result: FAILED. 0 passed; 6 failed; 0 ignored; 0 measured; 0 filtered out

</details>

[thedataking]

We don't use cargo test which is probably why it has bitrotted. Can you try path/to/scripts/test_translator.py path/to/tests? Please keep in mind that you should use the nightly version of rust specified in rust-toolchain. Any other version is not expected to work.

[chrysn]

OK, that explains how things went south. Running ./scripts/test_translator.py tests/ now, I get what looks like an overall success on this branch:

Test summary:
  unexpected failures: 0
  unexpected successes: 0
  expected failures: 3
  successes: 108

[chrysn]

Just to ensure I'm testing the right thing next time (I reinstalled to be sure and ran the test again with the same result): Does this test the installed version or the crate / git repo it is in?

[thedataking]

Good to hear that the tests pass on your host. test_transpiler.py will pick the c2rust binary that you have under target/{debug,release} in your local git repo.

[chrysn]

The unsuccessful tests all appear to be unrelated to the PR itself. What can I do to progress this? Can I trigger the CI again, eg. by force-pushing an amend with a new timestamp? Is there anything review-wise that still needs addressing?

[thedataking]

I prefer this wording: "Include static and inline functions in translation".

[thedataking]

@chrysn I looked at the failures and agree that you didn't cause these. I (think I) reconfigured GitHub actions to not build PRs. As for the Azure pipeline failure on macOS, that is caused by a homebrew issue with the LLVM formula on macOS.

Update: can you try rebasing your PR on top of master? Should pass the Azure tests on all platforms now.

[chrysn]

Wording adapted and rebased.

[chrysn]

Revisiting this from a bindgen interaction point of view, I'm questioning my choice of limiting this to body: Some(_). The limitation to "bodily" functions makes sense for the use case I'm working towards. But for users or developers coming from the traditional C2Rust usage: Does the name --preserve-unused-functions make sense and imply that it's only about additionally transpiling static inlines but not about pulling in function delcarations? (I still vaguely think so, but it'd help to double-check.)

[thedataking]

I think the flag name is fine. We can rely on the help text to provide further guidance to other users – and update the text in the unlikely event that users are frequently confused about this flag.