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

feat[lang]: allow module intrinsic interface call #4090

Open
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

charles-cooper
Copy link
Member

@charles-cooper charles-cooper commented Jun 1, 2024

allow module.__interface__ to be used in call position by adding it to the module membership data structure.

additionally, fix a bug where interfaces defined inline could not be exported. this is simultaneously fixed as a related bug because previously, interfaces could come up in export analysis as InterfaceT or TYPE_T depending on their provenance. this commit fixes the bug by making them TYPE_T in both imported and inlined provenance.

refactor:

  • wrap interfaces in TYPE_T
  • streamline an isinstance(t, (VyperType, TYPE_T)) check. TYPE_T now inherits from VyperType, so it doesn't need to be listed separately

What I did

fixes #3943

How I did it

How to verify it

Commit message

Commit message for the final, squashed PR. (Optional, but reviewers will appreciate it! Please see our commit message style guide for what we would ideally like to see in a commit message.)

Description for the changelog

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

allow `module.__interface__` to be used in call position by adding it to
the module membership data structure.

additionally, fix a bug where interfaces defined inline could not be
exported. this is simultaneously fixed as a related bug because
previously, interfaces could come up in export analysis as `InterfaceT`
or `TYPE_T` depending on their provenance. this commit fixes the bug by
making them `TYPE_T` in both imported and inlined provenance.

refactor:
- wrap interfaces in TYPE_T
- streamline an `isinstance(t, (VyperType, TYPE_T))` check.
  `TYPE_T` now inherits from `VyperType`, so it doesn't need to be
  listed separately
there was a test for unimplemented `.vyi` interfaces, this commit adds a
test for unimplemented inline interface for completeness
@charles-cooper charles-cooper marked this pull request as ready for review June 1, 2024 13:45
@cyberthirst
Copy link
Collaborator

we allow for exporting a module with no external functions - i think we should raise in such case:

# main.vy
import lib1

def f(x: Bytes[32*5]) -> uint256:
     k: uint256 = lib1.foo()
     return k


exports: lib1.__interface__

# lib1.vy
@internal
def foo() -> uint256:
    return 1

@cyberthirst
Copy link
Collaborator

import lib1

uses: lib1

@deploy
def __init__():
    lib1.__interface__(self).__init__()


exports: lib1.__interface__

#lib1.vy
k: uint256

@external
def bar():
    pass

@deploy
def __init__():
    self.k = 10
AttributeError: 'NoneType' object has no attribute 'module_t'

@cyberthirst
Copy link
Collaborator

__interface__ can be used in "weird positions:

main.vy
import lib1

#uses: lib1

@deploy
def __init__():
    log lib1.__interface__.Foo(1)
    s: lib1.Structt = lib1.__interface__.Structt(i=1)


exports: lib1.__interface__

#lib1.vy
event Foo:
    i: uint256

struct Structt:
    i: uint256

k: uint256

@external
def bar():
    pass

@deploy
def __init__():
    self.k = 10

@cyberthirst
Copy link
Collaborator

should address #3943, right?

@cyberthirst cyberthirst added this to the v0.4.1 milestone Aug 5, 2024
@cyberthirst
Copy link
Collaborator

with -f annotated_ast i get:

AttributeError: 'FunctionDef' object has no attribute 'node_id'
# main.vy
import lib1

@external
def foo() -> uint256:
    return staticcall lib1.__interface__(self).d()
# lib1.vy
d: public(uint256)

@fubuloubu
Copy link
Member

fubuloubu commented Sep 14, 2024

would it be too much to ask for implicit cast of mod to mod.__interface__? For either v: Mod in argument (convert to Interface to use like v.method(...)) or extcall Mod(v).method(...)

.__interface__ is a little awkward to specify

@charles-cooper
Copy link
Member Author

with -f annotated_ast i get:

AttributeError: 'FunctionDef' object has no attribute 'node_id'
# main.vy
import lib1

@external
def foo() -> uint256:
    return staticcall lib1.__interface__(self).d()
# lib1.vy
d: public(uint256)

on latest (9621397) we get:

vyper.exceptions.UnknownAttribute: tmp/lib1.vy has no member 'd'.

  contract "tmp/main.vy:6", function "foo", line 6:22 
       5 def foo() -> uint256:
  ---> 6     return staticcall lib1.__interface__(self).d()
  -----------------------------^
       7

