Skip to content

Commit

Permalink
der-derive: avoid type inference when using default (#1442)
Browse files Browse the repository at this point in the history
* Revert "x509-cert: specify concrete types to help the compiler (#1441)"

This reverts commit 7a2d38a.

* der-derive: avoid type inference when using default

This is another take on #1441. This is intended to make sure we can
still continue to use `Default::default` and not break future releases
by mistake.
  • Loading branch information
baloo authored Jul 9, 2024
1 parent 7a2d38a commit 9497bed
Show file tree
Hide file tree
Showing 6 changed files with 72 additions and 13 deletions.
15 changes: 9 additions & 6 deletions der/derive/src/sequence/field.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ impl SequenceField {
!attrs.optional,
"`default`, and `optional` are mutually exclusive"
);
lowerer.apply_default(&self.ident, default);
lowerer.apply_default(&self.ident, default, &self.field_type);
}

lowerer.into_tokens()
Expand Down Expand Up @@ -189,14 +189,17 @@ impl LowerFieldEncoder {
}

/// Handle default value for a type.
fn apply_default(&mut self, ident: &Ident, default: &Path) {
fn apply_default(&mut self, ident: &Ident, default: &Path, field_type: &Type) {
let encoder = &self.encoder;

self.encoder = quote! {
if &self.#ident == &#default() {
None
} else {
Some(#encoder)
{
let default_value: #field_type = #default();
if &self.#ident == &default_value {
None
} else {
Some(#encoder)
}
}
};
}
Expand Down
56 changes: 56 additions & 0 deletions der/tests/derive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -461,3 +461,59 @@ mod sequence {
);
}
}

mod infer_default {
//! When another crate might define a PartialEq for another type, the use of
//! `default="Default::default"` in the der derivation will not provide enough
//! information for `der_derive` crate to figure out.
//!
//! This provides a reproduction for that case. This is intended to fail when we
//! compile tests.
//! ```
//! error[E0282]: type annotations needed
//! --> der/tests/derive.rs:480:26
//! |
//!480 | #[asn1(default = "Default::default")]
//! | ^^^^^^^^^^^^^^^^^^ cannot infer type
//!
//!error[E0283]: type annotations needed
//! --> der/tests/derive.rs:478:14
//! |
//!478 | #[derive(Sequence)]
//! | ^^^^^^^^ cannot infer type
//! |
//!note: multiple `impl`s satisfying `bool: PartialEq<_>` found
//! --> der/tests/derive.rs:472:5
//! |
//!472 | impl PartialEq<BooleanIsh> for bool {
//! | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
//! = note: and another `impl` found in the `core` crate:
//! - impl<host> PartialEq for bool
//! where the constant `host` has type `bool`;
//! = note: required for `&bool` to implement `PartialEq<&_>`
//! = note: this error originates in the derive macro `Sequence` (in Nightly builds, run with -Z macro-backtrace for more info)
//! ```
use der::Sequence;

struct BooleanIsh;

impl PartialEq<BooleanIsh> for bool {
fn eq(&self, _other: &BooleanIsh) -> bool {
unimplemented!("This is only here to mess up the compiler's type inference")
}
}

#[derive(Sequence)]
struct Foo {
#[asn1(default = "Default::default")]
pub use_default_default: bool,

#[asn1(default = "something_true")]
pub use_custom: bool,
}

fn something_true() -> bool {
todo!()
}
}
2 changes: 1 addition & 1 deletion x509-cert/src/ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ pub mod pkix;
pub struct Extension {
pub extn_id: ObjectIdentifier,

#[asn1(default = "bool::default")]
#[asn1(default = "Default::default")]
pub critical: bool,

pub extn_value: OctetString,
Expand Down
2 changes: 1 addition & 1 deletion x509-cert/src/ext/pkix/constraints/basic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use der::Sequence;
#[derive(Clone, Debug, Eq, PartialEq, Sequence)]
#[allow(missing_docs)]
pub struct BasicConstraints {
#[asn1(default = "bool::default")]
#[asn1(default = "Default::default")]
pub ca: bool,
pub path_len_constraint: Option<u8>,
}
Expand Down
2 changes: 1 addition & 1 deletion x509-cert/src/ext/pkix/constraints/name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ pub struct GeneralSubtree {
#[asn1(
context_specific = "0",
tag_mode = "IMPLICIT",
default = "u32::default"
default = "Default::default"
)]
pub minimum: u32,

Expand Down
8 changes: 4 additions & 4 deletions x509-cert/src/ext/pkix/crl/dp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,14 @@ pub struct IssuingDistributionPoint {
#[asn1(
context_specific = "1",
tag_mode = "IMPLICIT",
default = "bool::default"
default = "Default::default"
)]
pub only_contains_user_certs: bool,

#[asn1(
context_specific = "2",
tag_mode = "IMPLICIT",
default = "bool::default"
default = "Default::default"
)]
pub only_contains_ca_certs: bool,

Expand All @@ -48,14 +48,14 @@ pub struct IssuingDistributionPoint {
#[asn1(
context_specific = "4",
tag_mode = "IMPLICIT",
default = "bool::default"
default = "Default::default"
)]
pub indirect_crl: bool,

#[asn1(
context_specific = "5",
tag_mode = "IMPLICIT",
default = "bool::default"
default = "Default::default"
)]
pub only_contains_attribute_certs: bool,
}
Expand Down

0 comments on commit 9497bed

Please sign in to comment.