Open obi1kenobi opened 1 year ago
Hi! I started to work on this. A couple of questions/uncertainties:
repr
, although I couldn't find anything on the cargo semver page. for no_mangle/export_name specificallyim_secretly_public
even when it's not public in the Rust API.
#[no_mangle]
pub fn func1() {}
pub fn func2() {}
to
```rust
pub fn func1() {}
#[export_name = "func1"]
pub fn func2() {}
Thanks!
Great questions!
I'm assuming it's a major-level change based on other violations like changing an established
repr
, although I couldn't find anything on the cargo semver page. for no_mangle/export_name specifically
Yes, it's a major breaking change. The cargo semver reference isn't yet an exhaustive list of major changes, and we've had many situations where cargo-semver-checks work triggered additions to the reference or vice versa. I expect that trend will continue for some time :)
Is it a violation when a non-public function changes export name? For example, in the example you linked, you can still access
im_secretly_public
even when it's not public in the Rust API.
Yes! I believe we might actually have a lint for this already. It seems familiar and may have been added sometime in the last couple of months.
Should this (these) lints proc when the export name of one function moves to a different function? For example,
Great edge case! No, this should not trigger either lint.
In general, we want to trigger lints when we are certain there's a problem, so that we skew heavily away from false-positives which cause users to distrust the tool. In this edge case, the change may have been intentional and from an API/ABI perspective seems fine.
However, if the func2()
function e.g. takes a different number of arguments, then that should trigger a lint — I'm adding that plus related cases to the list.
Consider a function like this:
The following change is non-breaking, since it's equivalent:
However, changing to any other
export_name
value (one that does not match the original name of the function) is breaking.Example code where you can play with
#[no_mangle]
and#[export_name]
: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=64663d9ca343dee3ba9d2c7cabe61ab4We have an
export_name
property on functions which sorts out#[no_mangle]
and#[export_name]
under the hood, and presents the actual export name (if any) of the function: https://github.com/obi1kenobi/trustfall-rustdoc-adapter/blob/rustdoc-v27/src/rustdoc_schema.graphql#L618We want the following lints: