refactor(transformer): shorten code

[overlookmotel]

Use BoundIdentifier::create_binding_pattern, rather than writing out the code to create a BindingPattern manually each time.

[graphite-app]

Your org has enabled the Graphite merge queue for merging into main

Add the label “0-merge” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

[overlookmotel]

• refactor(transformer): shorten code #6809 👈
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @overlookmotel and the rest of your teammates on Graphite

[codspeed-hq]

CodSpeed Performance Report

Merging #6809 will not alter performance

_{Comparing 10-23-refactor_transformer_shorten_code (b8dfa19) with main (e59b5d9)}

Summary

✅ 30 untouched benchmarks

[Dunqing]

I have to say the BoundIdentifier is so great!

[Dunqing]

Merge activity

• Oct 23, 9:11 AM EDT: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
• Oct 23, 9:11 AM EDT: A user added this pull request to the Graphite merge queue.
• Oct 23, 9:25 AM EDT: A user merged this pull request with the Graphite merge queue.

[overlookmotel]

I have to say the BoundIdentifier is so great!

I still think we can improve the API. I would prefer methods on the AST types e.g. BindingIdentifier::from_binding(binding, ctx), BindingPattern::from_binding(binding, ctx). But at least BoundIdentifier removes boilerplate code for things we do commonly.

[Dunqing]

I still think we can improve the API. I would prefer methods on the AST types e.g. BindingIdentifier::from_binding(binding, ctx), BindingPattern::from_binding(binding, ctx). But at least BoundIdentifier removes boilerplate code for things we do commonly.

Yes, that would be better, but unfortunately, we can't get the Atom from SymbolTable::get_name, this may incur additional performance overhead.

[overlookmotel]

Yes, that would be better, but unfortunately, we can't get the Atom from SymbolTable::get_name, this may incur additional performance overhead.

Yeah we need to switch Semantic from storing names as CompactStrs to Atoms. That'd be a significant perf boost, and make lots of other things simpler. The blocker on that is that it makes SymbolTable need a lifetime SymbolTable<'a>, and apparently that causes problems for Rolldown (though I don't understand why).