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.

Sign up for GitHub

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

Improve error message #681

Merged
merged 4 commits into from
Jan 25, 2025
Merged

Improve error message #681

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
61fe7fb
improve error message
gui1117 Jan 16, 2025
5ab6849
Apply suggestions from code review
gui1117 Jan 17, 2025
876a14a
finally understanding rust const environment
gui1117 Jan 17, 2025
3d8507d
fmt
gui1117 Jan 17, 2025
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
27 changes: 27 additions & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ byte-slice-cast = { version = "1.2.2", default-features = false }
generic-array = { version = "0.14.7", optional = true }
arbitrary = { version = "1.4.1", features = ["derive"], optional = true }
impl-trait-for-tuples = "0.2.2"
const_format = { version = "0.2.34" }

[dev-dependencies]
criterion = "0.4.0"
Expand Down
9 changes: 6 additions & 3 deletions derive/src/decode.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,13 @@ pub fn quote(
},
}
});
let recurse_indices =
variants.iter().enumerate().map(|(i, v)| utils::variant_index(v, i));
let recurse_indices = variants
.iter()
.enumerate()
.map(|(i, v)| (v.ident.clone(), utils::variant_index(v, i)));

let const_eval_check = utils::const_eval_check_variant_indexes(recurse_indices);
let const_eval_check =
utils::const_eval_check_variant_indexes(recurse_indices, crate_path);

let read_byte_err_msg =
format!("Could not decode `{type_name}`, failed to read variant byte");
Expand Down
15 changes: 8 additions & 7 deletions derive/src/encode.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ fn impl_encode(data: &Data, type_name: &Ident, crate_path: &syn::Path) -> TokenS
}
};

[hinting, encoding, index]
(hinting, encoding, index, name.clone())
},
Fields::Unnamed(ref fields) => {
let fields = &fields.unnamed;
Expand Down Expand Up @@ -371,7 +371,7 @@ fn impl_encode(data: &Data, type_name: &Ident, crate_path: &syn::Path) -> TokenS
}
};

[hinting, encoding, index]
(hinting, encoding, index, name.clone())
},
Fields::Unit => {
let hinting = quote_spanned! { f.span() =>
Expand All @@ -387,14 +387,14 @@ fn impl_encode(data: &Data, type_name: &Ident, crate_path: &syn::Path) -> TokenS
}
};

[hinting, encoding, index]
(hinting, encoding, index, name.clone())
},
}
});

let recurse_hinting = recurse.clone().map(|[hinting, _, _]| hinting);
let recurse_encoding = recurse.clone().map(|[_, encoding, _]| encoding);
let recurse_indices = recurse.clone().map(|[_, _, index]| index);
let recurse_hinting = recurse.clone().map(|(hinting, _, _, _)| hinting);
let recurse_encoding = recurse.clone().map(|(_, encoding, _, _)| encoding);
let recurse_variant_indices = recurse.clone().map(|(_, _, index, name)| (name, index));

let hinting = quote! {
// The variant index uses 1 byte.
Expand All @@ -404,7 +404,8 @@ fn impl_encode(data: &Data, type_name: &Ident, crate_path: &syn::Path) -> TokenS
}
};

let const_eval_check = const_eval_check_variant_indexes(recurse_indices);
let const_eval_check =
const_eval_check_variant_indexes(recurse_variant_indices, crate_path);

let encoding = quote! {
#const_eval_check
Expand Down
61 changes: 46 additions & 15 deletions derive/src/utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,24 +39,55 @@ where
}

