codegen: Add codegen error #841
Conversation
Signed-off-by: Alexandru Vasile <[email protected]>
Signed-off-by: Alexandru Vasile <[email protected]>
Signed-off-by: Alexandru Vasile <[email protected]>
Signed-off-by: Alexandru Vasile <[email protected]>
Signed-off-by: Alexandru Vasile <[email protected]>
Signed-off-by: Alexandru Vasile <[email protected]>
Signed-off-by: Alexandru Vasile <[email protected]>
Signed-off-by: Alexandru Vasile <[email protected]>
Signed-off-by: Alexandru Vasile <[email protected]>
codegen/src/api/storage.rs
Outdated
| let mut storage_fns = Vec::new(); | ||
| for entry in &storage.entries { | ||
| let storage_fn = | ||
| generate_storage_entry_fns(metadata, type_gen, pallet, entry, crate_path)?; | ||
| storage_fns.push(storage_fn); | ||
| } |
There was a problem hiding this comment.
I guess some of these bits could have been done with a .collect::<Result<Vec<_>,_>() style thing (like you did ielsewhere) instead of changing to loops, but I'm happy with either really!
There was a problem hiding this comment.
Have reverted to using collect::<...> everywhere, that sounds pretty cool!
One strange thing about it, the compiler seems to be quite strict in resolving collect::<Result<Vec<_>, _>.
If the Error is not explicitly typed in the closure, then the scope in which the compiler finds Error type is not increased. Such is the case where we use ? and the error could be deduced from the function's return type.
This leads to some places where the CodegenError needs to be typed next to collect.
cli/src/commands/codegen.rs
Outdated
| match runtime_api { | ||
| Ok(runtime_api) => println!("{runtime_api}"), | ||
| Err(e) => { | ||
| // `Span` cannot be sent between threads. |
There was a problem hiding this comment.
I don't understand this comment, is that the reason why you can't propagate the error?
There was a problem hiding this comment.
The CodegenError includes a Span in some variants that can be used to hint at better compile errors.
The color_eyre does impose StdError + Send + Sync on the returned error, to circumvent that I've just printed the error message in red. I'll update the comment :D
| calls | ||
| } else { | ||
| return quote!() | ||
| let Some(call) = &pallet.calls else { |
There was a problem hiding this comment.
this syntax is neat, I probably need to add this to my bank as well.
There was a problem hiding this comment.
Yep its a really cool syntax, tho I can see two small downsides:
- it cannot capture the error, that should be totally fine with Options tho
- rustfmt doesn't seem to work with this syntax, seems that anything in the else { } block is ignored for formatting
There was a problem hiding this comment.
Interesting on the latter point; I guess they will fix it eventually and we'll have to do some reformatting everywhere!
codegen/src/api/mod.rs
Outdated
| @@ -211,35 +271,36 @@ impl RuntimeGenerator { | |||
|
|
|||
| let metadata_hash = get_metadata_per_pallet_hash(&self.metadata, &pallet_names); | |||
|
|
|||
| let modules = pallets_with_mod_names.iter().map(|(pallet, mod_name)| { | |||
| let mut modules = Vec::new(); | |||
There was a problem hiding this comment.
I don't think you need to use a Vec here as as quote is implemented for iterators as well but it's fine either way
Signed-off-by: Alexandru Vasile <[email protected]>
Signed-off-by: Alexandru Vasile <[email protected]>
Signed-off-by: Alexandru Vasile <[email protected]>
|
Have picked the |
| InvalidModule(Span), | ||
| /// Expected named or unnamed fields. | ||
| #[error("Fields should either be all named or all unnamed, make sure you are providing a valid metadata V14: {0}")] | ||
| InvalidFields(String), |
There was a problem hiding this comment.
The error messages look good to me overall :)
Signed-off-by: Alexandru Vasile <[email protected]>
This PR adds a specific
CodegenErrorto ensure that codegen functions can be used outside ofproc_macros.Previously, the codegen was using the
proc-macro-errorwhich panicked upon errors when used from subxt/cli.Closes: #740.