refactor: merge front-end and codegen type systems #3182

charles-cooper · 2022-12-08T03:07:46Z

What I did

For historical reasons, vyper models its type system in two places, in vyper/semantics/types and vyper/codegen/types. this merges the two systems (w/ plenty of pair programming help from @z80dev)

also:

rename ModuleNodeVisitor to ModuleAnalyzer
move the VariableRecord class to vyper/codegen/context.py
simplify GlobalContext.parse_type and a lot of associated machinery

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

lgtm-com · 2022-12-08T03:54:33Z

This pull request introduces 4 alerts and fixes 5 when merging 8d817b9 into e60b021 - view on LGTM.com

new alerts:

3 for Unused import
1 for Missing call to `__init__` during object initialization

fixed alerts:

4 for `__eq__` not overridden when adding attributes
1 for 'import *' may pollute namespace

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine ⚙️ that powers LGTM.com. For more information, please check out our post on the GitHub blog.

lgtm-com · 2022-12-08T04:43:26Z

This pull request introduces 6 alerts and fixes 5 when merging d6ed7d0 into e60b021 - view on LGTM.com

new alerts:

6 for Unused import

fixed alerts:

4 for `__eq__` not overridden when adding attributes
1 for 'import *' may pollute namespace

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine ⚙️ that powers LGTM.com. For more information, please check out our post on the GitHub blog.

lgtm-com · 2022-12-08T21:43:33Z

This pull request introduces 6 alerts and fixes 5 when merging 5a224a0 into 44bb434 - view on LGTM.com

new alerts:

6 for Unused import

fixed alerts:

4 for `__eq__` not overridden when adding attributes
1 for 'import *' may pollute namespace

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine ⚙️ that powers LGTM.com. For more information, please check out our post on the GitHub blog.

the is_literal_bytes32_assign was dead code from the days when one could assign a literal bytestring of length 32 (i.e. b"111...11") to a variable with type bytes32.

lgtm-com · 2022-12-08T22:04:59Z

This pull request introduces 7 alerts and fixes 5 when merging b349a36 into 44bb434 - view on LGTM.com

new alerts:

7 for Unused import

fixed alerts:

4 for `__eq__` not overridden when adding attributes
1 for 'import *' may pollute namespace

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine ⚙️ that powers LGTM.com. For more information, please check out our post on the GitHub blog.

This reverts commit 0efb1db.

This reverts commit 9585561.

charles-cooper · 2022-12-30T22:35:12Z

@fubuloubu @skellet0r this is ready for review. @z80dev to do a final look over as well.

tserg

Amazing work sir, this greatly simplifies the type system and all the way through to testing.

Also, I really like the signeds, unsigneds and all methods of the primitives.

vyper/codegen/context.py

vyper/codegen/expr.py

skellet0r

You really get an appreciation for the refactoring in expr.py and functions.py. Not seeing typ=BaseType(...) or typ="...", really is a glory to behold. That and, type instance checks throughout the codebase becoming much clearer and concise really help readability.

Damn fine refactoring work!

fubuloubu

Pretty impressive refactor. Some small suggestions for improvement, but overall looks good to me

vyper/builtins/_convert.py

fubuloubu · 2023-01-03T03:30:06Z

vyper/codegen/core.py

+def is_bytes_m_type(typ):
+    return isinstance(typ, BytesM_T)
+
+
+def is_numeric_type(typ):
+    return isinstance(typ, (IntegerT, DecimalT))
+
+
+def is_integer_type(typ):
+    return isinstance(typ, IntegerT)
+
+
+def is_decimal_type(typ):
+    return isinstance(typ, DecimalT)
+
+
+def is_enum_type(typ):
+    return isinstance(typ, EnumT)


Would these make sense to inline for readability reasons?

tried it both ways, but some sections of code just read more cleanly with the helper functions.

fubuloubu · 2023-01-03T03:32:45Z

vyper/codegen/core.py

+def is_tuple_like(typ):
+    # A lot of code paths treat tuples and structs similarly
+    # so we have a convenience function to detect it
+    ret = isinstance(typ, (TupleT, StructT))
+    assert ret == hasattr(typ, "tuple_items")
+    return ret
+
+
+def is_array_like(typ):
+    # For convenience static and dynamic arrays share some code paths
+    ret = isinstance(typ, (DArrayT, SArrayT))
+    assert ret == typ._is_array_type
+    return ret


Are these codepaths just here to double-check that there are no other codepaths that produce a tuple or dynamic array?

Also could inline these too if these were just to double-check

the codepath is to double check that typ._is_array_type means the same as membership in DArrayT, SArrayT. it's a sanity check in case any future types break that assumption.

github-advanced-security

CodeQL found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.

z80dev and others added 7 commits December 6, 2022 14:32

rename ModuleNodeVisitor to ModuleAnalyzer

e2558fe

add a sanity check

38837b9

move VariableRecord to context.py

4ed268e

fix TupleT

b4a263a

