FRAME: pallet tasks: code seems to handle manual definition of tasks, but it actually fail to compile.

[gui1117]

TLDR:

there is some code to handle manual definition of tasks. But the code is wrong AFAICT, not tested, and not documented. Should we fix it or drop it?

Details:

In pallet macro, there is some specific code to retrieve an item like the following, without any explicit attribute:

impl frame_support::traits::Task for Task { .. }
pub enum Task { .. }

The code is here: https://github.com/paritytech/polkadot-sdk/blob/21b3a46b11c15ff3915557201657322f42527a2b/substrate/frame/support/procedural/src/pallet/parse/mod.rs#L296-L352

But writing such code fails to compile, the macro panics. Because the item is removed from input, then expanded here: https://github.com/paritytech/polkadot-sdk/blob/21b3a46b11c15ff3915557201657322f42527a2b/substrate/frame/support/procedural/src/pallet/expand/tasks.rs#L118 Then parsed again but as 2 items: https://github.com/paritytech/polkadot-sdk/blob/21b3a46b11c15ff3915557201657322f42527a2b/substrate/frame/support/procedural/src/pallet/expand/tasks.rs#L241 so it fails to compile with End of input, expect token impl.

This code is not tested, it is also not documented. Should we simply drop it or fix it?

If we want to fix it I suggest a new attribute which just check that it declares an enum with the name Task and sensible generics:

#[pallet::task_manual]
pub enum Task<T> { ...}

[kianenigma]

cc @gupnik

[gupnik]

This code is not tested, it is also not documented. Should we simply drop it or fix it?

Yes, @gui1117 we can go ahead and drop it. This was added by Sam, as the requirements were not completely clear at the time. Later, when the macros were implemented, it was left under the assumption that the manual specification still worked. But, I don't think its worthwhile spending the time to fix it.