Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Error on implementation of traits with default config points to the macro instead of expected item #14286

Closed
2 tasks done
gui1117 opened this issue Jun 2, 2023 · 6 comments
Closed
2 tasks done
Labels
I4-annoyance The client behaves within expectations, however this “expected behaviour” itself is at issue.

Comments

@gui1117
Copy link
Contributor

gui1117 commented Jun 2, 2023

Is there an existing issue?

  • I have searched the existing issues

Experiencing problems? Have you tried our Stack Exchange first?

  • This is not a support question.

Description of bug

when implementing a trait with default configs, if there is an error, then it just points to the macro instead of the correct item in the trait implementation>
I believe this can be solved by adding some spans or generating the tokens differently>

Steps to reproduce

1- create a faulty implementation like:

diff --git a/frame/examples/default-config/src/lib.rs b/frame/examples/default-config/src/lib.rs
index adb2469e92f..ce79836dad6 100644
--- a/frame/examples/default-config/src/lib.rs
+++ b/frame/examples/default-config/src/lib.rs
@@ -135,7 +135,7 @@ pub mod tests {
                // these items are defined by frame-system as `no_default`, so we must specify them here.
                // Note that these are types that actually rely on the outer runtime, and can't sensibly
                // have an _independent_ default.
-               type BaseCallFilter = frame_support::traits::Everything;
+               type BaseCallFilter = traits::Everything;
                type RuntimeOrigin = RuntimeOrigin;
                type RuntimeCall = RuntimeCall;
                type RuntimeEvent = RuntimeEvent;

2- compile and see the error resulting

error[E0433]: failed to resolve: use of undeclared crate or module `traits`
   --> frame/examples/default-config/src/lib.rs:133:2
    |
133 |       #[derive_impl(frame_system::config_preludes::TestDefaultConfig as frame_system::DefaultConfig)]
    |       ^----------------------------------------------------------------------------------------------
    |       |
    |  _____in this procedural macro expansion
    | |
134 | |     impl frame_system::Config for Test {
135 | |         // these items are defined by frame-system as `no_default`, so we must specify them here.
136 | |         // Note that these are types that actually rely on the outer runtime, and can't sensibly
...   |
    |
    = note: this error originates in the macro `__import_tokens_attr_derive_impl_inner` which comes from the expansion of the attribute macro `derive_impl` (in Nightly builds, run with -Z macro-backtrace for more info)

For more information about this error, try `rustc --explain E0433`.
error: could not compile `pallet-default-config-example` due to previous error

instead it should point to traits

@bkchr bkchr added the I4-annoyance The client behaves within expectations, however this “expected behaviour” itself is at issue. label Jun 2, 2023
@bkchr
Copy link
Member

bkchr commented Jun 2, 2023

CC @sam0x17

@sam0x17
Copy link
Contributor

sam0x17 commented Jun 5, 2023

I believe this one is also addressed by sam0x17/macro_magic#3 so should have something for this soon

@gui1117
Copy link
Contributor Author

gui1117 commented Jun 6, 2023

@sam0x17 as you wrote here sam0x17/macro_magic#5 (comment)

extra is a string literal used pass through all the macros the tokens of the impl item.
because impl item is converted to string and back to token stream the spans are lost.

you pointed the reason for string convertion is to have extra being in any position.
I think we can also use {} block to capture any token stream.

so the export_tokens generated macro, instead of matching on

($(::)?$($tokens_var:ident)::*, $(::)?$($callback:ident)::*, $extra:expr) => {

it could match on

($(::)?$($tokens_var:ident)::*, $(::)?$($callback:ident)::*, { $($extra:tt)* }) => {

this way it could take others parameters like:

($(::)?$($tokens_var:ident)::*, $(::)?$($callback:ident)::*, { $($extra1:tt)* }, { $($extra2:tt)* }, $extra3:ident) => {

But I don't know if macro_rules call will keep the spans or not.

@gui1117
Copy link
Contributor Author

gui1117 commented Jun 6, 2023

macro_rules keeps the spans, initial implementation is here sam0x17/macro_magic#6

@sam0x17
Copy link
Contributor

sam0x17 commented Jun 6, 2023

This specific improvement will be really good for #13950 since we were having span issues there

@gui1117
Copy link
Contributor Author

gui1117 commented Jun 7, 2023

should be fixed in sam0x17/macro_magic#6
fix will come in substrate with next upgrade of macro magic

@gui1117 gui1117 closed this as completed Jun 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
I4-annoyance The client behaves within expectations, however this “expected behaviour” itself is at issue.
Projects
Status: Done
Development

No branches or pull requests

3 participants