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

Remove libc from std #12526

Closed
wants to merge 10 commits into from
Closed

Remove libc from std #12526

wants to merge 10 commits into from

Conversation

emberian
Copy link
Member

Remove libc from std

@alexcrichton
Copy link
Member

cc @brson/@thestinger

  • liblibc :)
  • Could extern crate c; work? Our naming scheme means that we'd never collide with a system libc.
  • std depending on libc (which kinda makes sense) means that we probably couldn't move c_vec or c_str into this module (which seem desirable?)
  • #[no_std] seems correct to me.

@brson
Copy link
Contributor

brson commented Feb 24, 2014

c_vec at least has very few dependencies. c_str is much harder. c_str on the other hand has very few dependencies on libc

@huonw
Copy link
Member

huonw commented Feb 24, 2014

cc #12514 and #2124

@brson
Copy link
Contributor

brson commented Feb 24, 2014

Regardless of the ultimate fate of libc bindings this is a good first step. The big question to me is "liblibc or libc?".

@brson
Copy link
Contributor

brson commented Feb 24, 2014

Or "libclib"?

@brson
Copy link
Contributor

brson commented Feb 24, 2014

It would be nice if std somebody does not depend on libc (or maybe just ctypes).

@alexcrichton
Copy link
Member

Ah another thing that I was just reminded of is that it's likely for many crates to depend on C types, but not necessarily on the C functions. Just a thought though.

I think that moving c_vec and c_str into this crate will require a libprim in one form or another. (both need the Drop trait for example).

@liigo
Copy link
Contributor

liigo commented Feb 25, 2014

"liblibc"?? A little weird. Why not just "libc"? I really think c::int is better than libc::c_int.

@emberian
Copy link
Member Author

I don't think c_* moving here is a good idea. This is bindings to the C standard library, not utilities for interfacing with C in general. Those could probably usefully live in a std::ffi

@huonw
Copy link
Member

huonw commented Feb 25, 2014

I really think c::int is better than libc::c_int.

