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

Weight trait and WeightInfo implementation should be compatible. #7028

Closed
kianenigma opened this issue Sep 4, 2020 · 2 comments
Closed

Weight trait and WeightInfo implementation should be compatible. #7028

kianenigma opened this issue Sep 4, 2020 · 2 comments
Labels
J0-enhancement An additional feature request.

Comments

@kianenigma
Copy link
Contributor

Currently, they are not if we have unused parameters. The trait definition will have the unused parameters while the impl will not.

I don't have a preference, but they should be consistent.

I see that currently it is problematic because in the function generating the trait you don't do the analysis, so you donno if a component is used or not. I would just remove the --weight-trait flag and always write the trait and impl together.
then we can actually have tests that ensure what they produce compiles.

Currently as @shawntabrizi said the trait definition is just a helper and it should be manually tweaked. I think it can and should be fully automatic.

@kianenigma kianenigma added the J0-enhancement An additional feature request. label Sep 4, 2020
@shawntabrizi
Copy link
Member

I have a feeling that us automatically hiding unused parameters is not a good thing. I may have added this feature without really thinking about the fact that it may constantly change the trait, when in fact the trait function should be constant in terms of the variables it needs as defined by the benchmarks.... even if a variable ends up having negligible effect...

@shawntabrizi
Copy link
Member

This was fixed in the PR that transitioned to use templates. #7390

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
J0-enhancement An additional feature request.
Projects
Development

No branches or pull requests

2 participants