Skip to content
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

Fix using for global function name collision #1625

Merged
merged 10 commits into from
Mar 10, 2023

Conversation

0xalpharush
Copy link
Contributor

Both SD59x18 and UD60x18 defined a mul function and globally bind the function to the type which wasn't being handled by _analyze_top_level_function. I don't think overloading can be done in a using for but that should be investigated.

Fix #1607

@0xalpharush 0xalpharush changed the base branch from master to dev January 24, 2023 23:39
@PaulRBerg
Copy link
Contributor

Nope, Solidity doesn't support overloading functions in the using for ... global directive. I have actually requested this as a feature on the Solidity forum.

Taking this code snippet as an example:

pragma solidity >=0.8.17;

type UD60x18 is uint256;

using { foo } for UD60x18;

function foo(UD60x18 x) pure returns (UD60x18 result) {
    result = x;
}

function foo(UD60x18 x, UD60x18 y) pure returns (UD60x18 result) {
    x;
    result = y;
}

Will produce this error:

from solidity:
DeclarationError: Identifier not found or not unique.
 --> contracts/Foo.sol:6:9:
  |
6 | using { foo } for UD60x18;

@montyly
Copy link
Member

montyly commented Feb 7, 2023

Can we add a test for this one?

@montyly
Copy link
Member

montyly commented Feb 14, 2023

Can we add the test to the CI? Do we actually need foundry to run this example?

@PaulRBerg
Copy link
Contributor

I'm sorry to chase, but are there any updates here? Is this fix still slated to be included in v0.9.3?

@montyly montyly mentioned this pull request Feb 25, 2023
@0xalpharush 0xalpharush force-pushed the using-for-global-lib-collision branch from c493c28 to 3c0ef74 Compare March 6, 2023 16:52
@0xalpharush
Copy link
Contributor Author

@montyly I added a test. @PaulRBerg This will be included in the upcoming release

@PaulRBerg
Copy link
Contributor

PaulRBerg commented Mar 6, 2023

Amazing news thanks so much ser @0xalpharush!! Our audits are slated to start on March 20, so I'm glad that we will be able to run Slither until then.

@montyly montyly merged commit af39ca9 into dev Mar 10, 2023
@montyly montyly deleted the using-for-global-lib-collision branch March 10, 2023 10:32
@montyly montyly mentioned this pull request Mar 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

type propagation for library calls on type aliases isn't correct
3 participants