Skip to content

minimal-copy deserialize for InternedString #7310

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

Merged
merged 1 commit into from
Sep 3, 2019
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/resolver-tests/src/lib.rs
Original file line number Diff line number Diff line change
@@ -171,7 +171,7 @@ pub fn resolve_with_config_raw(
pkg_id("root"),
deps,
&BTreeMap::<String, Vec<String>>::new(),
None::<String>,
None::<&String>,
false,
)
.unwrap();
2 changes: 0 additions & 2 deletions crates/resolver-tests/tests/resolve.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
use std::env;

use cargo::core::dependency::Kind;
use cargo::core::{enable_nightly_features, Dependency};
use cargo::util::{is_ci, Config};
26 changes: 14 additions & 12 deletions src/cargo/core/dependency.rs
Original file line number Diff line number Diff line change
@@ -101,7 +101,7 @@ pub enum Kind {
}

fn parse_req_with_deprecated(
name: &str,
name: InternedString,
req: &str,
extra: Option<(PackageId, &Config)>,
) -> CargoResult<VersionReq> {
@@ -163,12 +163,13 @@ impl ser::Serialize for Kind {
impl Dependency {
/// Attempt to create a `Dependency` from an entry in the manifest.
pub fn parse(
name: &str,
name: impl Into<InternedString>,
version: Option<&str>,
source_id: SourceId,
inside: PackageId,
config: &Config,
) -> CargoResult<Dependency> {
let name = name.into();
let arg = Some((inside, config));
let (specified_req, version_req) = match version {
Some(v) => (true, parse_req_with_deprecated(name, v, arg)?),
@@ -187,10 +188,11 @@ impl Dependency {

/// Attempt to create a `Dependency` from an entry in the manifest.
pub fn parse_no_deprecated(
name: &str,
name: impl Into<InternedString>,
version: Option<&str>,
source_id: SourceId,
) -> CargoResult<Dependency> {
let name = name.into();
let (specified_req, version_req) = match version {
Some(v) => (true, parse_req_with_deprecated(name, v, None)?),
None => (false, VersionReq::any()),
@@ -206,11 +208,11 @@ impl Dependency {
Ok(ret)
}

pub fn new_override(name: &str, source_id: SourceId) -> Dependency {
pub fn new_override(name: InternedString, source_id: SourceId) -> Dependency {
assert!(!name.is_empty());
Dependency {
inner: Rc::new(Inner {
name: InternedString::new(name),
name,
source_id,
registry_id: None,
req: VersionReq::any(),
@@ -338,12 +340,9 @@ impl Dependency {
/// Sets the list of features requested for the package.
pub fn set_features(
&mut self,
features: impl IntoIterator<Item = impl AsRef<str>>,
features: impl IntoIterator<Item = impl Into<InternedString>>,
) -> &mut Dependency {
Rc::make_mut(&mut self.inner).features = features
.into_iter()
.map(|s| InternedString::new(s.as_ref()))
.collect();
Rc::make_mut(&mut self.inner).features = features.into_iter().map(|s| s.into()).collect();
self
}

@@ -376,8 +375,11 @@ impl Dependency {
self
}

pub fn set_explicit_name_in_toml(&mut self, name: &str) -> &mut Dependency {
Rc::make_mut(&mut self.inner).explicit_name_in_toml = Some(InternedString::new(name));
pub fn set_explicit_name_in_toml(
&mut self,
name: impl Into<InternedString>,
) -> &mut Dependency {
Rc::make_mut(&mut self.inner).explicit_name_in_toml = Some(name.into());
self
}

44 changes: 44 additions & 0 deletions src/cargo/core/interning.rs
Original file line number Diff line number Diff line change
@@ -23,6 +23,18 @@ pub struct InternedString {
inner: &'static str,
}

impl<'a> From<&'a str> for InternedString {
fn from(item: &'a str) -> Self {
InternedString::new(item)
}
}

impl<'a> From<&'a String> for InternedString {
fn from(item: &'a String) -> Self {
InternedString::new(item)
}
}

impl PartialEq for InternedString {
fn eq(&self, other: &InternedString) -> bool {
ptr::eq(self.as_str(), other.as_str())
@@ -56,6 +68,12 @@ impl Deref for InternedString {
}
}

impl AsRef<str> for InternedString {
fn as_ref(&self) -> &str {
self.as_str()
}
}

impl Hash for InternedString {
// N.B., we can't implement this as `identity(self).hash(state)`,
// because we use this for on-disk fingerprints and so need
@@ -105,3 +123,29 @@ impl Serialize for InternedString {
serializer.serialize_str(self.inner)
}
}

struct InternedStringVisitor;

impl<'de> serde::Deserialize<'de> for InternedString {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: serde::Deserializer<'de>,
{
deserializer.deserialize_str(InternedStringVisitor)
}
}

impl<'de> serde::de::Visitor<'de> for InternedStringVisitor {
type Value = InternedString;

fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
formatter.write_str("an String like thing")
}

fn visit_str<E>(self, v: &str) -> Result<Self::Value, E>
where
E: serde::de::Error,
{
Ok(InternedString::new(v))
}
}
8 changes: 6 additions & 2 deletions src/cargo/core/package_id.rs
Original file line number Diff line number Diff line change
@@ -113,9 +113,13 @@ impl Hash for PackageId {
}

impl PackageId {
pub fn new<T: ToSemver>(name: &str, version: T, sid: SourceId) -> CargoResult<PackageId> {
pub fn new<T: ToSemver>(
name: impl Into<InternedString>,
version: T,
sid: SourceId,
) -> CargoResult<PackageId> {
let v = version.to_semver()?;
Ok(PackageId::pure(InternedString::new(name), v, sid))
Ok(PackageId::pure(name.into(), v, sid))
}

pub fn pure(name: InternedString, version: semver::Version, source_id: SourceId) -> PackageId {
2 changes: 1 addition & 1 deletion src/cargo/core/registry.rs
Original file line number Diff line number Diff line change
@@ -366,7 +366,7 @@ impl<'cfg> PackageRegistry<'cfg> {
fn query_overrides(&mut self, dep: &Dependency) -> CargoResult<Option<Summary>> {
for &s in self.overrides.iter() {
let src = self.sources.get_mut(s).unwrap();
let dep = Dependency::new_override(&*dep.package_name(), s);
let dep = Dependency::new_override(dep.package_name(), s);
let mut results = src.query_vec(&dep)?;
if !results.is_empty() {
return Ok(Some(results.remove(0)));
2 changes: 1 addition & 1 deletion src/cargo/core/resolver/dep_cache.rs
Original file line number Diff line number Diff line change
@@ -433,7 +433,7 @@ impl Requirements<'_> {
for fv in self
.summary
.features()
.get(feat.as_str())
.get(&feat)
.expect("must be a valid feature")
{
match *fv {
4 changes: 2 additions & 2 deletions src/cargo/core/summary.rs
Original file line number Diff line number Diff line change
@@ -37,7 +37,7 @@ impl Summary {
pkg_id: PackageId,
dependencies: Vec<Dependency>,
features: &BTreeMap<K, Vec<impl AsRef<str>>>,
links: Option<impl AsRef<str>>,
links: Option<impl Into<InternedString>>,
namespaced_features: bool,
) -> CargoResult<Summary>
where
@@ -66,7 +66,7 @@ impl Summary {
dependencies,
features: feature_map,
checksum: None,
links: links.map(|l| InternedString::new(l.as_ref())),
links: links.map(|l| l.into()),
namespaced_features,
}),
})
2 changes: 1 addition & 1 deletion src/cargo/sources/registry/index.rs
Original file line number Diff line number Diff line change
@@ -716,7 +716,7 @@ impl IndexSummary {
links,
} = serde_json::from_slice(line)?;
log::trace!("json parsed registry {}/{}", name, vers);
let pkgid = PackageId::new(&name, &vers, source_id)?;
let pkgid = PackageId::new(name, &vers, source_id)?;
let deps = deps
.into_iter()
.map(|dep| dep.into_dep(source_id))
21 changes: 11 additions & 10 deletions src/cargo/sources/registry/mod.rs
Original file line number Diff line number Diff line change
@@ -219,17 +219,18 @@ pub struct RegistryConfig {

#[derive(Deserialize)]
pub struct RegistryPackage<'a> {
name: Cow<'a, str>,
name: InternedString,
vers: Version,
#[serde(borrow)]
deps: Vec<RegistryDependency<'a>>,
features: BTreeMap<Cow<'a, str>, Vec<Cow<'a, str>>>,
features: BTreeMap<InternedString, Vec<InternedString>>,
cksum: String,
yanked: Option<bool>,
links: Option<Cow<'a, str>>,
links: Option<InternedString>,
}

#[test]
fn escaped_cher_in_json() {
fn escaped_char_in_json() {
let _: RegistryPackage<'_> = serde_json::from_str(
r#"{"name":"a","vers":"0.0.1","deps":[],"cksum":"bae3","features":{}}"#,
)
@@ -275,15 +276,16 @@ enum Field {

#[derive(Deserialize)]
struct RegistryDependency<'a> {
name: Cow<'a, str>,
name: InternedString,
#[serde(borrow)]
req: Cow<'a, str>,
features: Vec<Cow<'a, str>>,
features: Vec<InternedString>,
optional: bool,
default_features: bool,
target: Option<Cow<'a, str>>,
kind: Option<Cow<'a, str>>,
registry: Option<Cow<'a, str>>,
package: Option<Cow<'a, str>>,
package: Option<InternedString>,
public: Option<bool>,
}

@@ -309,10 +311,9 @@ impl<'a> RegistryDependency<'a> {
default
};

let mut dep =
Dependency::parse_no_deprecated(package.as_ref().unwrap_or(&name), Some(&req), id)?;
let mut dep = Dependency::parse_no_deprecated(package.unwrap_or(name), Some(&req), id)?;
if package.is_some() {
dep.set_explicit_name_in_toml(&name);
dep.set_explicit_name_in_toml(name);
}
let kind = match kind.as_ref().map(|s| &s[..]).unwrap_or("") {
"dev" => Kind::Development,
6 changes: 3 additions & 3 deletions src/cargo/util/toml/mod.rs
Original file line number Diff line number Diff line change
@@ -16,7 +16,7 @@ use url::Url;
use crate::core::dependency::{Kind, Platform};
use crate::core::manifest::{LibKind, ManifestMetadata, TargetSourcePath, Warnings};
use crate::core::profiles::Profiles;
use crate::core::{Dependency, Manifest, PackageId, Summary, Target};
use crate::core::{Dependency, InternedString, Manifest, PackageId, Summary, Target};
use crate::core::{Edition, EitherManifest, Feature, Features, VirtualManifest};
use crate::core::{GitReference, PackageIdSpec, SourceId, WorkspaceConfig, WorkspaceRootConfig};
use crate::sources::{CRATES_IO_INDEX, CRATES_IO_REGISTRY};
@@ -650,7 +650,7 @@ impl<'de> de::Deserialize<'de> for VecStringOrBool {
#[derive(Deserialize, Serialize, Clone, Debug)]
pub struct TomlProject {
edition: Option<String>,
name: String,
name: InternedString,
version: semver::Version,
authors: Option<Vec<String>>,
build: Option<StringOrBool>,
@@ -697,7 +697,7 @@ pub struct TomlWorkspace {

impl TomlProject {
pub fn to_package_id(&self, source_id: SourceId) -> CargoResult<PackageId> {
PackageId::new(&self.name, self.version.clone(), source_id)
PackageId::new(self.name, self.version.clone(), source_id)
}
}