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 sp_std #2101

Open
koute opened this issue Oct 31, 2023 · 5 comments
Open

Remove sp_std #2101

koute opened this issue Oct 31, 2023 · 5 comments
Labels
D0-easy Can be fixed primarily by duplicating and adapting code by an intermediate coder. I4-refactor Code needs refactoring.

Comments

@koute
Copy link
Contributor

koute commented Oct 31, 2023

Historically having the sp_std crate might have had benefits, but nowadays I think there isn't much point in keeping it as the only thing it's doing is reexporting a few thing from core/alloc and being confusing to newcomers. We can get rid of it and just use core and alloc directly like everyone else in the Rust ecosystem.

For example:

-use sp_std::{marker::PhantomData, vec, vec::Vec};
+extern crate alloc;
+use core::marker::PhantomData;
+use alloc::{vec, vec::Vec};

This works in both std and no_std contexts.

@koute koute added I4-refactor Code needs refactoring. D0-easy Can be fixed primarily by duplicating and adapting code by an intermediate coder. labels Oct 31, 2023
@koute
Copy link
Contributor Author

koute commented Oct 31, 2023

And just for reference, there are currently 1525 sp_std references in the whole repository. Here's a hacky (and untested!) script that I quickly whipped up in 10 minutes which cuts down 1525 sp_std references into 397, where 328 of the remaining ones are sp_std::prelude::* imports, so they probably can also be easily automated away:

#!/usr/bin/ruby

require 'json'
require 'set'

ALLOC = Set.new [
    "borrow",
    "boxed",
    "collections",
    "format!",
    "rc",
    "vec",
    "vec!"
]

CORE = Set.new [
    "any",
    "cell",
    "cmp",
    "convert",
    "default",
    "fmt",
    "hash",
    "iter",
    "marker",
    "mem",
    "num",
    "ops",
    "result",
    "slice",
    "str",
    "sync",
    "time"
]

def classify mod
    if CORE.include? mod
       "core"
    elsif ALLOC.include? mod
        "alloc"
    else
        "sp_std"
    end
end

paths = `rg -t rust sp_std --json`.each_line.map { |line| (JSON.parse(line)["data"]["path"] || {})["text"] }.select { |p| p != nil }.sort.uniq
paths.each do |path|
    old_data = File.read(path)
    new_data = old_data.gsub(/sp_std::([a-z_\!]+)(::|;|\[)/) do
        "#{classify $1}::#{$1}#{$2}"
    end.gsub(/use sp_std::\{([^\};]+)\};/) do
        per_crate = {}
        $1.strip.split(",").each do |path|
            crate = classify(path.strip.split("::")[0])
            per_crate[crate] ||= []
            per_crate[crate] << path.strip
        end
        out = ""
        per_crate.each do |crate, list|
            if list.length == 1
                out << "use #{crate}::#{list[0]};\n"
            else
                out << "use #{crate}::\{#{list.join(", ")}\};\n"
            end
        end
        out.strip
    end

    next if old_data == new_data
    File.write(path, new_data)
end

@davxy
Copy link
Member

davxy commented Nov 1, 2023

Then we need to do this explicit extern crate alloc in every primitive crate which uses Vec (maybe all)?

I don't have a strong opinion, but maybe the sp-std facade is a bit less verbose and convenient.

EDIT: just for reference. We are not alone https://github.com/arkworks-rs/std

@burdges
Copy link

burdges commented Nov 1, 2023

I've no opinion here but..

Arkworks' ark-std exists largely to provide core::io, which their serialization uses. I'd kinda expect rust gains core::io eventually, but afaik no real progress exists towards this yet. Arguably, scale should be similarly designed around core::io, not its own Input/Output traits, although one difference exists in error propagation, which made sense back when native runtimes existed.

This was referenced Jan 18, 2024
jasl added a commit to Phala-Network/substrate-evm_account_mapping that referenced this issue Jan 21, 2024
@kianenigma
Copy link
Contributor

Seeing people have worked on this I would move forward and assist with merging them, but in general have mixed feeling about whether this is easier for esp. Junior Rust devs.

@athei
Copy link
Member

athei commented Feb 6, 2024

I am in favour of that. One less special thing that people need to wrap their head around when using the substrate code base. To me this crate was always an annoyance.

github-merge-queue bot pushed a commit that referenced this issue Feb 6, 2024
First in a series of PRs that reduces our use of sp-std with a view to
deprecating it.

This is just looking at /substrate and moving some of the references
from `sp-std` to `core`.
These particular changes should be uncontroversial.

Where macros are used `::core` should be used to remove any ambiguity.

part of #2101
bgallois pushed a commit to duniter/duniter-polkadot-sdk that referenced this issue Mar 25, 2024
First in a series of PRs that reduces our use of sp-std with a view to
deprecating it.

This is just looking at /substrate and moving some of the references
from `sp-std` to `core`.
These particular changes should be uncontroversial.

Where macros are used `::core` should be used to remove any ambiguity.

part of paritytech#2101
github-merge-queue bot pushed a commit that referenced this issue Jul 12, 2024
Following PR for #4941
that removes usage of `sp-std` on templates

`sp-std` crate was proposed to deprecate on
#2101

@kianenigma

---------

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
TomaszWaszczyk pushed a commit to TomaszWaszczyk/polkadot-sdk that referenced this issue Jul 13, 2024
Following PR for paritytech#4941
that removes usage of `sp-std` on templates

`sp-std` crate was proposed to deprecate on
paritytech#2101

@kianenigma

---------

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
TarekkMA pushed a commit to moonbeam-foundation/polkadot-sdk that referenced this issue Aug 2, 2024
Following PR for paritytech#4941
that removes usage of `sp-std` on templates

`sp-std` crate was proposed to deprecate on
paritytech#2101

@kianenigma

---------

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
immae pushed a commit to duniter/duniter-v2s that referenced this issue Oct 2, 2024
* remove unused deps

* fix https://git.duniter.org/nodes/rust/duniter-v2s/-/issues/241

* remove sp_std dependency

paritytech/polkadot-sdk#2101 This is the commit message #2:

* update metadata

* clean Cargo.toml

* run benchmarks

* update node

* update crates
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
D0-easy Can be fixed primarily by duplicating and adapting code by an intermediate coder. I4-refactor Code needs refactoring.
Projects
Status: backlog
Development

Successfully merging a pull request may close this issue.

5 participants