hyperledger / solang

Solidity Compiler for Solana and Polkadot
https://solang.readthedocs.io/
Apache License 2.0
1.23k stars 206 forks source link

The three-address code format CFG #1577

Closed fanyi-zhao closed 7 months ago

fanyi-zhao commented 9 months ago

This PR links to the issue#923.

Here's a summary of the tasks related to the new three-address code format CFG:

  1. Define the data structure of the new three-address code format CFG by using the following files:

    • src/lir/cfg.rs: defines the enum type lir::LIR and lir::Block
    • src/lir/instructions.rs: defines the enum type lir::instructions::Instruction for instructions
    • src/lir/expressions.rs: defines the enum type lir::expressions::Expression
    • src/lir/ssa_type.rs: defines the enum type lir::lir_type::Type
  2. Implement a text dump function for the new CFG by using the following files:

    • src/lir/printer/mod.rs: defines the struct lir::printer::Printer, which delegates some operations on lir::vartable::Vartable
    • Other files in the folder src/lir/printer/: implement the lir::printer::Printer for the new CFG
  3. Convert the old CFG into the new CFG of three-address codes by using the following files:

    • src/lir/converter/mod.rs: defines the struct type lir::converter::Converter, which delegates some operations on sema::ast::Namespace
    • src/lir/vartable.rs: defines the struct type lir::vartable::Vartable, which will later be used in the lir::converter::Converter to mainly keep track of the temporary identifiers' number, name, and type
    • Other files in the folder src/lir/converter/: implements the lir::converter::Converter, which converts the codegen::cfg::ControlFlowGraph to the lir::LIR
LucasSte commented 7 months ago

@fanyi-zhao The linter test is failing. Would you mind running cargo fmt --all?

codecov[bot] commented 7 months ago

Codecov Report

Attention: 121 lines in your changes are missing coverage. Please review.

Comparison is base (358a184) 88.19% compared to head (32e8d9c) 88.42%.

Files Patch % Lines
src/lir/converter/expression.rs 95.33% 40 Missing :warning:
src/lir/printer/instruction.rs 92.00% 33 Missing :warning:
src/lir/converter/instruction.rs 96.53% 12 Missing :warning:
src/lir/converter/lir_type.rs 87.05% 11 Missing :warning:
src/lir/lir_type.rs 88.05% 8 Missing :warning:
src/lir/converter/mod.rs 96.77% 5 Missing :warning:
src/lir/expressions.rs 91.66% 5 Missing :warning:
src/lir/mod.rs 0.00% 2 Missing :warning:
src/lir/vartable.rs 96.49% 2 Missing :warning:
src/lir/instructions.rs 0.00% 1 Missing :warning:
... and 2 more
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1577 +/- ## ========================================== + Coverage 88.19% 88.42% +0.23% ========================================== Files 137 150 +13 Lines 65973 68322 +2349 ========================================== + Hits 58184 60415 +2231 - Misses 7789 7907 +118 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

LucasSte commented 7 months ago

The LIR Type has no reference to the ast or any higher-level information

I think @fanyi-zhao added references to the original AST type in the Vartable. Adding a Rust reference in the type itself is challenging due to the lifetimes.

LucasSte commented 7 months ago

The LIR Type has no reference to the ast or any higher-level information

I think @fanyi-zhao added references to the original AST type in the Vartable. Adding a Rust reference in the type itself is challenging due to the lifetimes.

@seanyoung

seanyoung commented 7 months ago

The LIR Type has no reference to the ast or any higher-level information

I think @fanyi-zhao added references to the original AST type in the Vartable. Adding a Rust reference in the type itself is challenging due to the lifetimes.

@seanyoung

I can think of two options:

LucasSte commented 7 months ago

@fanyi-zhao Can you address @seanyoung's suggestion?

I think we can consider the LIR type as being the following struct?

struct Type {
    ast_type: Option<ast::Type>,
    lir_type: lir::Type,
}

This way you can remove the references to the AST Type from the VarTable.

fanyi-zhao commented 7 months ago

@fanyi-zhao Can you address @seanyoung's suggestion?

I think we can consider the LIR type as being the following struct?

struct Type {
    ast_type: Option<ast::Type>,
    lir_type: lir::Type,
}

This way you can remove the references to the AST Type from the VarTable.

Sure. Thanks for the suggestion. This looks doable. I'll give it a try.

fanyi-zhao commented 7 months ago

@fanyi-zhao Can you address @seanyoung's suggestion?

I think we can consider the LIR type as being the following struct?

struct Type {
    ast_type: Option<ast::Type>,
    lir_type: lir::Type,
}

This way you can remove the references to the AST Type from the VarTable.

Hi @seanyoung, @LucasSte, I made some changes and pushed. Now we have a LIRType that holds the ast::Type.

seanyoung commented 7 months ago

AFAIK @xermicus is away, no need to wait for his review