Closed fanyi-zhao closed 9 months ago
@fanyi-zhao The linter test is failing. Would you mind running cargo fmt --all
?
Attention: 121 lines
in your changes are missing coverage. Please review.
Comparison is base (
358a184
) 88.19% compared to head (32e8d9c
) 88.42%.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
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.
The LIR
Type
has no reference to the ast or any higher-level informationI 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
The LIR
Type
has no reference to the ast or any higher-level informationI 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:
Type
is 128 bytes. It's not huge though.Type
and then each time you need a type you index into the type vector (by index no, not rust reference).@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 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 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.
AFAIK @xermicus is away, no need to wait for his review
This PR links to the issue#923.
Here's a summary of the tasks related to the new three-address code format CFG:
Define the data structure of the new three-address code format CFG by using the following files:
src/lir/cfg.rs
: defines the enum typelir::LIR
andlir::Block
src/lir/instructions.rs
: defines the enum typelir::instructions::Instruction
for instructionssrc/lir/expressions.rs
: defines the enum typelir::expressions::Expression
src/lir/ssa_type.rs
: defines the enum typelir::lir_type::Type
Implement a text dump function for the new CFG by using the following files:
src/lir/printer/mod.rs
: defines the structlir::printer::Printer
, which delegates some operations onlir::vartable::Vartable
src/lir/printer/
: implement thelir::printer::Printer
for the new CFGConvert the old CFG into the new CFG of three-address codes by using the following files:
src/lir/converter/mod.rs
: defines the struct typelir::converter::Converter
, which delegates some operations onsema::ast::Namespace
src/lir/vartable.rs
: defines the struct typelir::vartable::Vartable
, which will later be used in thelir::converter::Converter
to mainly keep track of the temporary identifiers' number, name, and typesrc/lir/converter/
: implements thelir::converter::Converter
, which converts thecodegen::cfg::ControlFlowGraph
to thelir::LIR