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

all([static(true), static(true)]) fails #36

Open
cscherrer opened this issue Jan 24, 2022 · 3 comments
Open

all([static(true), static(true)]) fails #36

cscherrer opened this issue Jan 24, 2022 · 3 comments

Comments

@cscherrer
Copy link
Contributor

With Julia 1.7.1 and Static 0.5.1,

julia> all([static(true), static(true)])
ERROR: TypeError: non-boolean (True) used in boolean context
Stacktrace:
 [1] _all
   @ ./reduce.jl:1161 [inlined]
 [2] _all
   @ ./reducedim.jl:903 [inlined]
 [3] #all#750
   @ ./reducedim.jl:901 [inlined]
 [4] all(a::Vector{True})
   @ Base ./reducedim.jl:901
 [5] top-level scope
   @ REPL[60]:1

The problem seems to coming from this in Base:

function _all(f, itr, ::Colon)
    anymissing = false
    for x in itr
        v = f(x)
        if ismissing(v)
            anymissing = true
        # this syntax allows throwing a TypeError for non-Bool, for consistency with any
        elseif v
            continue
        else
            return false
        end
    end
    return anymissing ? missing : true
end

Interestingly, these work fine:

julia> all([static(true), false])
false

julia> all([static(true), true])
true
@chriselrod
Copy link
Collaborator

chriselrod commented Jan 24, 2022

julia> [static(true), false]
2-element Vector{Bool}:
 1
 0

julia> [static(true), static(false)]
2-element Vector{StaticBool}:
  static(true)
 static(false)

I think we want a Vector{Bool} in both cases.
The last isn't type stable.

We probably always want Vector{Bool}, even if it'd be type stable.
I don't see why we'd ever want to construct a Vector{StaticBool{true}}. It could never contain a false, and is just wasting memory. The user should create a FillArray instead.

@cscherrer
Copy link
Contributor Author

I agree, but this could work too:

julia> using Static

julia> x = [static(true), static(true)]
2-element Vector{True}:
 static(true)
 static(true)

julia> Base.all(v::AbstractArray{B}) where {B<:StaticBool} = all(dynamic, v)

julia> all(x)
true

@chriselrod
Copy link
Collaborator

chriselrod commented Jan 24, 2022

If you really want to define that, I'd add

Base.all(v::AbstractArray{SB}) where {B,SB<:StaticBool{B}} = B

That way

julia> w = [static(true), static(true)]
2-element Vector{True}:
 static(true)
 static(true)

julia> x = [static(true), static(false)]
2-element Vector{StaticBool}:
  static(true)
 static(false)

julia> Base.all(v::AbstractArray{B}) where {B<:StaticBool} = all(dynamic, v)

julia> Base.all(v::AbstractArray{SB}) where {B,SB<:StaticBool{B}} = B

julia> @code_typed all(w)
CodeInfo(
1return $(Expr(:static_parameter, 1))
) => Bool

julia> @code_llvm all(w)
define i8 @julia_all_1706({} addrspace(10)* nonnull align 16 dereferenceable(40) %0) #0 !dbg !5 {
top:
  ret i8 1, !dbg !7
}

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

No branches or pull requests

2 participants