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

Allow a Tuple{typeof(f),...} as root of ascend tree #588

Merged
merged 3 commits into from
Aug 17, 2024
Merged

Conversation

timholy
Copy link
Member

@timholy timholy commented Aug 15, 2024

@timholy timholy requested a review from aviatesk August 15, 2024 08:59
@codecov-commenter
Copy link

codecov-commenter commented Aug 16, 2024

Codecov Report

Attention: Patch coverage is 0% with 18 lines in your changes missing coverage. Please review.

Project coverage is 0.00%. Comparing base (2c33b9a) to head (79d600f).
Report is 8 commits behind head on master.

Files Patch % Lines
src/callsite.jl 0.00% 12 Missing ⚠️
src/Cthulhu.jl 0.00% 3 Missing ⚠️
src/backedges.jl 0.00% 2 Missing ⚠️
src/reflection.jl 0.00% 1 Missing ⚠️
Additional details and impacted files
@@          Coverage Diff           @@
##           master    #588   +/-   ##
======================================
  Coverage    0.00%   0.00%           
======================================
  Files           9       9           
  Lines        1556    1574   +18     
======================================
- Misses       1556    1574   +18     

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

@timholy timholy removed the request for review from aviatesk August 17, 2024 18:30
@timholy
Copy link
Member Author

timholy commented Aug 17, 2024

I've been using this for a couple of hours and it's been great, so I think it's baked well enough to release.

@timholy timholy merged commit 6a6c12d into master Aug 17, 2024
17 of 29 checks passed
@timholy timholy deleted the teh/for_jet branch August 17, 2024 18:32
Comment on lines +525 to +526
for (a, b) in zip(info.argtyps, tt.parameters[2:end])
a === b || return false
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When iterating over a zip created from a Vector{Any}, objects of type Tuple{<:Any,<:Any} are created, which can sometimes lead to unnecessary specializations, especially when the vector contains Type objects.
Since we're only using ===, it shouldn't be a problem for this case, but it's something to be mindful of when working on Core.Compiler and other external abstract interpreters in the future.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I figured this case would be OK. I was leaning on the "self check" tests too, I admit!

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.

3 participants