-
Notifications
You must be signed in to change notification settings - Fork 428
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove Salt
in code generation during build_create
#842
Remove Salt
in code generation during build_create
#842
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@xgreenx thanks for finding this.
Would you be able to provide us with the code that you used to trigger the build error? I'm curious as to why it took so long to find this. Maybe we need to improve our test coverage here
Updated the test with this use case=) |
Please fix the failing UI test so that the CI runs through again:
|
…tors with different signature causes compilation error on linux platform.
I don't have ideas, why it fails.
All builds fine locally.
|
…ve compilation error in CI.
Seems it is a planned error, but why I don't have it locally?=) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what's up with the Codecov failure, maybe someone else can shed some light
#[ink(storage)] | ||
pub struct Flipper { | ||
value: bool, | ||
} | ||
|
||
impl Flipper { | ||
#[ink(constructor)] | ||
pub fn new(init_value: bool) -> Self { | ||
pub fn new2(init_value: bool) -> Self { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of doing this I would add a new UI test instead. This way we have the bug isolated
The function is marked by the The question is: Why behavior for trait impl block of forwarder logic is not the same as for inherit impl block? Why read-only method can't be forwarded?(here we check that method is I think behavior for read-only functions must be the same as for mutable. But it still will not resolve the compilation issue with constructors=) I agree with idea that the constructor must be not forwarded. |
ink! currently generates code like this in order to provide the user with a compile time warning (upon link time) in case the user specified some incorrect call to a contract. This won't be happening once the new trait codegen has been merged. Until then we have to face this. However, so far it has never been a problem if used properly so I am slightly confused about this error atm. |
Is it a new trait codegen? Yes, I understand why did you add these errors=) My question is related to messages with the Maybe CI doesn't analyze code and doesn't remove the definition of forwarding calls with errors? |
I can't reproduce the failure on my local machine either - even with the same @xgreenx Can you try switching the Docker image used by the CI from |
83797db
to
57f1514
Compare
Added `cc --version` in info section
So the CI failure for The For the time being I would opt for just merging the PR now and if it still fails on |
Co-authored-by: Michael Müller <mich@elmueller.net>
@xgreenx Can you apply my suggestions which revert the debugging? |
@cmichi https://gitlab.parity.io/parity/ink/-/jobs/1013394#L997 - the reason why it fails. |
I used the same command to run tests locally, and tests were success=) |
@xgreenx the failure is |
* Add `Fixed` entry for #842 * Replace `3.0.0-rc3` with `3.0.0-rc4` * Add bump allocator to release notes
build_create method supports only 2 generic parameters. But code here is passing 3 parametrs.
It causes a compilation error.