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

Refactored Type Checking and Analysis Annotations #13365

Closed
ekpyron opened this issue Aug 9, 2022 · 4 comments
Closed

Refactored Type Checking and Analysis Annotations #13365

ekpyron opened this issue Aug 9, 2022 · 4 comments
Assignees
Labels
epic effort Multi-stage task that may require coordination between team members across multiple PRs. high impact Changes are very prominent and affect users or the project in a major way. needs design The proposal is too vague to be implemented right away refactor selected for development It's on our short-term development

Comments

@ekpyron
Copy link
Member

ekpyron commented Aug 9, 2022

What

Make type checking robust against several invocations (creation mode; runtime mode), as well as out-of-order execution, even in the presence of cycles.

Why

To enable compile-time constant expression evaluation and the code data location, as well as making support of generics easier in the future.

What

Currently, the Solidity AST is const after parsing and then annotation()-annotated by the analysis steps in CompilerStack::analyze.

However, some AST nodes and some Types have interfaces that implicitly depend on annotations. (e.g. for types: ContractType::stateVariables, StructType::nativeMembers; e.g. for AST nodes: StructDefinition::type, VariableDeclaration::hasReferenceOrMappingType, etc.).

In the past we regularly had ICEs due to such implicit dependencies. That has gotten better recently, but it's still an easy source of error.

This issue is going to get worse, once we want to move to compile-time constant expression evaluation since this will probably entail switching multiple analysis steps to simultaneous cycle-resistant lazy-evaluation.

Furthermore we need to evaluate additional requirements looking ahead to type checking in the presence of future generics.

So we need to come up with a nicer way to:

  • Make all implicit dependencies of AST node and Type interfaces explicit. If possible, even syntactically prohibit errors (as in "too early use").
  • Make the relevant parts of type checking and analysis procedure robust against out-of-order lazy evaluation.
    • Since array lengths should be allowed to be calculated using compile-time constant expressions, this at least entails DeclarationTypeChecker and TypeChecker, with implicit interactions with ContractLevelChecker.

Notes

@ekpyron ekpyron added this to New issues in Solidity via automation Aug 9, 2022
@ekpyron ekpyron removed this from New issues in Solidity Aug 9, 2022
@cameel cameel added selected for development It's on our short-term development high effort A lot to implement but still doable by a single person. The task is large or difficult. high impact Changes are very prominent and affect users or the project in a major way. refactor needs design The proposal is too vague to be implemented right away epic effort Multi-stage task that may require coordination between team members across multiple PRs. and removed high effort A lot to implement but still doable by a single person. The task is large or difficult. labels Sep 15, 2022
@ekpyron
Copy link
Member Author

ekpyron commented Sep 15, 2022

Given #12932 there may be cause to do something similar even as early as the NameAndTypeResolver - this does not necessarily have to be done in one go, but it's something to keep in mind here.

@ekpyron
Copy link
Member Author

ekpyron commented Jun 6, 2023

This is actually superseded by #14284 now, so we won't try to fix this in current analysis, but rather keep this in mind as an issue to consider when writing experimental analysis.

@cameel
Copy link
Member

cameel commented Jun 7, 2023

This reminds me that I recently found something that seems to solve both tracking of dependencies and out of order evaluation: Attribute grammars. The nice thing about it is that it's not inventing anything new - seems to be a well known and researched subject.

Basically it's a formalization of the simple concept where AST nodes have attributes and these attributes are calculated based on rules. Rules can depend on nodes above (producing inherited attributes) or below (producing synthesized attributes). Instead of walking the AST and setting attributes ad-hoc like we do, you have an evaluator that determines the order and executes the rules. In our case the rules could be just functions that take already computed attributes or other AST nodes (or maybe just whole nodes) as input.

@NunoFilipeSantos
Copy link
Contributor

NunoFilipeSantos commented Jul 6, 2023

Dropped in favor of #14284 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
epic effort Multi-stage task that may require coordination between team members across multiple PRs. high impact Changes are very prominent and affect users or the project in a major way. needs design The proposal is too vague to be implemented right away refactor selected for development It's on our short-term development
Projects
Status: [ARCHIVED] ☀️ Q2 2023
Development

No branches or pull requests

4 participants