Strange Lispy warning on master


#1

I’m not sure what I’m supposed to do with this:

WARNING: deprecated syntax “parametric method syntax (::Core.Type{Str{T}}){(var-bounds T #f #f)}(S, v::Vector{UInt8}) around /Users/scott/.julia/v0.7/Strs/src/types.jl:57”.
Use “(::Core.Type{Str{T}})(S, v::Vector{UInt8}) where (var-bounds T #f #f)” instead.
ERROR: LoadError: LoadError: MethodError: Cannot convert an object of type Array{UInt8,1} to an object of type Strs.Str{:Binary}

The code was this:

struct Str{T} <: AbstractString
    data::Vector{UInt8}
    Str(S, v::Vector{UInt8}) = (new{S}(v) where {S})
end

Things were working fine, before I added the inner constructor, but now I get that strange error.
What I’m trying accomplish is simply that I need a constructor, that can safely take a Vector{UInt8} that has already be set up (allocated always via Base.StringVector(len), and with the correct type T that indicates what the character set and encoding are), but make Str(vec) where vec is a Vector{UInt8}, call an outer constructor, that looks at vec, assuming it’s UTF-8, and checks validity and returns with an instance of one of the Str subtypes.

Anybody know what’s going on? Thanks in advance!


#2

I think you need

struct Str{T} <: AbstractString
    data::Vector{UInt8}
               
    Str{T}(S, v::Vector{UInt8}) where {T} = (new{S}(v) where {S})
end

i.e. to make the inner constructor parametric on T.


#3

Hmm, shouldn’t S be related to T, though?


#4

What is that where {S} on the right side for? Is it really necessary?
I’m assuming S must be a symbol right?
Doesn’t the inner constructor below work?

Str(S::Symbol,v) = new{S}(v)

#5

I’m still trying to work out all the details of the new syntax, we only recently (this month) moved over to v0.6.1 for our code (still using the old v0.5 syntax that has been deprecated on master)
That’s why I’m getting twisted about so much! (I do appreciate the much greater power of the new syntax,
and don’t mind having to rewrite things to adapt to it, it just will take a little time to learn all the nitty gritty details!)


#6

Currently a symbol, but I’m in the process of changing it, using @nalimilan’s nice approach (in the pull request https://github.com/nalimilan/StringEncodings.jl/pull/9) to use singleton types that take a Symbol.
(Splitting out CharSet, Encoding, and CharSetEncoding - those are not the same, but people frequently conflate them)


#7

Anyway, part of my real issue, was that I feel this deprecation warning is a bug - I don’t think what it tells you to use instead (var-bounds T #f #f) is valid Julia syntax :slight_smile:

edit: (It’s probably something that @jeff.bezanson can fix in a minute!)


#8

Is this what you want?

julia> struct Str{T} <: AbstractString
           data::Vector{UInt8}
           Str{S}(v::Vector{UInt8}) where {S} = new(v)
       end

julia> Str(S, v::Vector{UInt8}) = Str{S}(v)
Str

julia> a = Str(:encoding, UInt8[1,2,3]);

julia> a.data
3-element Array{UInt8,1}:
 0x01
 0x02
 0x03

julia> typeof(a)
Str{:encoding}

#9

Thank you very much!
With that, and the addition of:
Str(v::Vector{UInt8}) = Str(:Binary, v)
it does exactly what I want!
:smile:


#10

Btw, this function is not type stable, because the function may return a whole family of output types for the same input types.

julia> @code_warntype Str(:encoding, UInt8[1, 2, 3])
Variables:
  #self# <optimized out>
  S::Symbol
  v::Array{UInt8,1}

Body:
  begin
      return ((Core.apply_type)(Main.Str, S::Symbol)::Type{Str{_}} where _)(v::Array{UInt8,1})::Str{_} where _
  end::Str{_} where _

If S is known at compile time, you could do this instead which is type stable.

Str(::Val{S}, v::Vector{UInt8}) where {S} = Str{S}(v)
# a = Str(Val{:encoding}(), UInt[1,2,3])

#11

Rather than using the Val trick, I would suggest just migrating your usage fromStr(S, v) to Str{S}(v), by analogy to the way the Array constructors have changed from Array(T, dims...) to Array{T}(dims...). You can even deprecate the two-argument version to make it easy to find usages that need to be updated:

@deprecate Str(S, v) Str{S}(v)

#12

No need to deprecate, this isn’t even released yet, I just started writing it Dec. 15th.

I need an inner constructor, so it will create it with the type, which determines the behavior, but is not in any fields, and also to prevent a problem that I’ve run into, that without an inner constructor, Julia sets up a default inner constructor, with behavior I want to make not possible (i.e. Str(vec) where vec is a Vector{UInt8}, or Str(str), where str is of type String, which for some reason is causing a deprecation warning, instead of calling the Str(str::String) outer constructor that I had defined.
I’ll have to see if I come up with a MWE of the problem, to see if it’s a bug, or just something wrong in my code.


#13

This might be due in part to the deprecation of special “inner constructor” behavior in 0.6: before 0.6, writing Str(x) = ... inside a type definition block actually defined a constructor for calls that look like Str{T}(x) (which is why it was confusing and needed to be deprecated). While this deprecation is in place, a constructor for Str(x) calls can be defined inside the type definition block using (::Type{Str})(x) = .... When the deprecation is removed (which could probably be right now, actually) it will be possible to write it as just Str(x) = ... like you’d expect.


#14

Thanks! It was driving me a bit batty this morning, I want to get the last few problems resolved so I can concentrate on improving the benchmarking, and push a WIP version of my Strs.jl package.


#15

I’m still not able to get this working:

julia> struct Str{T} <: AbstractString
           data::Vector{UInt8}
           (::Type{Str})(::Type{S}, v::Vector{UInt8}) where {S} = new{S}(v)
       end

julia> (::Type{Str{T} where T})(v::Vector{UInt8}) = Str(T, v)

julia> Str{:Binary}(b"foo")
ERROR: MethodError: Cannot `convert` an object of type Array{UInt8,1} to an object of type Str{:Binary}
Stacktrace:
 [1] Str{:Binary}(::Array{UInt8,1}) at ./deprecated.jl:1590
 [2] top-level scope

Any ideas?

edit: What I’m trying to accomplish is make it so that the const type aliases such as BinaryStr, ASCIIStr, UTF8Str, which are simply (currently) Str{:Binary}, Str{:ASCII}, Str{:UTF8} (I’m in the middle of changing them so that they might be more complicated, such as UTF16BEStr being Str{cse"UTF16BE", Nothing, Nothing} (where the unused type parameters are for built-in substrings, and cached raw data, UTF-8 or UTF-16 encodings of the string, much as Python does).


#16
julia> struct Str{T} <: AbstractString
         data::Vector{UInt8}
         Str{S}(v::Vector{UInt8}) where {S} = new{S}(v)
       end

julia> Str{:binary}(b"foo");

if you want your non-type-stable outer constructor, it’s just:

julia> Str(S, v::Vector{UInt8}) = Str{S}(v)
Str

julia> typeof(Str(:binary, b"foo"))
Str{:binary}

#17

I’m still running into this problem:

julia> struct Str{T} <: AbstractString
           data::Vector{UInt8}
           (::Type{Str})(::Type{S}, v::Vector{UInt8}) where {S} = new{S}(v)
       end

julia> Str(S, v::Vector{UInt8}) = Str{S}(v)
Str

julia> const BinaryStr = Str{:Binary}
Str{:Binary}

julia> BinaryStr(b"123")
ERROR: MethodError: Cannot `convert` an object of type Array{UInt8,1} to an object of type Str{:Binary}
Stacktrace:
 [1] Str{:Binary}(::Array{UInt8,1}) at ./deprecated.jl:1590
 [2] top-level scope

#18

FWIW, with IPO, the “non-type stable” version infers fine.


#19

Yes, I’ve been very happy with some of the new enhancements such as that!


#20

Your inner constructor is still incorrect (you’re only defining Str(S, v) where S is a Type, but (a) S is a Symbol, not a Type and (b) you need the Str{S}(v) constructor, which can only be defined as an inner constructor). This works with the version I posted above:

julia> struct Str{T} <: AbstractString
          data::Vector{UInt8}
          Str{S}(v::Vector{UInt8}) where {S} = new{S}(v)
        end

julia> const BinaryStr = Str{:Binary}
Str{:Binary}

julia> BinaryStr(b"123");