-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Add explanation for E0756 #76489
Add explanation for E0756 #76489
Conversation
b76f40f
to
d5a1765
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I thought #[ffi_const]
was somehow related to const fn
but it wasn't.
extern "C" { | ||
#[ffi_const] // ok! | ||
pub fn strlen(s: *const i8) -> i32; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same suggestion here. Show another method (or maybe a better one) with less contrast, no need to remove the current one.
#[ffi_pure]
extern "C" pub fn foo() {}
@@ -0,0 +1,29 @@ | |||
The `ffi_const` attribute was used on something else than a foreign function |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The `ffi_const` attribute was used on something else than a foreign function | |
The `ffi_const` attribute was used on something other than a foreign function |
``` | ||
|
||
The `ffi_const` attribute can only be used on foreign function declarations | ||
which have no effects except for their return value: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's not confuse people into thinking about effect type systems ;)
which have no effects except for their return value: | |
which have no side effects except for their return value: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously I was suggesting
The `ffi_pure` attribute can only be used on foreign functions which do not have
side effects or infinite loops:
which could be used exactly the same here, I wonder if that is better since I combined multiple parts from the book.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
C assumes that all functions don't have infinite loops; it's called "forward progress guarantee". The rules around it are somewhat odd and I don't think they belong here, see https://blog.rust-lang.org/inside-rust/2020/03/19/terminating-rust.html for details.
r? @jyn514 |
d5a1765
to
f72fc26
Compare
@bors: r=jyn514 rollup |
📌 Commit f72fc26cddb9255aa8a7fb12107fc41abe27879f has been approved by |
Rollup of 10 pull requests Successful merges: - rust-lang#76439 (Add error explanation for E0755) - rust-lang#76521 (Fix segfault if pthread_getattr_np fails) - rust-lang#76835 (make replace_prefix only take &str as arguments ) - rust-lang#76967 (Revert adding Atomic::from_mut.) - rust-lang#76977 (Add a regression test for copy propagation miscompilation) - rust-lang#76981 (liballoc bench use imported path Bencher) - rust-lang#76983 (BTreeMap: extra testing & fixed comments) - rust-lang#76996 (Fix typo in rustc_lexer docs) - rust-lang#77009 (Dogfood total_cmp in the test crate) - rust-lang#77012 (update Miri for another bugfix) Failed merges: - rust-lang#76489 (Add explanation for E0756) r? `@ghost`
☔ The latest upstream changes (presumably #77013) made this pull request unmergeable. Please resolve the merge conflicts. Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:
|
Merged in #77013, not sure why github didn't notice. |
No, it was E0755, not this one. ;) |
f72fc26
to
63195ec
Compare
Let's go again! @bors: r=jyn514 rollup |
📌 Commit 63195ec has been approved by |
…6, r=jyn514 Add explanation for E0756 r? @pickfire
…6, r=jyn514 Add explanation for E0756 r? @pickfire
…atic-morse Rollup of 13 pull requests Successful merges: - rust-lang#72734 (Reduce duplicate in liballoc reserve error handling) - rust-lang#76131 (Don't use `zip` to compare iterators during pretty-print hack) - rust-lang#76150 (Don't recommend ManuallyDrop to customize drop order) - rust-lang#76275 (Implementation of Write for some immutable ref structs) - rust-lang#76489 (Add explanation for E0756) - rust-lang#76581 (do not ICE on bound variables, return `TooGeneric` instead) - rust-lang#76655 (Make some methods of `Pin` unstable const) - rust-lang#76783 (Only get ImplKind::Impl once) - rust-lang#76807 (Use const-checking to forbid use of unstable features in const-stable functions) - rust-lang#76888 (use if let instead of single match arm expressions) - rust-lang#76914 (extend `Ty` and `TyCtxt` lints to self types) - rust-lang#77022 (Reduce boilerplate for BytePos and CharPos) - rust-lang#77032 (lint missing docs for extern items) Failed merges: r? `@ghost`
r? @pickfire