pub fn const_eval_check_variant_indexes(
recurse_indices: impl Iterator<Item = TokenStream>,
recurse_variant_indices: impl Iterator<Item = (syn::Ident, TokenStream)>,
crate_path: &syn::Path,
) -> TokenStream {
let mut recurse_indices = vec![];
for (ident, index) in recurse_variant_indices {
let ident_str = ident.to_string();
recurse_indices.push(quote! { (#index, #ident_str) });
}
let len = recurse_indices.len();

if len == 0 {
return quote! {};
}

quote! {
const _: () = {
let indices = [#( #recurse_indices ,)*];
let len = indices.len();

// Check each pair for uniqueness
let mut i = 0;
while i < len {
let mut j = i + 1;
while j < len {
if indices[i] == indices[j] {
::core::panic!("Found Variants that have duplicate indexes. Use different indexes for each variant");
}
j += 1;
}
i += 1;
const indices: [(usize, &'static str); #len] = [#( #recurse_indices ,)*];

// Returns if there is duplicate, and if there is some the duplicate indexes.
const fn duplicate_info(array: &[(usize, &'static str); #len]) -> (bool, usize, usize) {
let len = array.len();
let mut i = 0;
while i < len {
let mut j = i + 1;
while j < len {
if array[i].0 == array[j].0 {
return (true, i, j);
}
j += 1;
}
i += 1;
}
(false, 0, 0)
}

const DUP_INFO: (bool, usize, usize) = duplicate_info(&indices);

if DUP_INFO.0 {
let msg = #crate_path::__private::concatcp!(
"Found variants that have duplicate indexes. Both `",
indices[DUP_INFO.1].1,
"` and `",
indices[DUP_INFO.2].1,
"` have the index `",
indices[DUP_INFO.1].0,
"`. Use different indexes for each variant."
);

::core::panic!("{}", msg);
Comment on lines +80 to +90
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let msg = #crate_path::__private::concatcp!(
"Found variants that have duplicate indexes. Both `",
indices[DUP_INFO.1].1,
"` and `",
indices[DUP_INFO.2].1,
"` have the index `",
indices[DUP_INFO.1].0,
"`. Use different indexes for each variant."
);
::core::panic!("{}", msg);
::core::panic!("Found variants that have duplicate indexes. Both `{}` and `{}` have index `{}`. Each variant is required to have an unique index.",
indices[DUP_INFO.1].1,
indices[DUP_INFO.2].1,
indices[DUP_INFO.1].0,
);

DOesn't that work? :D

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wish constant evaluation gets a bit more love from rustc developers.

But formatting macro doesn't actually work at compile-time:

		const _: () = {
		    const FOO: &str = "a";
		    const BAR: &str = "b";
			panic!("{}{}", FOO, BAR);
		};

error[E0015]: cannot call non-const formatting macro in constants
 --> src/lib.rs:4:12
  |
4 |             panic!("{}{}", FOO, BAR);
  |                     ^^
  |
  = note: calls in constants are limited to constant functions, tuple structs and tuple variants
  = note: this error originates in the macro `$crate::const_format_args` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info)

}
};
}
Expand Down
8 changes: 8 additions & 0 deletions src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,14 @@ pub mod alloc {
pub use std::{alloc, borrow, boxed, collections, rc, string, sync, vec};
}

/// Private module to reexport items used by derive macros.
// We don't feature gate this module with `derive` to avoid compilation error when
// `parity-scale-codec-derive` is used on its own and this crate doesn't have the feature enabled.
#[doc(hidden)]
pub mod __private {
pub use const_format::concatcp;
}

#[cfg(feature = "bit-vec")]
mod bit_vec;
mod btree_utils;
Expand Down
8 changes: 4 additions & 4 deletions tests/scale_codec_ui/codec_discriminiant_variant_counted_in_default_index.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,30 +2,30 @@ error[E0080]: evaluation of constant value failed
--> tests/scale_codec_ui/codec_discriminiant_variant_counted_in_default_index.rs:1:10
|
1 | #[derive(::parity_scale_codec::Decode, ::parity_scale_codec::Encode)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the evaluated program panicked at 'Found Variants that have duplicate indexes. Use different indexes for each variant', $DIR/tests/scale_codec_ui/codec_discriminiant_variant_counted_in_default_index.rs:1:10
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the evaluated program panicked at 'Found variants that have duplicate indexes. Both `A` and `B` have the index `1`. Use different indexes for each variant.', $DIR/tests/scale_codec_ui/codec_discriminiant_variant_counted_in_default_index.rs:1:10
|
= note: this error originates in the macro `$crate::panic::panic_2021` which comes from the expansion of the macro `::core::panic` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0080]: evaluation of constant value failed
--> tests/scale_codec_ui/codec_discriminiant_variant_counted_in_default_index.rs:1:40
|
1 | #[derive(::parity_scale_codec::Decode, ::parity_scale_codec::Encode)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the evaluated program panicked at 'Found Variants that have duplicate indexes. Use different indexes for each variant', $DIR/tests/scale_codec_ui/codec_discriminiant_variant_counted_in_default_index.rs:1:40
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the evaluated program panicked at 'Found variants that have duplicate indexes. Both `A` and `B` have the index `1`. Use different indexes for each variant.', $DIR/tests/scale_codec_ui/codec_discriminiant_variant_counted_in_default_index.rs:1:40
|
= note: this error originates in the macro `$crate::panic::panic_2021` which comes from the expansion of the macro `::core::panic` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0080]: evaluation of constant value failed
--> tests/scale_codec_ui/codec_discriminiant_variant_counted_in_default_index.rs:8:10
|
8 | #[derive(::parity_scale_codec::Decode, ::parity_scale_codec::Encode)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the evaluated program panicked at 'Found Variants that have duplicate indexes. Use different indexes for each variant', $DIR/tests/scale_codec_ui/codec_discriminiant_variant_counted_in_default_index.rs:8:10
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the evaluated program panicked at 'Found variants that have duplicate indexes. Both `A` and `B` have the index `1`. Use different indexes for each variant.', $DIR/tests/scale_codec_ui/codec_discriminiant_variant_counted_in_default_index.rs:8:10
|
= note: this error originates in the macro `$crate::panic::panic_2021` which comes from the expansion of the macro `::core::panic` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0080]: evaluation of constant value failed
--> tests/scale_codec_ui/codec_discriminiant_variant_counted_in_default_index.rs:8:40
|
8 | #[derive(::parity_scale_codec::Decode, ::parity_scale_codec::Encode)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the evaluated program panicked at 'Found Variants that have duplicate indexes. Use different indexes for each variant', $DIR/tests/scale_codec_ui/codec_discriminiant_variant_counted_in_default_index.rs:8:40
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the evaluated program panicked at 'Found variants that have duplicate indexes. Both `A` and `B` have the index `1`. Use different indexes for each variant.', $DIR/tests/scale_codec_ui/codec_discriminiant_variant_counted_in_default_index.rs:8:40
|
= note: this error originates in the macro `$crate::panic::panic_2021` which comes from the expansion of the macro `::core::panic` (in Nightly builds, run with -Z macro-backtrace for more info)
8 changes: 4 additions & 4 deletions tests/scale_codec_ui/codec_duplicate_index.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,30 +2,30 @@ error[E0080]: evaluation of constant value failed
--> tests/scale_codec_ui/codec_duplicate_index.rs:1:10
|
1 | #[derive(::parity_scale_codec::Decode, ::parity_scale_codec::Encode)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the evaluated program panicked at 'Found Variants that have duplicate indexes. Use different indexes for each variant', $DIR/tests/scale_codec_ui/codec_duplicate_index.rs:1:10
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the evaluated program panicked at 'Found variants that have duplicate indexes. Both `A` and `B` have the index `3`. Use different indexes for each variant.', $DIR/tests/scale_codec_ui/codec_duplicate_index.rs:1:10
|
= note: this error originates in the macro `$crate::panic::panic_2021` which comes from the expansion of the macro `::core::panic` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0080]: evaluation of constant value failed
--> tests/scale_codec_ui/codec_duplicate_index.rs:1:40
|
1 | #[derive(::parity_scale_codec::Decode, ::parity_scale_codec::Encode)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the evaluated program panicked at 'Found Variants that have duplicate indexes. Use different indexes for each variant', $DIR/tests/scale_codec_ui/codec_duplicate_index.rs:1:40
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the evaluated program panicked at 'Found variants that have duplicate indexes. Both `A` and `B` have the index `3`. Use different indexes for each variant.', $DIR/tests/scale_codec_ui/codec_duplicate_index.rs:1:40
|
= note: this error originates in the macro `$crate::panic::panic_2021` which comes from the expansion of the macro `::core::panic` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0080]: evaluation of constant value failed
--> tests/scale_codec_ui/codec_duplicate_index.rs:9:10
|
9 | #[derive(::parity_scale_codec::Decode, ::parity_scale_codec::Encode)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the evaluated program panicked at 'Found Variants that have duplicate indexes. Use different indexes for each variant', $DIR/tests/scale_codec_ui/codec_duplicate_index.rs:9:10
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the evaluated program panicked at 'Found variants that have duplicate indexes. Both `A` and `B` have the index `0`. Use different indexes for each variant.', $DIR/tests/scale_codec_ui/codec_duplicate_index.rs:9:10
|
= note: this error originates in the macro `$crate::panic::panic_2021` which comes from the expansion of the macro `::core::panic` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0080]: evaluation of constant value failed
--> tests/scale_codec_ui/codec_duplicate_index.rs:9:40
|
9 | #[derive(::parity_scale_codec::Decode, ::parity_scale_codec::Encode)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the evaluated program panicked at 'Found Variants that have duplicate indexes. Use different indexes for each variant', $DIR/tests/scale_codec_ui/codec_duplicate_index.rs:9:40
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the evaluated program panicked at 'Found variants that have duplicate indexes. Both `A` and `B` have the index `0`. Use different indexes for each variant.', $DIR/tests/scale_codec_ui/codec_duplicate_index.rs:9:40
|
= note: this error originates in the macro `$crate::panic::panic_2021` which comes from the expansion of the macro `::core::panic` (in Nightly builds, run with -Z macro-backtrace for more info)
Loading