grabbag of subtyping punchlist items #20407

vtjnash · 2017-02-02T19:26:29Z

From the list in #19998. Will check those off when this is merged.

tkelman · 2017-02-02T20:07:34Z

doc/src/manual/types.md

@@ -1045,6 +1045,10 @@ end
 Of course, this depends on what `Int` is aliased to -- but that is predefined to be the correct
 type -- either `Int32` or `Int64`.

+(Note that unlike `Int`, `Float` does not exist as a type-alias for a specific sized `AbstractFloat`.
+Unlike with integer registers, the size of the floating point representation is problem dependent,


Integer sizes often need to be problem dependent too, I'm not sure this is the right wording to justify this.

Hm. Maybe I should say something along the lines of "the floating point register sizes are standardized across CPUs based on the IEEE-754 standard."?

practically speaking I'd say that's more accurately the reason

tkelman · 2017-02-02T20:08:14Z

doc/src/stdlib/punctuation.md

@@ -42,3 +42,6 @@ Extended documentation for mathematical symbols & functions is [here](@ref math-
 | `::`        | type annotation, depending on context                                                       |
 | `:( )`      | quoted expression                                                                           |
 | `:a`        | symbol a                                                                                    |
+| `<:`        | [`subtype operator`](@ref <:)                                                               |
+| `>:`        | [`supertype operator`](@ref >:) (reverse of subtype operator)                               |
+| `===`       | [`eqal comparison operator`](@ref ===)                                                      |


eqal or egal?

tkelman · 2017-02-02T20:45:08Z

base/show.jl

+#The same as `IOContext(io::IO, KV::Pair)`, but accepting properties as keyword arguments.
+#"""
+#IOContext(io::IO; kws...) = IOContext(IOContext(io); kws...)
+#function IOContext(io::IOContext; kws...)


this might break Documenter, Juno, IJulia, SHA and others, is it possible to deprecate this?

Not easily. The problem is that it replaces the primitive IOContext(io) constructor (which would otherwise be inlined), with several kwarg calls. (since the kwarg method needs to be the same as the no-keyword args function)

So the commit message should say "remove keyword argument IOContext constructors". That's a pretty sneaky thing to combine with an implementation-only change.

Seriously – this is not non-breaking...

you mean sneaky like the commit message that added the alternate constructor? "use IOContext more consistently" (ce23174)

Unfortunately there's a ratchet effect. Adding a feature or method doesn't break anything, but removing one does.

Also, why does the performance of this constructor matter?

JeffBezanson · 2017-02-02T23:01:35Z

base/operators.jl

+
+Supertype operator, equivalent to `issubtype(T2, T1)`.
+"""
+const (>:)(a::ANY, b::ANY) = issubtype(b, a)


I believe this needs special inference support to be able to infer Const(true) or Const(false).

Probably doable, and perhaps not a bad idea anyways (so that it doesn't attempt to actually specialize on any arguments that it sees). I guess the idea is that it's preferable to ensure >: and <: behave the same. Should I make both of them (issupertype) builtins to keep it identical, or just make it close enough via inference?

I'd rather not add more builtins; let's just add it to inference.

JeffBezanson · 2017-02-03T02:44:54Z

base/show.jl

+#The same as `IOContext(io::IO, KV::Pair)`, but accepting properties as keyword arguments.
+#"""
+#IOContext(io::IO; kws...) = IOContext(IOContext(io); kws...)
+#function IOContext(io::IOContext; kws...)


So the commit message should say "remove keyword argument IOContext constructors". That's a pretty sneaky thing to combine with an implementation-only change.

...by removing it. This type can't be represented in Julia correctly, but it also isn't needed.

…d be correct

Since UnionAll bounds are encountered recursively, it's easiest to represent this the same way in the IOContext. Also avoid the performance / allocation overhead of using `IOContext(io)` as a no-op convert (not a no-op since keyword support was added)

the previous version was effectively equivalent, but did more work than necessary and could fail in the wrong direction in the unlikely case that `ambi` was not a strict subtype of `mambig->sig`.

Seems to be a common FAQ. Not sure if those users would have seen it here, but it's right after explaining the existance of the `Int` type-alias.

… declaration

…yping relationships

the general inliner is better and also respects effect_free (related to #9765, although none of these specific cases were mentioned there)

vtjnash · 2017-02-09T19:30:19Z

bump

JeffBezanson · 2017-02-09T19:57:33Z

test/show.jl

@@ -569,7 +569,7 @@ let repr = sprint(dump, Any)
    @test length(repr) > 100000
    @test ismatch(r"^Any\n  [^ \t\n]", repr)
    @test endswith(repr, '\n')
-    @test_broken contains(repr, "     Base.Vector{T} = Array{T,1}\n")
+    @test contains(repr, "     Base.Vector{T} = Array{T,1} where T\n")


It would be more accurate print either Base.Vector = Array{T,1} where T or Base.Vector{T} = Array{T,1}. That second syntax will be added soon.

JeffBezanson · 2017-02-09T21:11:34Z

LGTM.

KristofferC · 2017-02-09T21:17:36Z

@nanosoldier runbenchmarks(ALL, vs = ":master")

nanosoldier · 2017-02-10T02:34:48Z

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @jrevels

tkelman · 2017-02-11T02:07:22Z

the comprehension slowdown there looks like it could be real

ararslan added the types and dispatch Types, subtyping and method dispatch label Feb 2, 2017

tkelman reviewed Feb 2, 2017

View reviewed changes

JeffBezanson reviewed Feb 2, 2017

View reviewed changes

JeffBezanson requested changes Feb 3, 2017

View reviewed changes

vtjnash force-pushed the jn/grabbag-subtypes branch 2 times, most recently from 22534b4 to 90357c3 Compare February 6, 2017 23:32

vtjnash added 11 commits February 7, 2017 11:42

allow running tests in any random subdirectory, not just linalg

3217db6

fix offsetof(Task, ctx)

a73af21

...by removing it. This type can't be represented in Julia correctly, but it also isn't needed.

simplify / improve jl_typemap_intersection visitor now that isa shoul…

6cc099c

…d be correct

correct the ambiguity detection test in ml_matches

6cd36bc

the previous version was effectively equivalent, but did more work than necessary and could fail in the wrong direction in the unlikely case that `ambi` was not a strict subtype of `mambig->sig`.

remove unnecessary work from check_ambiguous_visitor

ec984b6

add parenthetical to manual regarding absence of Float typealias

209be03

Seems to be a common FAQ. Not sure if those users would have seen it here, but it's right after explaining the existance of the `Int` type-alias.

add supertype operator >: for symmetry with issubtype

19051e4

fix typetree dump to consider that a UnionAll may be the primary type…

ea55670

… declaration

fix typetree example to demonstrate new type representations and subt…

5fc582b

…yping relationships

remove several unnecessary special-case inliners

0c819fe

the general inliner is better and also respects effect_free (related to #9765, although none of these specific cases were mentioned there)

vtjnash force-pushed the jn/grabbag-subtypes branch from 90357c3 to 0c819fe Compare February 7, 2017 16:43

JeffBezanson reviewed Feb 9, 2017

View reviewed changes

use new "where" syntax in dumpsubtypes

dee86b3

JeffBezanson approved these changes Feb 9, 2017

View reviewed changes

Merge branch 'master' into jn/grabbag-subtypes

3159acc

vtjnash merged commit 6fa37f6 into master Feb 11, 2017

vtjnash deleted the jn/grabbag-subtypes branch February 11, 2017 01:53

felixrehren mentioned this pull request Feb 16, 2017

new subtyping issues #19998

Closed

53 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

grabbag of subtyping punchlist items #20407

grabbag of subtyping punchlist items #20407

vtjnash commented Feb 2, 2017

tkelman Feb 2, 2017

vtjnash Feb 2, 2017

tkelman Feb 2, 2017

tkelman Feb 2, 2017

tkelman Feb 2, 2017 •

edited

Loading

vtjnash Feb 2, 2017

JeffBezanson Feb 3, 2017

StefanKarpinski Feb 3, 2017

vtjnash Feb 3, 2017

JeffBezanson Feb 3, 2017

JeffBezanson Feb 3, 2017

JeffBezanson Feb 2, 2017

vtjnash Feb 3, 2017

JeffBezanson Feb 3, 2017

JeffBezanson Feb 3, 2017

vtjnash commented Feb 9, 2017

JeffBezanson Feb 9, 2017

vtjnash Feb 9, 2017

JeffBezanson commented Feb 9, 2017

KristofferC commented Feb 9, 2017

nanosoldier commented Feb 10, 2017

tkelman commented Feb 11, 2017

grabbag of subtyping punchlist items #20407

grabbag of subtyping punchlist items #20407

Conversation

vtjnash commented Feb 2, 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

tkelman Feb 2, 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

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

vtjnash commented Feb 9, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JeffBezanson commented Feb 9, 2017

KristofferC commented Feb 9, 2017

nanosoldier commented Feb 10, 2017

tkelman commented Feb 11, 2017

tkelman Feb 2, 2017 •

edited

Loading