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

[Rust] Minimize unstable feature usage #21208

Closed
asfimport opened this issue Feb 26, 2019 · 9 comments
Closed

[Rust] Minimize unstable feature usage #21208

asfimport opened this issue Feb 26, 2019 · 9 comments

Comments

@asfimport
Copy link

asfimport commented Feb 26, 2019

The Rust implementation currently uses quite a few nightly features. This is unfortunately a hard blocker on using these crates for many users.

Here's the list of currently use nightly features:

  • type_ascription: Unused, can be trivially removed.

  • rustc_private: Unused, can be trivially removed.

  • box_syntax: Indefinitely far from stabilization, trivially replaceable with Box::new.

  • box_patterns: Indefinitely far from stabilization, replaceable with some minor restructuring of a couple of matches.

  • serde's alloc feature: Unused, can be trivially removed.

  • try_from: Scheduled for stabilization in Rust 1.35.

  • specialization: Actively being worked on - maybe ~1 year timeframe?

  • packed_simd: Actively being worked on - maybe ~1 year timeframe?

    The first set of features are easy enough to get rid of - I'll make a PR to do that (https://github.com/sfackler/arrow/tree/more-stable). I'm a bit less sure of what to do with specialization and packed_simd, though.

Reporter: Steven Fackler / @sfackler
Assignee: Steven Fackler / @sfackler

Related issues:

PRs and other links:

Note: This issue was originally created as ARROW-4678. Please see the migration documentation for further details.

@asfimport
Copy link
Author

Chao Sun / @sunchao:
I don't think we can remove specialization right now. It's been used in many places. 

@asfimport
Copy link
Author

Steven Fackler / @sfackler:
Right, yeah.

 

In the arrow crate itself, it's only used for the BufferBuilderTrait. One possibility there is to have a Cargo feature that has the specialized, blanket impl, and separate impls for each primitive type without the feature. You can't work with it generically over arbitrary primitive types then, but it's still better than nothing.

There's more usage in parquet, but I haven't dug too deeply into it.

@asfimport
Copy link
Author

Paddy Horan / @paddyhoran:
Yeah specialization is the big one.  We did have an elaborate macro approach that just started to break down once we adopted a bit packed Boolean array representation.  Generic code became impossible.

Once we took that step and it was agreed that we were building against nightly we adopted packed_simd.

We should minimize the nightly features though and try to get back on stable when possible.

@asfimport
Copy link
Author

Neville Dipale / @nevi-me:
I'm the one who added box_syntax and box_patterns. If there's a way of matching datatypes in a ListArray(Box) that can be done without the box pattern, we can remove the 2 unstable features.

EDIT: looks like try_from is getting stabilised in 1.34
 

error: the feature `try_from` has been stable since 1.34.0 and no longer requires an attribute to enable
  --> arrow/src/lib.rs:27:12
   |
27 | #![feature(try_from)]
   |            ^^^^^^^^
   |
   = note: `-D stable-features` implied by `-D warnings`

 

@asfimport
Copy link
Author

Chao Sun / @sunchao:
Issue resolved by pull request 3764
#3764

@asfimport
Copy link
Author

Chao Sun / @sunchao:
@sfackler could you try to assign this to yourself? since the issue was resolved by you.

@asfimport
Copy link
Author

Steven Fackler / @sfackler:
It looks like I'm not allowed to.

@asfimport
Copy link
Author

Chao Sun / @sunchao:
Seems I can't assign that to you as well - you name doesn't show up in the available assignees.

@wesm, @xhochy: do you know how to assign this JIRA to @sfackler? do we need to add him as a contributor first? Thanks.

@asfimport
Copy link
Author

Uwe Korn / @xhochy:
@sunchao yes an admin needs to add him first. Taking care of that now.

@asfimport asfimport added this to the 0.13.0 milestone Jan 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant