Inconsistent rules for .address and .selector

[cameel]

Description

Rules for deciding whether the code using .address or .selector on public functions is valid or not seem pretty random. It sometimes works without this and sometimes doesn't. Behavior is different between for .selector and for .address. It also changes depending on whether the value is actually used or not.

Example

contract B {
    function g() public {}
}

contract C is B {
    function h() public  {
        address a1 = g.address;     // Error: Member "address" not found or not visible after argument-dependent lookup in function ().
        address a2 = this.g.address;
        address a3 = h.address;     // Error: Member "address" not found or not visible after argument-dependent lookup in function ().
        address a4 = this.h.address;

        g.address;                  // Error: Expected identifier but got 'address'
        this.g.address;             // Error: Expected identifier but got 'address'
        h.address;                  // Error: Expected identifier but got 'address'
        this.h.address;             // Error: Expected identifier but got 'address'

        bytes4 s1 = g.selector;
        bytes4 s2 = this.g.selector;
        bytes4 s3 = h.selector;     // Error: Member "selector" not found or not visible after argument-dependent lookup in function ().
        bytes4 s4 = this.h.selector;

        g.selector;
        this.g.selector;
        h.selector;                 // Error: Member "selector" not found or not visible after argument-dependent lookup in function ().
        this.h.selector;
    }
}

Environment

• Compiler version: 0.8.1
• Operating system: Arch Linux

[chriseth]

The Expected identifier errors are from the parser - I guess it has a missing "use keyword for identifier" workaround, which maybe is also just fine.

[ekpyron]

It's not exactly random - address and selector are allowed, whenever the function is referred to as external function, i.e. using this.. The expression-statement version of those could arguably be changed, but then again those don't have an effect anyways, so maybe it's fine, yeah.

[ekpyron]

Ah, no, now I see what you mean... yeah maybe we should disallow selector on functions used in internal context...

[ekpyron]

However, selector just depends on the arguments and can be used without there actually being an actual external function pointer, so we can fetch a selector as long as it's a function that might be used externally. On the other hand we only have an address, if we actually have it. So also the difference between the two does make sense...

[cameel]

Well, I know that it's not really random but there are at least 2 or 3 things (either bugs or design decisions) that make the rules complex so it may just as well be random from user's perspective :)

At least the usage with this is consistently allowed if we ignore the Expected identifier case.

The Expected identifier errors are from the parser - I guess it has a missing "use keyword for identifier" workaround, which maybe is also just fine.

Unless it happens in other expressions where it would not be a no-op. The fact that this.g.address; on its own is illegal was certainly baffling when I ran into it while trying things out.

However, selector just depends on the arguments and can be used without there actually being an actual external function pointer, so we can fetch a selector as long as it's a function that might be used externally.

I think it's fine to be able to use it on internal stuff that can be called externally but it's inconsistent that it works for g and not for h while both this.g and this.h work. Also, the error message does not explain why selector is missing which adds to the confusion.

[ekpyron]

A bit weird to now double-discuss this with #10891 :-). Short version of what I said there: the fact that g.selector is valid is a side-effect of B.g.selector being valid and us assigning both of them the same type internally. The latter works and should work (that was an explicit feature request), even if used from a contract that does not inherit from B. So yeah, it might be nice to disallow g.selector there, but it'll probably involve some tinkering, since there are cases that currently have the same types that should be allowed.

[cameel]

Any why not just allow h.selector instead? As a bonus, it would not even be a breaking change.

[chriseth]

If we allow h.selector, we have to check whether the function's parameters are valid outside of the contract which turns this into an even more complicated rule.

[ekpyron]

Hm, maybe :-). My guess without looking into it would be that internally the compiler treats h just as having FunctionType::Kind::Internal, i.e. you no longer know from its type alone that it's actually public... and we can't just allow it for all internal functions, since they may use internal types, s.t. there is no selector... but yeah - both disallowing g.selector and allowing h.selector sounds good to me, but both may require some reworking of the internals...

[ekpyron]

If we allow h.selector, we have to check whether the function's parameters are valid outside of the contract which turns this into an even more complicated rule.

As long as h is public, it should be fine, shouldn't it? That would be a consistent rule - as long as we refer to a public function, we can fetch a selector... but still probably messy internally, since h gets an internal function type kind... so the general problem is that we collapse too many function types in the AST/compiler type system - maybe we should have spent more time on that in #8137, but we settled for a "close enough" back then...

[chriseth]

I think these are two unrelated issues:

• this.g.address; should parse. In general, .address should be valid everywhere in the parser.
• g.selector; should not work.

[chriseth]

Acutally there are quite complicated rules about .selector and maybe it's best to just leave them as they are...