I think there's a bug in resolve that types with the same name as primitives aren't resolved correctly, so this would restrict it to always being used as c::int, possibly resulting in weird FFI bugs. (I can't find the bug filed about the resolution bug now.)

@emberian
Copy link
Member Author

Additionally, they're always imported as "c_int" etc, so the name is
actually getting longer and harder to read IMO.

On Mon, Feb 24, 2014 at 7:19 PM, Huon Wilson notifications@github.comwrote:

I really think c::int is better than libc::c_int.

I think there's a bug in resolve that types with the same name as
primitives aren't resolved correctly, so this would restrict it to
always being used as c::int, possibly resulting in weird FFI bugs. (I
can't find the bug filed about the resolution bug now.)


Reply to this email directly or view it on GitHubhttps://github.com//pull/12526#issuecomment-35960256
.

@brson
Copy link
Contributor

brson commented Feb 25, 2014

It would be nice if "someday" std doesn't depend on libc.

@emberian
Copy link
Member Author

Do we want to name it c? I can try and fix the resolve bug (which I just confirmed) if so.

@bharrisau
Copy link
Contributor

If rt has all the "talk to the OS" stuff, and libc/ffi has all the "talk to
external C code", would that count as std not depending on libc?

@emberian
Copy link
Member Author

rt is in std, but there's also a std::os, and std::io uses a lot of libc. at least the types.

@liigo
Copy link
Contributor

liigo commented Feb 25, 2014

c::int VS. libc::c_int
To import: don't need VS. use libc::{c_int}
To use: let x: c::int VS. let x: c_int
To read: the same, read as c int
c::int is a bit more cleaner.
@cmr @huonw @brson

@huonw
Copy link
Member

huonw commented Feb 25, 2014

Having two types called int that behave differently is a no-go for me; the bugs it will cause will be very annoying to track down. Examples just off the top of my head

extern {
    fn foo(x: int); // whoops, meant to be c::int, subtly incorrect on x86-64
}
use c::int;
extern {
    // ... use `int` a lot ...
}

fn x() -> uint { computation() }

fn y() {
    let i = x() as int; // whoops, meant to be built-in `int`, lost precision on x86-64
    // ...
}

One might say "no-one will ever import c::int to be just int", but if it's legal Rust then it will end up in codebases.

Basically, it would make FFI a bigger mine-field than it already is: it's confusing enough that Rust's int isn't the same as C's int: having two Rust ints which sometime match C and sometimes don't would be horrible.

@emberian
Copy link
Member Author

So r?

@flaper87
Copy link
Contributor

@cmr looks like this needs to be rebased.

@emberian
Copy link
Member Author

updated

On Tue, Feb 25, 2014 at 9:13 AM, Flavio Percoco Premoli <
notifications@github.com> wrote:

@cmr https://github.com/cmr looks like this needs to be rebased.


Reply to this email directly or view it on GitHubhttps://github.com//pull/12526#issuecomment-36010552
.

@liigo
Copy link
Contributor

liigo commented Feb 25, 2014

Please keep in mind, we do have namespaces, c::int is not the same as
int.
2014年2月25日 下午7:19于 "Huon Wilson" notifications@github.com写道:

Having two types called int that behave differently is a no-go; the bugs
it will cause will be very annoying to track down. Examples just off the
top of my head

extern {
fn foo(x: int); // whoops, meant to be c::int, subtly incorrect on x86-64}

use c::int;extern {
// ... use int a lot ...}
fn x() -> uint { computation() }
fn y() {
let i = x() as int; // whoops, meant to be built-in int, lost precision on x86-64
// ...}

One might say "no-one will ever import c::int to be just int", but if
it's legal Rust then it will end up in codebases.

Basically, it would make FFI a bigger mine-field than it already is: it's
confusing enough that Rust's int isn't the same as C's int: having two
Rust ints which sometime match C and sometimes don't would be horrible.


Reply to this email directly or view it on GitHubhttps://github.com//pull/12526#issuecomment-35997743
.

@ghost
Copy link

ghost commented Feb 25, 2014

We should keep c_int and friends in std. We'll need them without libc too.

@emberian
Copy link
Member Author

@liigo not when you use it, and libc itself won't be able to use those
due to a bug. There's traction for making the built-in types keywords, too,
so that would make it a moot point.

On Tue, Feb 25, 2014 at 10:51 AM, György Andrasek
notifications@github.comwrote:

We should keep c_int and friends in std. We'll need them without libc too.


Reply to this email directly or view it on GitHubhttps://github.com//pull/12526#issuecomment-36021945
.

@thestinger
Copy link
Contributor

@Jurily: libstd can depend on libc

@ghost
Copy link

ghost commented Feb 25, 2014

The basic C types are not defined by libc. We need them to do any sort of FFI including kernel modules and writing libc itself. They're just typedefs over Rust builtins anyway, I don't see a good reason to move them out of std.

@emberian
Copy link
Member Author

I'm comfortable moving them out of libstd, until we have
libclang-integrated FFI to get the actual sizes for the platform ABI.

On Tue, Feb 25, 2014 at 3:39 PM, György Andrasek
notifications@github.comwrote:

The basic C types are not defined by libc. We need them to do any sort of
FFI including kernel modules and writing libc itself. They're just
typedefs over Rust builtins anyway, I don't see a good reason to move them
out of std.


Reply to this email directly or view it on GitHubhttps://github.com//pull/12526#issuecomment-36055234
.

@alexcrichton
Copy link
Member

It sounds like everyone is in mostly favor of moving libc out of libstd. There's still a lot of decisions to make about things like whether the types are separate from the functions and such, but I believe that this is a good step forward.

I would like to make progress on this extraction. I'd like to take one last look, but other than that r+ with a rebase.

@huonw
Copy link
Member

huonw commented Feb 26, 2014

@liigo, also, I forgot to say, but

To import: don't need VS. use libc::{c_int}

isn't true. In anything but the crate root, use c; is required to bring c::int into scope (or else it's required to write ::c::int instead).

@liigo
Copy link
Contributor

liigo commented Feb 26, 2014

Oh yes, now i know why c_int can't be renamed to c::int. Thank you @cmr @huonw !

@alexcrichton
Copy link
Member

r=me with an update to src/doc/index.md (sorry I forgot that earlier)

@alexcrichton
Copy link
Member

Closing in favor of #13315

bors added a commit that referenced this pull request Apr 6, 2014
Rebasing of #12526 with a very obscure bug fixed on windows.
flip1995 pushed a commit to flip1995/rust that referenced this pull request Apr 4, 2024
Mention `size_hint()` effect in `flat_map_option` lint documentation.

The previous documentation for `flat_map_option` mentioned only readability benefits, but there is also at least one performance benefit: the `size_hint()` upper bound is preserved, whereas `flat_map().size_hint()` is always `(0, None)`.

Program demonstrating this difference:

```rust
fn main() {
    let evens = |i| if i % 2 == 0 { Some(i) } else { None };

    dbg!(
        [1, 2, 3].iter().flat_map(evens).size_hint(),
        [1, 2, 3].iter().filter_map(evens).size_hint(),
    );
}
```

changelog: [`flat_map_option`]: Mention the benefit to `size_hint()`.
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 this pull request may close these issues.

8 participants