Skip to content

Coerce the result of a function call to the function's return type #6309

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

Merged
merged 1 commit into from
Apr 21, 2025

Conversation

nrc
Copy link
Contributor

@nrc nrc commented Apr 14, 2025

No description provided.

@nrc nrc requested a review from jtran April 14, 2025 07:15
Copy link

qa-wolf bot commented Apr 14, 2025

QA Wolf here! As you write new code it's important that your test coverage is keeping up.
Click here to request test coverage for this PR!

Copy link

vercel bot commented Apr 14, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
modeling-app ✅ Ready (Inspect) Visit Preview Apr 21, 2025 9:24pm

Copy link

codecov bot commented Apr 14, 2025

Codecov Report

Attention: Patch coverage is 92.06349% with 10 lines in your changes missing coverage. Please review.

Project coverage is 85.16%. Comparing base (30ee547) to head (5a061d4).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
rust/kcl-lib/src/execution/exec_ast.rs 91.45% 10 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6309   +/-   ##
=======================================
  Coverage   85.15%   85.16%           
=======================================
  Files         108      108           
  Lines       46269    46348   +79     
=======================================
+ Hits        39402    39470   +68     
- Misses       6867     6878   +11     
Flag Coverage Δ
rust 85.16% <92.06%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link

codspeed-hq bot commented Apr 14, 2025

CodSpeed Walltime Performance Report

Merging #6309 will improve performances by 18.15%

Comparing nrc-coerce-return (e5111d6) with main (941911e)

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

Summary

⚡ 44 improvements
✅ 48 untouched benchmarks

Benchmarks breakdown

Benchmark BASE HEAD Change
parse_ball-bearing 3.8 ms 3.4 ms +12.4%
parse_bench 3.7 ms 3.2 ms +14.86%
parse_bracket 4.6 ms 4.2 ms +11.51%
parse_car-wheel-assembly 398.6 µs 360.8 µs +10.46%
parse_color-cube 2.4 ms 2.1 ms +12.67%
parse_cycloidal-gear 4.1 ms 3.7 ms +12.31%
parse_dodecahedron 7.4 ms 6.5 ms +13.82%
parse_dual-basin-utility-sink 15.6 ms 13.7 ms +13.66%
parse_enclosure 11.7 ms 10.3 ms +13.92%
parse_exhaust-manifold 9.3 ms 8 ms +15.26%
execute_flange 2.1 s 1.9 s +10.1%
parse_flange 1.5 ms 1.3 ms +11.69%
parse_focusrite-scarlett-mounting-bracket 8.6 ms 7.6 ms +13.28%
parse_food-service-spatula 12.6 ms 10.9 ms +15.7%
parse_french-press 11.7 ms 10.1 ms +15.95%
parse_gear-rack 2.6 ms 2.3 ms +12.28%
parse_gear 9.4 ms 8.4 ms +12.22%
parse_gridfinity-baseplate-magnets 16.3 ms 14.5 ms +12.42%
parse_gridfinity-baseplate 10.3 ms 8.9 ms +14.59%
parse_gridfinity-bins-stacking-lip 22.1 ms 19.6 ms +12.63%
... ... ... ... ...

ℹ️ Only the first 20 benchmarks are displayed. Go to the app to view all benchmarks.

@jtran
Copy link
Contributor

jtran commented Apr 14, 2025

This makes it so that a return type annotation on a function can have a runtime effect? What's a motivating use case for this? Maybe we can add a simple unit test.

@nrc
Copy link
Contributor Author

nrc commented Apr 15, 2025

This makes it so that a return type annotation on a function can have a runtime effect? What's a motivating use case for this? Maybe we can add a simple unit test.

The use case is something like fn foo(): number(mm) { return 42 } so that the returned value has the mm type rather than the default type. I'll add it as a test (I was being lazy and this is tested by the std lib functions already,but you're right it should have a unit test).

@nrc nrc force-pushed the nrc-coerce-return branch 2 times, most recently from c79561b to 64673ef Compare April 15, 2025 10:16
@nrc
Copy link
Contributor Author

nrc commented Apr 15, 2025

This makes it so that a return type annotation on a function can have a runtime effect? What's a motivating use case for this? Maybe we can add a simple unit test.

The use case is something like fn foo(): number(mm) { return 42 } so that the returned value has the mm type rather than the default type. I'll add it as a test (I was being lazy and this is tested by the std lib functions already,but you're right it should have a unit test).

Adding the test was easy, but in fixing the failing test, I had to do a bunch of work because types were not being properly exported (and had to factor out a std::types module to avoid import cycles in std).

Copy link

codspeed-hq bot commented Apr 17, 2025

CodSpeed Instrumentation Performance Report

Merging #6309 will not alter performance

Comparing nrc-coerce-return (5a061d4) with main (f8ca6ad)

Summary

✅ 54 untouched benchmarks

@nrc nrc merged commit e4e18df into main Apr 21, 2025
58 checks passed
@nrc nrc deleted the nrc-coerce-return branch April 21, 2025 23:00
modeling-app-github-app bot pushed a commit that referenced this pull request Apr 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants