Skip to content

refactor: front-end type system#2974

Merged
charles-cooper merged 178 commits intovyperlang:masterfrom
charles-cooper:chore/refactor_type_system
Nov 27, 2022
Merged

refactor: front-end type system#2974
charles-cooper merged 178 commits intovyperlang:masterfrom
charles-cooper:chore/refactor_type_system

Conversation

@charles-cooper
Copy link
Copy Markdown
Member

@charles-cooper charles-cooper commented Jul 18, 2022 •
edited
Loading

What I did

refactor type system. redo the type hierarchy - instead of distinction between "Primitives" and "Definitions", have single type system + ExprInfo

  • simplify type hierarchy
  • merge old and new type systems will do this in a follow up PR
  • refactor builtins dir structure. merge builtin_functions/ and builtin_interfaces/ to builtins/.

How I did it

rethinking of the approach; don't need two classes for each type, factor into VyperType + ExprInfo + VarInfo. handling of types in the regular namespace is generally by dispatching into special constructor (for structs and interfaces) or attributes (for enums) mode.

How to verify it

Commit message

This commit refactors the front-end type system.

Currently, there are essentially two classes types for each vyper type.
"Primitives" represented raw types, and "Definitions" represent
the instantiation of types attached to each expression in a program.
"Definitions" additionally carry around annotation information related
to the expressions they tag, such as mutability and location info. This
system also handled dispatching into the correct routines for cases
where a raw type (e.g. "MyStruct") is used in expr land (vs an instance
of a type, e.g.  "my_struct"). This is important because certain types
are callable or attributable - specifically, constructors and enum
members (ex. `MyStruct()` and `MyEnum.FOO`).

This commit reworks the system by factoring the annotation info into
VyperType and ExprInfo. "Primitives" in the old system can be thought of
as VyperTypes, and "Definitions" in the old system are most similar to
the new ExprInfo. To handle cases where types can live in expr land,
this commit also clarifies that usage by promoting raw types into the
special, internal TYPE_T type (which can be thought of as "the type of
a type"). This commit also simplifies the inheritance structure of vyper
types.

Some other miscellaneous things in this commit:
- AST class hierarchy gets more refined - addition of ExprNode
- rename of `vyper/semantics/validation/` -> `vyper/semantics/analysis/`
- type system directory structure simplified
- merge of `builtin_functions/` and `builtin_interfaces/` to `builtins/`

Co-authored-by: z80 <[email protected]>

Description for the changelog

Cute Animal Picture

image

@lgtm-com
Copy link
Copy Markdown
Contributor

lgtm-com bot commented Jul 18, 2022

This pull request introduces 13 alerts and fixes 2 when merging 53de3d1 into 1d1ef5d - view on LGTM.com

new alerts:

  • 9 for Unused import
  • 3 for Unused local variable
  • 1 for First parameter of a method is not named 'self'

fixed alerts:

  • 1 for Conflicting attributes in base classes
  • 1 for Missing call to `__init__` during object initialization

@lgtm-com
Copy link
Copy Markdown
Contributor

lgtm-com bot commented Jul 18, 2022

This pull request introduces 10 alerts and fixes 2 when merging 516a8f7 into 1d1ef5d - view on LGTM.com

new alerts:

  • 6 for Unused import
  • 3 for Unused local variable
  • 1 for First parameter of a method is not named 'self'

fixed alerts:

  • 1 for Conflicting attributes in base classes
  • 1 for Missing call to `__init__` during object initialization

@charles-cooper
refactor builtins directory structure
1e10958 
merge builtin_functions/ and builtin_interfaces/ to builtins/
@lgtm-com
Copy link
Copy Markdown
Contributor

lgtm-com bot commented Nov 27, 2022

This pull request introduces 1 alert and fixes 3 when merging 1e10958 into 2d18e60 - view on LGTM.com

new alerts:

  • 1 for `__eq__` not overridden when adding attributes

fixed alerts:

  • 1 for Conflicting attributes in base classes
  • 1 for Missing call to `__init__` during object initialization
  • 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.

skellet0r
skellet0r approved these changes Nov 27, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@skellet0r skellet0r left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chad work ser.
I definitely like the reorganization done, adds clarity.

@lgtm-com
Copy link
Copy Markdown
Contributor

lgtm-com bot commented Nov 27, 2022

This pull request fixes 3 alerts when merging 7e565bd into 2d18e60 - view on LGTM.com

fixed alerts:

  • 1 for Conflicting attributes in base classes
  • 1 for Missing call to `__init__` during object initialization
  • 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.

@fubuloubu fubuloubu changed the title chore: refactor type system refactor: update type system hierarchy Nov 27, 2022
fubuloubu
fubuloubu approved these changes Nov 27, 2022
View reviewed changes
@charles-cooper charles-cooper changed the title refactor: update type system hierarchy refactor: front-end type system Nov 27, 2022
@charles-cooper charles-cooper mentioned this pull request Nov 27, 2022
Merged
@lgtm-com
Copy link
Copy Markdown
Contributor

lgtm-com bot commented Nov 27, 2022

This pull request fixes 3 alerts when merging 7a7a339 into 2d18e60 - view on LGTM.com

fixed alerts:

  • 1 for Conflicting attributes in base classes
  • 1 for Missing call to `__init__` during object initialization
  • 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.

@charles-cooper charles-cooper enabled auto-merge (squash) November 27, 2022 17:52
@lgtm-com
Copy link
Copy Markdown
Contributor

lgtm-com bot commented Nov 27, 2022

This pull request fixes 3 alerts when merging c7e23c4 into 2d18e60 - view on LGTM.com

fixed alerts:

  • 1 for Conflicting attributes in base classes
  • 1 for Missing call to `__init__` during object initialization
  • 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.

@charles-cooper charles-cooper merged commit 046ea16 into vyperlang:master Nov 27, 2022
@tserg tserg mentioned this pull request Nov 28, 2022
Merged
@tserg tserg mentioned this pull request Dec 10, 2022
Closed
@trocher trocher mentioned this pull request Jun 28, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@fubuloubu fubuloubu fubuloubu approved these changes

@skellet0r skellet0r skellet0r approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@charles-cooper @codecov-commenter @fubuloubu @skellet0r @z80dev