WIP merge VyperType and NodeType

8d817b9

Merge remote-tracking branch 'origin/master' into refactor/typechecker

563dd5d

WIP remove usages of old types

0bc1990

charles-cooper added 2 commits December 7, 2022 20:10

wip more type system merging, quash bugs

386de80

fix some lint

d6ed7d0

charles-cooper added 4 commits December 8, 2022 13:22

add a comment

3114fa5

clarify a check

796422d

remove dead code

310dff7

add a sanity check

5a224a0

clean up parse_AnnAssign

b349a36

the is_literal_bytes32_assign was dead code from the days when one could assign a literal bytestring of length 32 (i.e. b"111...11") to a variable with type bytes32.

charles-cooper added 12 commits December 8, 2022 15:14

wip - remove GlobalContext.parse_type

0efb1db

wip, fix some small bugs

d1de8fd

fix lint

19fb2f3

fix an import

0c1244d

Revert "wip - remove GlobalContext.parse_type"

0b860e6

This reverts commit 0efb1db.

fix lint again

f9e5a74

fix GlobalContext.parse_type

7b2020a

remove bad use of GlobalContext, and fix extract_sigs

6f85b30

fix memorymock

15dfdf2

Merge branch 'master' into refactor/typechecker

f236e36

fix some small bugs

f4a4669

fix struct codegen

45cebf4

charles-cooper added 17 commits December 28, 2022 17:03

sanity check

9073231

remove dead comment

80fb7d6

add a sanity check

35db4e9

stray comment

ff46d9a

rename for clarity

a40dcc8

some small fixes

dd0ef75

fix to_address when input is uint160

4d8c759

fix tuple ctor

9585561

Revert "fix tuple ctor"

ad75eb4

This reverts commit 9585561.

add a sanity check

5aec183

more sanity check

046daa5

fix a branch

71d1cc5

remove dead code

0a13b89

use allocate_mem instead of expand_mem

82792bc

fix instantiation of enum type

788dfad

add test for enum dereference

cd0b179

fix a comment

d694315

charles-cooper mentioned this pull request Jan 1, 2023

refactor: simplify GlobalContext #3209

Merged

tserg reviewed Jan 2, 2023

View reviewed changes

vyper/codegen/context.py Outdated Show resolved Hide resolved

vyper/codegen/expr.py Show resolved Hide resolved

skellet0r approved these changes Jan 2, 2023

View reviewed changes

charles-cooper added 3 commits January 2, 2023 08:47

remove outdated comment

6ab9c29

clean up parse_BinOp

d41ab60

fix lint

682a770

fubuloubu approved these changes Jan 3, 2023

View reviewed changes

Merge branch 'master' into refactor/typechecker

489348f

github-advanced-security bot found potential problems Jan 4, 2023

View reviewed changes

charles-cooper merged commit 3abe588 into vyperlang:master Jan 4, 2023

charles-cooper deleted the refactor/typechecker branch January 4, 2023 21:34

tserg mentioned this pull request Feb 9, 2023

chore: update vyper imports vyperlang/titanoboa#46

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: merge front-end and codegen type systems #3182

refactor: merge front-end and codegen type systems #3182

charles-cooper commented Dec 8, 2022 •

edited

Loading

lgtm-com bot commented Dec 8, 2022

lgtm-com bot commented Dec 8, 2022

lgtm-com bot commented Dec 8, 2022

lgtm-com bot commented Dec 8, 2022

charles-cooper commented Dec 30, 2022

tserg left a comment

skellet0r left a comment

fubuloubu left a comment

fubuloubu Jan 3, 2023

charles-cooper Jan 3, 2023

fubuloubu Jan 3, 2023

fubuloubu Jan 3, 2023

charles-cooper Jan 3, 2023

github-advanced-security bot left a comment

refactor: merge front-end and codegen type systems #3182

refactor: merge front-end and codegen type systems #3182

Conversation

charles-cooper commented Dec 8, 2022 • edited Loading

What I did

How I did it

How to verify it

Commit message

Description for the changelog

Cute Animal Picture

lgtm-com bot commented Dec 8, 2022

lgtm-com bot commented Dec 8, 2022

lgtm-com bot commented Dec 8, 2022

lgtm-com bot commented Dec 8, 2022

charles-cooper commented Dec 30, 2022

tserg left a comment

Choose a reason for hiding this comment

skellet0r left a comment

Choose a reason for hiding this comment

fubuloubu left a comment

Choose a reason for hiding this comment

fubuloubu Jan 3, 2023

Choose a reason for hiding this comment

charles-cooper Jan 3, 2023

Choose a reason for hiding this comment

fubuloubu Jan 3, 2023

Choose a reason for hiding this comment

fubuloubu Jan 3, 2023

Choose a reason for hiding this comment

charles-cooper Jan 3, 2023

Choose a reason for hiding this comment

github-advanced-security bot left a comment

Choose a reason for hiding this comment

charles-cooper commented Dec 8, 2022 •

edited

Loading