fix(compiler): implicit optional args not working (sometimes) for methods

winglang / wing

A programming language for the cloud ☁️ A unified programming model, combining infrastructure and runtime code into one language ⚡

https://winglang.io

Other

4.75k stars 186 forks source link

fix(compiler): implicit optional args not working (sometimes) for methods #6633

Closed yoav-steinberg closed 3 weeks ago

yoav-steinberg commented 3 weeks ago

fixes #6559

The fold mechanism re-generated the IDs used to identify argument lists in the AST. This means that even though we currently detected implicitly optional arguments we lost that information after the closure transform forld operation. I fixed the default fold implementation not to regenerated these IDs, similar to how we preserve expresion IDs during folds, for example.

Checklist

[x] Title matches Winglang's style guide
[x] Description explains motivation and solution
[x] Tests added (always)
[ ] Docs updated (only required for features)
[ ] Added pr/e2e-full label if this feature requires end-to-end testing

By submitting this pull request, I confirm that my contribution is made under the terms of the Wing Cloud Contribution License.

github-actions[bot] commented 3 weeks ago

Thanks for opening this pull request! :tada: Please consult the contributing guidelines for details on how to contribute to this project. If you need any assistance, don't hesitate to ping the relevant owner over Discord.

Topic	Owner
Wing SDK and utility APIs	`@chriscbr`
Wing Console	`@ainvoner`, `@skyrpex`, `@polamoros`
JSON, structs, primitives and collections	`@hasanaburayyan`
Platforms and plugins	`@hasanaburayyan`
Frontend resources (website, react, etc)	`@tsuf239`
Language design	`@chriscbr`
VSCode extension and language server	`@markmcculloh`
Compiler architecture, inflights, lifting	`@yoav-steinberg`
Wing Testing Framework	`@tsuf239`
Wing CLI	`@markmcculloh`
Build system, dev environment, releases	`@markmcculloh`
Library Ecosystem	`@chriscbr`
Documentation	`@hasanaburayyan`
SDK test suite	`@tsuf239`
Examples	`@hasanaburayyan`
Wing Playground	`@eladcon`

monadabot commented 3 weeks ago

Console preview environment is available at https://wing-console-pr-6633.fly.dev :rocket:

Last Updated (UTC) 2024-06-04 08:23

monadabot commented 3 weeks ago

Benchmarks

Comparison to Baseline ⬜⬜⬜⬜⬜⬜🟥⬜⬜⬜⬜🟥⬜

| Benchmark | Before | After | Change | | :-- | --: | --: | --: | | version | 58ms±0.65 | 59ms±0.65 | +1ms (+2.03%)⬜ | | functions_10.test.w -t sim | 519ms±22.56 | 496ms±9.87 | -22ms (-4.29%)⬜ | | functions_10.test.w -t tf-aws | 2136ms±15.59 | 2159ms±32.83 | +24ms (+1.11%)⬜ | | hello_world.test.w -t sim | 411ms±8.2 | 413ms±7.06 | +2ms (+0.47%)⬜ | | hello_world.test.w -t tf-aws | 1459ms±15.45 | 1495ms±11.64 | +36ms (+2.49%)⬜ | | functions_1.test.w -t sim | 410ms±5.26 | 414ms±5.75 | +4ms (+0.96%)⬜ | | functions_1.test.w -t tf-aws | 836ms±3.63 | 863ms±5.83 | +27ms (+3.25%)🟥 | | jsii_big.test.w -t sim | 2829ms±23.52 | 2837ms±22.38 | +8ms (+0.28%)⬜ | | jsii_big.test.w -t tf-aws | 3013ms±12.93 | 3051ms±25.73 | +38ms (+1.25%)⬜ | | empty.test.w -t sim | 372ms±5.34 | 374ms±3.95 | +2ms (+0.41%)⬜ | | empty.test.w -t tf-aws | 598ms±4.69 | 610ms±3.98 | +12ms (+2%)⬜ | | jsii_small.test.w -t sim | 375ms±4.38 | 387ms±5.88 | +12ms (+3.21%)🟥 | | jsii_small.test.w -t tf-aws | 621ms±4.17 | 629ms±5.63 | +8ms (+1.32%)⬜ | ⬜ Within 1.5 standard deviations 🟩 Faster, Above 1.5 standard deviations 🟥 Slower, Above 1.5 standard deviations _Benchmarks may vary outside of normal expectations, especially when running in GitHub Actions CI._

Results

|name|mean|min|max|moe|sd| |----|----|----|----|----|----| |version|59ms|57ms|60ms|1ms|1ms| |functions_10.test.w -t sim|496ms|473ms|520ms|10ms|14ms| |functions_10.test.w -t tf-aws|2159ms|2089ms|2266ms|33ms|46ms| |hello_world.test.w -t sim|413ms|403ms|432ms|7ms|10ms| |hello_world.test.w -t tf-aws|1495ms|1472ms|1521ms|12ms|16ms| |functions_1.test.w -t sim|414ms|407ms|435ms|6ms|8ms| |functions_1.test.w -t tf-aws|863ms|853ms|877ms|6ms|8ms| |jsii_big.test.w -t sim|2837ms|2792ms|2887ms|22ms|31ms| |jsii_big.test.w -t tf-aws|3051ms|3007ms|3143ms|26ms|36ms| |empty.test.w -t sim|374ms|364ms|381ms|4ms|6ms| |empty.test.w -t tf-aws|610ms|599ms|619ms|4ms|6ms| |jsii_small.test.w -t sim|387ms|379ms|402ms|6ms|8ms| |jsii_small.test.w -t tf-aws|629ms|619ms|645ms|6ms|8ms|

Last Updated (UTC) 2024-06-04 08:28

mergify[bot] commented 3 weeks ago

Thanks for contributing, @yoav-steinberg! This PR will now be added to the merge queue, or immediately merged if yoav/fix_implicit_optional_arg is up-to-date with main and the queue is empty.

monadabot commented 3 weeks ago

Congrats! :rocket: This was released in Wing 0.74.36.