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

transformer: general helper for insert, delete and replace statement #6993

Open
Dunqing opened this issue Oct 29, 2024 · 4 comments · May be fixed by #7015
Open

transformer: general helper for insert, delete and replace statement #6993

Dunqing opened this issue Oct 29, 2024 · 4 comments · May be fixed by #7015
Assignees
Labels
A-transformer Area - Transformer / Transpiler C-enhancement Category - New feature or request

Comments

@Dunqing
Copy link
Member

Dunqing commented Oct 29, 2024

Now, we have StatementInjection to help us inject statements, this is very useful but is not enough for our usage. So would be better if we combine these usages into one statement helper

Cases

Delete

fn exit_statements(
&mut self,
stmts: &mut ArenaVec<'a, Statement<'a>>,
_ctx: &mut TraverseCtx<'a>,
) {
// Remove TS specific statements
stmts.retain(|stmt| match stmt {
Statement::ExpressionStatement(s) => !s.expression.is_typescript_syntax(),
// Any namespaces left after namespace transform are type only, so remove them
Statement::TSModuleDeclaration(_) => false,
match_declaration!(Statement) => !stmt.to_declaration().is_typescript_syntax(),
// Ignore ModuleDeclaration as it's handled in the program
_ => true,
});
}

Replace

// We need to replace the current statement with new statements,
// but we don't have a such method to do it, so we leverage the statement injector.
//
// Now, we use below steps to workaround it:
// 1. Use the last statement as the new statement.
// 2. insert the rest of the statements before the current statement.
// TODO: Once we have a method to replace the current statement, we can simplify this logic.
let mut statements = self.transform_for_of_statement(for_of, ctx);
let last_statement = statements.pop().unwrap();
*stmt = last_statement;
self.ctx.statement_injector.insert_many_before(&stmt.address(), statements);

@Dunqing Dunqing added C-enhancement Category - New feature or request A-transformer Area - Transformer / Transpiler labels Oct 29, 2024
@Dunqing Dunqing self-assigned this Oct 29, 2024
@magic-akari
Copy link
Contributor

Why is a helper needed for replacement?

@Dunqing
Copy link
Member Author

Dunqing commented Oct 30, 2024

Why is a helper needed for replacement?

As I mentioned above cases, we need to replace a statement with multiple statements. If just one-to-one replacement, we can mutate the original statement directly

@Boshen
Copy link
Member

Boshen commented Oct 30, 2024

replace -> splice

@overlookmotel
Copy link
Collaborator

overlookmotel commented Oct 30, 2024

Isn't "replace 1 statement with 3 statements" equivalent to "mutate existing statement + insert 2 new statements after it"? (like in your example)

So then do we need the "replace" action? Is there a reason why that's unergonomic?

2nd point: "Delete" action doesn't necessarily need an Address. It could be a single boolean flag per Vec<Statement> - probably stored as a NonEmptyStack<bool>.

  • Helper contains a flag for "statement block needs some statements deleted".
  • During transform of the statement, replace it with ctx.ast.move_statement() which creates a dummy EmptyStatement with Span { start: 0, end: 0 }.
  • Call method on helper to indicate that some statements need to be deleted, but you don't need to tell it which.
  • Helper checks the "does it need statements deleted" flag in it's exit_statements visitor, and if that flag says "yes", it deletes all EmptyStatements with 0 span.

I can't imagine a situation where a transform actually needs to insert a real EmptyStatement and that it must not be deleted.

Note: Ideal pattern if you're going to delete statements is to do it as early as possible, so then other transforms don't spend time traversing and transforming code which is going to be deleted anyway.

Note2: NonEmptyStack is really cheap. Because the stack is never allowed to be empty, stack.last() is infallible (returns T not Option<T>) and is just a single assembly instruction. This is much cheaper than Vec::last, which has to do bounds checks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-transformer Area - Transformer / Transpiler C-enhancement Category - New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants
@overlookmotel @Boshen @magic-akari @Dunqing and others