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

Find a way to detect API breakage. #17152

Closed
mahkoh opened this issue Sep 10, 2014 · 15 comments
Closed

Find a way to detect API breakage. #17152

mahkoh opened this issue Sep 10, 2014 · 15 comments

Comments

@mahkoh
Copy link
Contributor

mahkoh commented Sep 10, 2014

Given that the current rust ecosystem tries very hard to force everyone to use semver, and given that rust code breaks the API easily, this is very important.

Some easy but surprising ways to break the API:

Therefore rustc needs to have something like this:

rustc --record-api out.json lib_old.rs

stores api information in out.json

rustc --compare-api out.json lib_new.rs

Checks if the new api is compatible with the old one.

@aturon
Copy link
Member

aturon commented Sep 10, 2014

We had an extensive discussion at the last work week about what exactly should count as a breaking change. I hope to write this up in a more formal way and propose it as official guidelines for semver/stability attributes.

Automating these checks via a tool would be amazing.

@huonw
Copy link
Member

huonw commented Sep 11, 2014

Adding a field to a struct.

Adding a public field.

@thestinger
Copy link
Contributor

Adding a private field is also a breaking change if it had no private fields before. Adding a public field is only a breakage change if it had no private fields before.

@mahkoh
Copy link
Contributor Author

mahkoh commented Nov 24, 2014

Has anything happened here? Another one:

  • Making a previously non-panicking function panic.

@eddyb
Copy link
Member

eddyb commented Nov 24, 2014

@mahkoh That affects ABI (nounwind and emitting landing pads), but why does it impact API compatibility?
All code that previously compiled against a library with non-panicking functions, should still compile when those functions are changed to panic.

@mahkoh
Copy link
Contributor Author

mahkoh commented Nov 24, 2014

You cannot call panicking functions in destructors if your program depends on destructors running.

@mahkoh
Copy link
Contributor Author

mahkoh commented Nov 24, 2014

This might also cause UB if you previously counted on a function not panicking while writing unsafe code.

@huonw
Copy link
Member

huonw commented Nov 24, 2014

A reliable program cannot depend on destructors running. E.g. the operating system can just kill it outright due to OOM, at essentially any time.

@mahkoh
Copy link
Contributor Author

mahkoh commented Nov 24, 2014

This is correct but some people do rely on it. See for example this thread #19245

Those who defend unwinding sometimes argue that it is essential for certain programs such as Servo which relies on unwinding. But the process will simply abort if destructors fail during unwinding.

@mahkoh
Copy link
Contributor Author

mahkoh commented Dec 18, 2014

It's actually worse than that. Changing a function from non-panicking to panicking can cause UB in previously completely safe code. Consider the following module written in rust and called from C code:

#[no_mangle]
pub extern fn hello_rust() -> *const u8 {
    "Hello, world!\0".as_ptr()
}

If as_ptr introduces a panicking case, then this will cause UB without having to use unsafe.

@eddyb
Copy link
Member

eddyb commented Dec 18, 2014

@mahkoh I think the plan is to catch unwinding and abort at extern "C" boundaries.

@mahkoh
Copy link
Contributor Author

mahkoh commented Jan 16, 2015

The more common case is unwinding in unsafe code.

ptr::copy_nonoverlapping_memory(dst.as_mut_ptr(), ptr, elts);

Changing the conditions under which a function can panic has to be considered a breaking change.

@mahkoh
Copy link
Contributor Author

mahkoh commented Jan 28, 2015

rust-lang/rfcs#757 addresses

  • Adding a variant to an enum.

@steveklabnik
Copy link
Member

I'm pulling a massive triage effort to get us ready for 1.0. As part of this, I'm moving stuff that's wishlist-like to the RFCs repo, as that's where major new things should get discussed/prioritized.

This issue has been moved to the RFCs repo: rust-lang/rfcs#759

@mahkoh
Copy link
Contributor Author

mahkoh commented Jan 29, 2015

I believe this should stay here at least until it has been decided what is considered breaking the API.

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

No branches or pull requests

6 participants