@charles-cooper
Copy link
Member Author

would it be too much to ask for implicit cast of mod to mod.__interface__? For either v: Mod in argument (convert to Interface to use like v.method(...)) or extcall Mod(v).method(...)

.__interface__ is a little awkward to specify

yes but it's a bit hairy. it will feel ambiguous once we add deploy Mod(...) syntax

@charles-cooper
Copy link
Member Author

we allow for exporting a module with no external functions - i think we should raise in such case:

# main.vy
import lib1

def f(x: Bytes[32*5]) -> uint256:
     k: uint256 = lib1.foo()
     return k


exports: lib1.__interface__

# lib1.vy
@internal
def foo() -> uint256:
    return 1

24ac428

@charles-cooper
Copy link
Member Author

should address #3943, right?

yes. updated the PR description

@charles-cooper
Copy link
Member Author

with -f annotated_ast i get:

AttributeError: 'FunctionDef' object has no attribute 'node_id'
# main.vy
import lib1

@external
def foo() -> uint256:
    return staticcall lib1.__interface__(self).d()
# lib1.vy
d: public(uint256)

should be fixed as of latest (24ac428)

@charles-cooper
Copy link
Member Author

__interface__ can be used in "weird positions:

main.vy
import lib1

#uses: lib1

@deploy
def __init__():
    log lib1.__interface__.Foo(1)
    s: lib1.Structt = lib1.__interface__.Structt(i=1)


exports: lib1.__interface__

#lib1.vy
event Foo:
    i: uint256

struct Structt:
    i: uint256

k: uint256

@external
def bar():
    pass

@deploy
def __init__():
    self.k = 10

i'm not too bothered. it looks a bit weird, but technically Structt is a member of the interface. are you suggesting we should block it and only allow using lib1.__interface__.<external function>?

it was only there for `-f abi` output -- since it is a standards
requirement to have the constructor in the abi output, but it doesn't
semantically make sense in-language for the init function to be the
interface, we add it back in later, at abi generation time.

add a test that `module.__interface__(...).__init__()` is not allowed.
@cyberthirst
Copy link
Collaborator

a bit oos for this PR, but we allow for implements: I where I is an empty .vyi file - which most likely is a bug in the user code

@cyberthirst
Copy link
Collaborator

cyberthirst commented Oct 21, 2024

for code like

# main.vy
import lib1

implements: lib1. __interface__

i get:

vyper.exceptions.UnknownAttribute: tests/custom/lib1.vy has no member '__interface__'.

  contract "tests/custom/test4.vy:4", line 4:12 
       3
  ---> 4 implements: lib1. __interface__
  -------------------^
       5

which is weird given we allow

# main.vy
import lib1

exports: lib1. __interface__

EDIT: i must have made a mistake during branch switching - i was cross-checking the behavior also against master and I can't repro this against this branch

@cyberthirst
Copy link
Collaborator

cyberthirst commented Oct 21, 2024

# main.vy
import ITest2

exports: ITest2

# ITest2.vyi
def foo() -> uint8:
    ...
@view
def foobar() -> uint8:
    ...

yields:

Error compiling: tests/custom/test4.vy
vyper.exceptions.StructureException: not a function or interface: `type(tests/custom/ITest2.vyi)`

  contract "tests/custom/test4.vy:4", line 4:9 
       3
  ---> 4 exports: ITest2
  ----------------^
       5

EDIT: i must have made a mistake during branch switching - i was cross-checking the behavior also against master and I can't repro this against this branch

@cyberthirst
Copy link
Collaborator

i'm not too bothered. it looks a bit weird, but technically Structt is a member of the interface. are you suggesting we should block it and only allow using lib1.__interface__.<external function>?

yes, that was the original intention

however, as i test it, the poc now doesn't work. is this intentional?

@charles-cooper
Copy link
Member Author

for code like

# main.vy
import lib1

implements: lib1. __interface__

i get:

vyper.exceptions.UnknownAttribute: tests/custom/lib1.vy has no member '__interface__'.

  contract "tests/custom/test4.vy:4", line 4:12 
       3
  ---> 4 implements: lib1. __interface__
  -------------------^
       5

which is weird given we allow

# main.vy
import lib1

exports: lib1. __interface__

i can't reproduce this. can you provide the contents you have for lib1.vy?

@charles-cooper
Copy link
Member Author

# main.vy
import ITest2

