Calls to attached functions in a library do not always match equivalent direct calls in terms of being internal/external
- Dominant language
- C++
- Stars
- 25.7k
- Forks
- 6.2k
- Avg merge
- 2d 19h
- Merged PRs (30d)
- 29
Description
### Description
In Solidity we distinguish between internal and external library calls by whether the function is qualified with a library name or not. Generally, a qualified call (`L.f()`) is external while an unqualified one (`f()`) is internal. The only exception are internal functions - in that case `L.f()` is an internal call.
This behavior should be preserved when the function is attached via `using for`. Whether the call is internal or external should depend on whether `using {L.f}` or `using {f}` was used. Currently this is not the case - there is no difference between these two directives.
### How to reproduce
#### Public functions
Attached public functions are currently called externally (which is not allowed in libraries), even if unqualified in `using for`:
```solidity
library L {
using {publicFunction} for uint;
function publicFunction(uint) public pure {}
function test(uint x) public pure {
x.publicFunction(); // Error: Libraries cannot call their own functions externally.
publicFunction(x); // internal call
}
}
```
`x.publicFunction()` here should be an internal call as well and compile without errors.
#### Private functions
Attached private functions are always called internally, even if qualified in `using for`:
```solidity
library L {
using {L.privateFunction} for uint;
function privateFunction(uint) private pure {}
function test(uint x) public pure {
x.privateFunction(); // internal call
L.privateFunction(x); // Error: Member "privateFunction" not found or not visible after argument-dependent lookup in type(library L).
}
}
```
`x.privateFunction()` here should be an external call and produce a compilation error.
#### External functions
External library functions are currently not usable with `using for` (#13765) but after that's fixed (#13855), the following example is going to be inconsistent:
```solidity
library L {
using {externalFunction} for uint;
function externalFunction(uint) external pure {}
function test(uint x) public pure {
x.externalFunction(); // external call
externalFunction(x); // Error: Undeclared identifier. "externalFunction" is not (or not yet) visible at this point.
}
}
```
`x.externalFunction()` here should be an external call and produce a compilation error.
### Specification
- Given `using {f}`, `x.f()` should be equivalent to `f()`.
- Given `using {L.f}`, `x.f()` should be equivalent to `L.f()`.
- Make sure the fix does not break functions qualified in different ways. For example `M.f()`, where `M` is a module and `f()` is a free function should always be an internal call.
- Calling the function in presence of conflicting directives (i.e. both `using {f}` and `using {L.f}` for the same type) should be an error.
- Unless the function is internal. Then it is allowed.
- To avoid introducing non-backwards-compatible changes, the error should be issued at the point of call. If the function is not actually called, it's not an error.
- Note that the conflict may exist only in some scopes, not everywhere. E.g. if there is a file-level `using {L.f}` and one library has `using {L}` while another does not.
- For the external case it would be fine to produce an error directly in the directive because such use currently an error (though this will change when #13765 is fixed).
Contributor guide
Research direction
Start with the public, private, and external Solidity reproducers and the specification, comparing qualified and unqualified attached-function calls. The work is done when the specified internal/external behavior and conflicting-directive errors are consistent without breaking differently qualified functions.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- solidity
- Domain
- compilers
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100