Improve error message#681
Merged
gui1117 merged 4 commits intopkhry/duplicate-indexes-const-evalfrom Jan 25, 2025
Merged
Conversation
bkchr
reviewed
Jan 16, 2025
derive/src/utils.rs
Outdated
| while j < len { | ||
| if indices[i] == indices[j] { | ||
| ::core::panic!("Found Variants that have duplicate indexes. Use different indexes for each variant"); | ||
| ::core::panic!(#error_message); |
Member
There was a problem hiding this comment.
And then we can generate an even better error message here :P
Contributor
Author
There was a problem hiding this comment.
I thought it was impossible, but it is. Done in 876a14a
Also are we ok adding a dependency to const_format, if not I can always write the concatenation of strings inlined.
(convertion to array, then calculate the new length in const, then instantiate the new array and copy content + plus special code to format u8).
Co-authored-by: Bastian Köcher <git@kchr.de>
bkchr
reviewed
Jan 17, 2025
Comment on lines
+80
to
+90
| 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); |
Member
There was a problem hiding this comment.
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
Contributor
Author
There was a problem hiding this comment.
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)
bkchr
added a commit
that referenced
this pull request
Jan 25, 2025
* init * Improve error message (#681) * improve error message * Apply suggestions from code review Co-authored-by: Bastian Köcher <git@kchr.de> * finally understanding rust const environment * fmt --------- Co-authored-by: Bastian Köcher <git@kchr.de> --------- Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com> Co-authored-by: Bastian Köcher <git@kchr.de>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
@pkhry I tried to improve the error message, I just print all variant and there index, if there is a formula it will just be the formula expression without the evaluation but I still think it can be enough information for debugging, WDYT?