-
Notifications
You must be signed in to change notification settings - Fork 220
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
Allow using {func} for type to use library functions #1528
Conversation
@@ -74,7 +74,7 @@ fn using() { | |||
let mut runtime = build_solidity( | |||
r##" | |||
contract test { | |||
using ints for uint32; |
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.
This potentially reduces test coverage, or is it tested in another place anyways?
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.
This is already tested above (line 50 to 70)
src/sema/namespace.rs
Outdated
@@ -376,7 +377,7 @@ impl Namespace { | |||
.map(|(id, namespace)| (id, namespace.iter().collect())) | |||
.unwrap(); | |||
|
|||
let s = self.resolve_namespace(namespace, file_no, None, id, diagnostics)?; | |||
let s = self.resolve_namespace(namespace, file_no, contract_no, id, diagnostics)?; |
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.
Not to be pedantic, but just s
is a bad variable name
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.
Not to be pedantic, but just
s
is a bad variable name
I agree
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.
Renamed to symbol
src/sema/namespace.rs
Outdated
@@ -376,7 +377,7 @@ impl Namespace { | |||
.map(|(id, namespace)| (id, namespace.iter().collect())) | |||
.unwrap(); | |||
|
|||
let s = self.resolve_namespace(namespace, file_no, None, id, diagnostics)?; | |||
let s = self.resolve_namespace(namespace, file_no, contract_no, id, diagnostics)?; |
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.
Not to be pedantic, but just
s
is a bad variable name
I agree
if dummy | ||
.cast( | ||
&loc, | ||
&func.params[0].ty, | ||
true, | ||
ns, | ||
&mut Diagnostics::default(), | ||
) | ||
.is_err() |
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.
Is there any test in which we are casting the first argument for the library call?
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.
The polkadot test does this
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.
LGTM
Fixes hyperledger-solang#1525 Signed-off-by: Sean Young <sean@mess.org>
Signed-off-by: Sean Young <sean@mess.org>
Fixes #1525