Add a few new reflection methods (after type system merge) #20006

Keno · 2017-01-12T23:51:51Z

This is informed by attempting to port some packages to the new type system.
I have abstracted out the functionality required by those packages into
documented/tested functions. The hope is to avoid introducing too many
implementation details to packages. These functions can also be implemented
in Compat for older julia versions, such that packages can immediate replace
their use of internal APIs with these functions.

Keno · 2017-01-12T23:52:09Z

Names subject to bikeshedding of course.

JeffBezanson · 2017-01-13T00:07:19Z

LGTM. These and various other type-related functions scattered around Base should probably be collected into a TypeUtils module eventually.

tkelman · 2017-01-13T00:08:02Z

base/exports.jl

@@ -1002,6 +1002,7 @@ export
    fieldoffset,
    fieldname,
    fieldnames,
+    isabstract,


needs to go in stdlib doc index

tkelman · 2017-01-13T00:10:28Z

base/reflection.jl

+   Base.parameter_upper_bound(t::UnionAll, idx)
+
+Determine the upper bound of a type parameter in the underlying type. E.g.:
+```


would make a good doctest (though I don't think unexported docstrings' doctests get run by default)

ararslan · 2017-01-13T00:11:49Z

Looks like you've used 3-space indents throughout the diff. For consistency it should be 4 spaces. (Indeed, AFAIK, to get the correct formatting in the resulting markdown from docstrings, the signature actually has to be indented 4 spaces.)

Keno · 2017-01-13T00:14:05Z

Unintentional. Sometimes editors are too smart for their own good.

Keno · 2017-01-13T00:28:29Z

@JeffBezanson expressed a preference for not exporting them from base until we create the TypeUtils module. As a result, I've unexported isabstract. To allow packages to use this, the proposed strategy is to go ahead and create a Compat.TypeUtils which re-exports these and provides the Compat-ed version for old versions. In the future that can then become const TypeUtils = Base.TypeUtils in Compat, without requiring any code changes.

JeffBezanson · 2017-01-13T01:39:58Z

Sorry, you'll have to rebase this.

Keno · 2017-01-13T22:01:11Z

Rebased + a new commit that allows ignoring ambiguities (in detect_ambiguities) due to type parameters becoming Union{}.

See JuliaLang/julia#20006

vtjnash · 2017-01-15T01:12:59Z

base/reflection.jl

+```
+"""
+unparameterized_type(t::DataType) = t.name.wrapper
+unparameterized_type(t::UnionAll) = unparameterized_type(unwrap_unionall(t))


maybe we could try to give this a better name, although I'm not sure what. I think this is essentially supertype? So maybe something like basetype? In general, I think we've tried to avoid providing this, since it makes it tempting to assume you could re-parameterize it (even though basetype(T){tparam} is not generally a well-defined operation)

(even though basetype(T){tparam} is not generally a well-defined operation)

Could you elaborate on this point?

That transform isn't meaningful for an arbitrary type T. It's actually not really even valid for a non-arbitrary type T (for example, f(D::Type{Dict}, K, V) = D{K, V} cannot be inferred, leading to this needed correction: 210d4ec#diff-d4736682004c1996e5d9b1c5c33c8c8e).

In the only cases where it is valid (e.g. you passed a type literal to basetype), you could just write that same type literal without the call to basetype. And the resulting code would also be less error-prone if someone replaced the original type with a type-alias (in addition to being shorter, by virtue of not containing no-op function calls).

What's your definition of valid? That function definition you wrote down works for at least some uses.

I mean that it's not valid to infer the output of that example function, and it's brittle to changes in the definition of Dict. Or consider Vector{Int} -- it has multiple base types, and one of the proposals for #4774 would have involved adding another. Thus you can't generally assume that you can still parameterize a type object after extracting the base type and get back something meaningful.

Ok, I think I see, the point, but I'm not sure I understand why we can't infer D{K,V}.

D is known to be type-equal to Dict, but it is not known to actually be Dict

Keno · 2017-01-22T03:28:47Z

Rebased and renamed unparameterized_type to basetype as suggested by @vtjnash

vtjnash · 2017-01-22T03:42:53Z

Not to derail my bikeshedding of unparameterized_type further, but maybe we should only expose typename(T), since that's probably the more useful thing to extract?

Keno · 2017-01-22T03:44:22Z

What would the semantics of that be?

Keno · 2017-01-22T03:51:27Z

The motivating example for basetype here is to replace this: https://github.com/JuliaGraphics/ColorTypes.jl/blob/master/src/traits.jl#L154

vtjnash · 2017-01-22T04:37:04Z

hm, that seems tricky. That code is implementing the sorts of dubious assumptions I was mentioning are usually best avoided (usually by implementing similar). Strictly speaking, it doesn't appear to do anything wrong, it's basically just equivalent to assuming that all subtypes of Color contain a field named .parameters[1] and that any other parameters are superfluous. This happens to work, since the subtypes implemented in Colors.jl don't contain any other parameters.

typename would be typename(T::DataType) = T.name, mainly to be used as a replacement for datatype_name

JeffBezanson · 2017-01-24T23:28:05Z

base/inference.jl

+            elseif isa(argtypes[2], PartialTypeVar)
+                ptv = argtypes[2]
+                tv = ptv.tv
+                canconst = false


Does this depend on lb_certain and ub_certain?

No. Two invocations of this functions do not create egal types (because it allocates a new one every time). I though we could do something with that at first, but I had to take that out because of that.

Keno · 2017-01-25T20:46:19Z

I've addressed some of the review here. @JeffBezanson @vtjnash, I'd appreciate if you could finish this up, since I'll be traveling the rest of this week and this is required for ColorTypes.

tkelman · 2017-01-30T20:23:57Z

base/inference.jl

+        t = argtypes[2]
+        if isa(t, Const) || isType(t)
+            return typename_static(t)
+         end


indent is a bit off

So it is. Sorry, I'm used to sublime doing this all automatically for me, but I've been trying to use Atom more. Will fix

Keno · 2017-01-30T20:42:11Z

Review comments addressed.

JeffBezanson · 2017-01-30T21:26:39Z

base/inference.jl

+    tv::TypeVar
+    lb_certain::Bool
+    ub_certain::Bool
+    PartialTypeVar(tv::TypeVar, lb_certain::ANY, ub_certain::ANY) = new(tv, lb_certain, ub_certain)


Why use ANY here if the arguments have to be Bool?

No reason. Will change.

JeffBezanson · 2017-01-30T21:31:28Z

base/inference.jl

+function _typename(a::Union)
+    ta = _typename(a.a)
+    tb = _typename(a.b)
+    ta == tb ? tb : (ta === Any || tb == Any) ? Any : Union{}


Should this use ta === tb? (also tb === Any for consistency)

No, I don't think it should since Const(T.name) != Const(T.name). tb === Any is fine though.

JeffBezanson · 2017-01-30T21:36:49Z

test/inference.jl

+
+fComplicatedUnionAll{T}(::Type{T}) = Type{Tuple{S,rand() >= 0.5 ? Int : Float64}} where S <: T
+let pub = Base.parameter_upper_bound, x = fComplicatedUnionAll(Real)
+    @test pub(pub(x, 1), 1) == Real


Do these belong in the reflection tests instead?

I put them here, because at some point I had inference mess up and constant fold this to the wrong answer.

This is informed by attempting to port some packages to the new type system. I have abstracted out the functionality required by those packages into documented/tested functions. The hope is to avoid introducing too many implementation details to packages. These functions can also be implemented in Compat for older julia versions, such that packages can immediate replace their use of internal APIs with these functions.

Because of the way methods are specified, there is often spurious ambiguities due to a type parameter being able to take the value Union{}, (e.g. Type{T} becomes Type{Union{}}). Since detect_ambiguities reports these, it can drown out more serious ambiguities among non-Union{} types. Add a keyword argument to detect_ambiguities, that ignores all ambiguities that only occur if one of the type parameters has to be Union{}.

Make inference be able to infer the type constructor. This is a little tricky, since we don't really have a good way to represent this. It's not quite `Const(TypeVar(:T,lb,ub))`, because a) the bounds may only be accurate up to type equality and b) TypeVar is not `isbits`, so it's not actually egal to the value we'll have at runtime. Additionally Type{T} already has meaning as a partially constructed type (e.g. an unwrapped UnionAll), so using T::Type{T} runs the risk of confusing this with an unwrapped type. Instead, introduce a new Const-like type, `PartialTypeVar`, which carries the type var and also keeps track of whether the bounds were egal or only typequal (we don't take advantage of that yet, but we could in the future). Additionally, improve the inference of `typename` and allow constant folding of field accesses on TypeName and SimpleVectors (to be able to constant fold T.parameters[1]).

for DataType/TypeName

tkelman · 2017-02-01T19:46:24Z

base/inference.jl

+const TypeName_wrapper_fieldindex = fieldindex(TypeName, :wrapper)
+
+function const_datatype_getfield_tfunc(sv, fld)
+  if (fld == DataType_name_fieldindex ||


wrong indent, again

See JuliaLang/julia#20006

tkelman reviewed Jan 13, 2017

View reviewed changes

Keno force-pushed the kf/newreflection branch from e5e3164 to 8c58732 Compare January 13, 2017 00:25

JeffBezanson force-pushed the jb/subtype branch from c9103ad to bb7c07b Compare January 13, 2017 01:37

JeffBezanson force-pushed the jb/subtype branch from bb7c07b to cff3cd6 Compare January 13, 2017 06:09

kshyatt added the domain:types and dispatch Types, subtyping and method dispatch label Jan 13, 2017

Keno force-pushed the kf/newreflection branch from 8c58732 to eecbf66 Compare January 13, 2017 21:59

Keno added a commit to Keno/Compat.jl that referenced this pull request Jan 13, 2017

Add Compat for TypeUtils

08972f1

See JuliaLang/julia#20006

Keno mentioned this pull request Jan 13, 2017

Add Compat for TypeUtils JuliaLang/Compat.jl#304

Closed

JeffBezanson force-pushed the jb/subtype branch from 7d2692f to 697c2ed Compare January 13, 2017 23:38

vtjnash reviewed Jan 15, 2017

View reviewed changes

Keno force-pushed the jb/subtype branch from 92535d0 to 3dc1419 Compare January 15, 2017 03:21

JeffBezanson force-pushed the jb/subtype branch 3 times, most recently from e95c0f8 to 7ae0bd2 Compare January 16, 2017 02:42

tkelman changed the base branch from jb/subtype to master January 16, 2017 17:25

tkelman mentioned this pull request Jan 20, 2017

Subtype relationships not detected with certain cases of Tuple{T,T} #20130

Open

Keno force-pushed the kf/newreflection branch from eecbf66 to 7d3c9de Compare January 22, 2017 03:27

JeffBezanson reviewed Jan 24, 2017

View reviewed changes

Keno added this to the 0.6.0 milestone Jan 25, 2017

Keno force-pushed the kf/newreflection branch from b4228f7 to 21a751f Compare January 25, 2017 20:45

Keno force-pushed the kf/newreflection branch from 21a751f to 1c6e437 Compare January 30, 2017 19:58

tkelman reviewed Jan 30, 2017

View reviewed changes

Keno force-pushed the kf/newreflection branch from 1c6e437 to 49af99f Compare January 30, 2017 20:41

JeffBezanson reviewed Jan 30, 2017

View reviewed changes

yuyichao mentioned this pull request Jan 30, 2017

Allow UnionAll as input to subtypes and include UnionAll in its output #20307

Merged

JeffBezanson reviewed Jan 30, 2017

View reviewed changes

Keno force-pushed the kf/newreflection branch 2 times, most recently from a0187ee to 68dcd5c Compare January 31, 2017 00:17

lobingera mentioned this pull request Jan 31, 2017

v0.6 precompile error: new UnionAll type breaks things JuliaGraphics/ColorTypes.jl#65

Closed

Keno force-pushed the kf/newreflection branch from 68dcd5c to 3a79b5b Compare January 31, 2017 19:51

Keno added 4 commits January 31, 2017 23:11

Don't constant-fold non-constant fields

b84483f

for DataType/TypeName

Keno force-pushed the kf/newreflection branch from 3a79b5b to b84483f Compare February 1, 2017 04:59

Merge branch 'master' into kf/newreflection

9db934c

Keno merged commit e0ab6a8 into master Feb 1, 2017

ararslan deleted the kf/newreflection branch February 1, 2017 19:44

tkelman reviewed Feb 1, 2017

View reviewed changes

Keno added a commit to Keno/Compat.jl that referenced this pull request Feb 1, 2017

Add Compat for TypeUtils

b7bd536

See JuliaLang/julia#20006

Keno added a commit to Keno/Compat.jl that referenced this pull request Feb 1, 2017

Add Compat for TypeUtils

5f945ff

See JuliaLang/julia#20006

Keno added a commit to Keno/Compat.jl that referenced this pull request Feb 2, 2017

Add Compat for TypeUtils

e98b9a2

See JuliaLang/julia#20006

stevengj mentioned this pull request Feb 3, 2017

add isimmutabletype #18168

Closed

timholy pushed a commit to JuliaLang/Compat.jl that referenced this pull request Feb 4, 2017

Add Compat for TypeUtils

39ca6b7

See JuliaLang/julia#20006

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a few new reflection methods (after type system merge) #20006

Add a few new reflection methods (after type system merge) #20006

Keno commented Jan 12, 2017

Keno commented Jan 12, 2017

JeffBezanson commented Jan 13, 2017

tkelman Jan 13, 2017

tkelman Jan 13, 2017

ararslan commented Jan 13, 2017 •

edited

Loading

Keno commented Jan 13, 2017

Keno commented Jan 13, 2017

JeffBezanson commented Jan 13, 2017

Keno commented Jan 13, 2017

vtjnash Jan 15, 2017

Keno Jan 15, 2017

vtjnash Jan 15, 2017

Keno Jan 15, 2017

vtjnash Jan 15, 2017

Keno Jan 15, 2017

vtjnash Jan 15, 2017

Keno commented Jan 22, 2017

vtjnash commented Jan 22, 2017

Keno commented Jan 22, 2017

Keno commented Jan 22, 2017

vtjnash commented Jan 22, 2017

JeffBezanson Jan 24, 2017

Keno Jan 25, 2017

Keno commented Jan 25, 2017

tkelman Jan 30, 2017

Keno Jan 30, 2017

Keno commented Jan 30, 2017

JeffBezanson Jan 30, 2017

Keno Jan 30, 2017

JeffBezanson Jan 30, 2017

Keno Jan 30, 2017 •

edited

Loading

JeffBezanson Jan 30, 2017

Keno Jan 30, 2017

tkelman Feb 1, 2017

Add a few new reflection methods (after type system merge) #20006

Add a few new reflection methods (after type system merge) #20006

Conversation

Keno commented Jan 12, 2017

Keno commented Jan 12, 2017

JeffBezanson commented Jan 13, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ararslan commented Jan 13, 2017 • edited Loading

Keno commented Jan 13, 2017

Keno commented Jan 13, 2017

JeffBezanson commented Jan 13, 2017

Keno commented Jan 13, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Keno commented Jan 22, 2017

vtjnash commented Jan 22, 2017

Keno commented Jan 22, 2017

Keno commented Jan 22, 2017

vtjnash commented Jan 22, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Keno commented Jan 25, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Keno commented Jan 30, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Keno Jan 30, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ararslan commented Jan 13, 2017 •

edited

Loading

Keno Jan 30, 2017 •

edited

Loading