Skip to content

Conversation

@Tokazama
Copy link
Collaborator

@Tokazama Tokazama commented May 4, 2021

There was a little reorganization here but this ones essentially about not generating extra code when checking traits using @nospecialize

Tokazama added 2 commits May 3, 2021 20:23
* added @nospecialize to is_static
* split each static type into its own file
* changed show method of StaticBool to static(true)/static(false)
@codecov
Copy link

codecov bot commented May 4, 2021

Codecov Report

Merging #13 (410762a) into master (dff22a6) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #13   +/-   ##
=======================================
  Coverage   99.70%   99.70%           
=======================================
  Files           4        6    +2     
  Lines         339      342    +3     
=======================================
+ Hits          338      341    +3     
  Misses          1        1           
Impacted Files Coverage Δ
src/int.jl 100.00% <ø> (ø)
src/Static.jl 100.00% <100.00%> (ø)
src/bool.jl 100.00% <100.00%> (ø)
src/symbol.jl 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dff22a6...410762a. Read the comment docs.

Comment on lines +90 to +97
is_static(@nospecialize(x)) = is_static(typeof(x))
is_static(@nospecialize(x::Type{<:StaticInt})) = True()
is_static(@nospecialize(x::Type{<:StaticBool})) = True()
is_static(@nospecialize(x::Type{<:StaticSymbol})) = True()
is_static(@nospecialize(x::Type{<:Val})) = True()
is_static(@nospecialize(x::Type{<:StaticFloat64})) = True()
is_static(x::Type{T}) where {T} = False()

Copy link
Collaborator Author

@Tokazama Tokazama May 4, 2021

Choose a reason for hiding this comment

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

@timholy , If I want to avoid generating a unique method for every variant of StaticInt is this the right approach or will it kill inference somewhere down the line? I used MethodAnalysis.methodinstances and it seems to avoid make extra methods but I'm never sure with this sort of stuff

Copy link
Collaborator

Choose a reason for hiding this comment

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

Presumably everything still infers and optimizes fine, because the behavior doesn't actually depend on the particular types?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's the goal. I was hoping to start strategically adding these to trait functions so that we can avoid some of the pitfalls of StaticArrays.

Copy link
Collaborator

@chriselrod chriselrod left a comment

Choose a reason for hiding this comment

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

LGTM.

@Tokazama Tokazama merged commit 0f293e9 into master Jun 2, 2021
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