exports: ITest2

# ITest2.vyi
def foo() -> uint8:
    ...
@view
def foobar() -> uint8:
    ...

yields:

Error compiling: tests/custom/test4.vy
vyper.exceptions.StructureException: not a function or interface: `type(tests/custom/ITest2.vyi)`

  contract "tests/custom/test4.vy:4", line 4:9 
       3
  ---> 4 exports: ITest2
  ----------------^
       5

i can't reproduce this. when i run this i get the error:

Error compiling: tmp/main.vy
vyper.exceptions.StructureException: invalid export

  contract "tmp/main.vy:6", line 6:0 
       5
  ---> 6 exports: ITest2
  -------^
       7

  (hint: exports should look like <module>.<function | interface>)

@charles-cooper
Copy link
Member Author

a bit oos for this PR, but we allow for implements: I where I is an empty .vyi file - which most likely is a bug in the user code

#4322

@cyberthirst
Copy link
Collaborator

you can call __default__ through __interface__

# main.vy
import lib2

@external
def bar():
    extcall lib2.__interface__(self).__default__()

# lib2.vy
@external
def __default__():
    pass

i think you wanna be able to export it but not call it

@cyberthirst
Copy link
Collaborator

exports of the following form are allowed:

# main.vy
import lib2

a: address

exports: lib2.__interface__(self.a).__default__

# lib2.vy
@external
def __default__():
    pass

also add a test for an exception case that was not tested before
@charles-cooper
Copy link
Member Author

exports of the following form are allowed:

# main.vy
import lib2

a: address

exports: lib2.__interface__(self.a).__default__

# lib2.vy
@external
def __default__():
    pass

ac43beb

@charles-cooper
Copy link
Member Author

charles-cooper commented Oct 22, 2024

would it be too much to ask for implicit cast of mod to mod.__interface__? For either v: Mod in argument (convert to Interface to use like v.method(...)) or extcall Mod(v).method(...)
.__interface__ is a little awkward to specify

yes but it's a bit hairy. it will feel ambiguous once we add deploy Mod(...) syntax

been thinking about this more and maybe it's feasible? since we already guard external calls with extcall or staticcall, you can kind of tell what type you get from the call AST context

x: Mod = deploy Mod(args...)
   ^^^          ^^^
   type: interface
                type: module ctor

extcall Mod(addr)
        ^^^
        type: interface

it will be kind of confusing to have Mod be a module or interface depending on context, though.

@charles-cooper
Copy link
Member Author

another fun alternative that is slightly less verbose is:

extcall Mod.__at__(addr).foo()

@@ -19,7 +19,7 @@
)
from vyper.semantics.data_locations import DataLocation
from vyper.semantics.types.base import TYPE_T, VyperType, is_type_t
from vyper.semantics.types.function import ContractFunctionT
from vyper.semantics.types.function import ContractFunctionT, MemberFunctionT

Check failure

Code scanning / CodeQL

Module-level cyclic import Error

'ContractFunctionT' may not be defined if module
vyper.semantics.types.function
is imported before module
vyper.semantics.types.module
, as the
definition
of ContractFunctionT occurs after the cyclic
import
of vyper.semantics.types.module.
'ContractFunctionT' may not be defined if module
vyper.semantics.types.function
is imported before module
vyper.semantics.types.module
, as the
definition
of ContractFunctionT occurs after the cyclic
import
of vyper.semantics.types.module.
@@ -19,7 +19,7 @@
)
from vyper.semantics.data_locations import DataLocation
from vyper.semantics.types.base import TYPE_T, VyperType, is_type_t
from vyper.semantics.types.function import ContractFunctionT
from vyper.semantics.types.function import ContractFunctionT, MemberFunctionT

Check failure

Code scanning / CodeQL

Module-level cyclic import Error

'MemberFunctionT' may not be defined if module
vyper.semantics.types.function
is imported before module
vyper.semantics.types.module
, as the
definition
of MemberFunctionT occurs after the cyclic
import
of vyper.semantics.types.module.
'MemberFunctionT' may not be defined if module
vyper.semantics.types.function
is imported before module
vyper.semantics.types.module
, as the
definition
of MemberFunctionT occurs after the cyclic
import
of vyper.semantics.types.module.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release - must release blocker
Projects
None yet
Development

Successfully merging this pull request may close these issues.

imported types cannot be used in call position
3 participants