Skip to content
This repository was archived by the owner on May 4, 2019. It is now read-only.

Fix breakage on 0.5-dev. #173

Merged
merged 1 commit into from
Nov 1, 2015
Merged

Fix breakage on 0.5-dev. #173

merged 1 commit into from
Nov 1, 2015

Conversation

yuyichao
Copy link
Contributor

I'm not sure how much of these can go to Compat.jl so I just implement everything here....

c.c. @timholy

Ref JuliaLang/julia#13803

@timholy
Copy link
Contributor

timholy commented Oct 30, 2015

I'm not sure those are material for Compat anyway since they were unexported functions. But if others feel differently, I'm fine with wherever they end up.

@yuyichao
Copy link
Contributor Author

Oh SegFault, great .....

Also note that if we backport JuliaLang/julia#13803, the version check in this PR will need to be tweaked....

@yuyichao
Copy link
Contributor Author

Hmm. I couldn't reproduce the segfault but I'm getting some very strange errors in reducedim.jl test.

Depending on what I do before, I get either

ERROR: LoadError: UndefVarError: N not defined
 in _mapreducedim! at ./no file:4294967295

or

ERROR: LoadError: MethodError: `_nextract` has no method matching _nextract(::Type{Any}, ::Symbol, ::Expr)
Closest candidates are:
  _nextract(::Int64, ::Symbol, ::Expr)
  _nextract(::Int64, ::Symbol, ::Symbol)
 in func_for_method_checked at ./reflection.jl:248

when I try to call @code_warntype Base._mapreducedim!(+, Base.AndFun(), Bool[], BitArray(10))

This only happens if the module is precompiled but not if I add --compilecache=no or define the corresponding method at runtime again.

Can anyone reproduce this?

@vtjnash The method that is causing this issue directly is here. Is there anything in it that looks suspicious for you in terms of precompilation compatibility? Also, is there a way I can check the .ji file and see if there's anything strange in there?

@yuyichao
Copy link
Contributor Author

The first error UndefVarError(:N) is what I get when running the test locally without --compilecache=no.

@yuyichao
Copy link
Contributor Author

Force pushed to trigger a travis rerun.

@yuyichao
Copy link
Contributor Author

.... this time the error is the same one I see locally .....

https://travis-ci.org/JuliaStats/DataArrays.jl/jobs/88360029

yuyichao referenced this pull request in JuliaLang/julia Oct 30, 2015
…is indempotent from the perspective of codegen
@yuyichao
Copy link
Contributor Author

The precompilation error is caused by JuliaLang/julia#13677 ....

type_pow
promote_eltype, broadcast_shape, eltype_plus

if VERSION < v"0.5.0-dev+1016"
Copy link
Contributor

Choose a reason for hiding this comment

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

can you use an isdefined check here rather than a version comparison?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, will do when I'm back.

@yuyichao
Copy link
Contributor Author

Updated the check to use isdefined so that it should work on 0.4 if we end up backporting JuliaLang/julia#13803 .

The nightly CI will fail until commit JuliaLang/julia@5498570 is included in the nightly binary (hopefully tomorrow...)

@tkelman
Copy link
Contributor

tkelman commented Oct 30, 2015

should be up already: http://buildbot.e.ip.saba.us:8010/builders/package_tarball64/builds/144, hit restart if the versioninfo() looks too old

@yuyichao
Copy link
Contributor Author

You are right, the travis run fetched exactly the commit that has the fix. This should be ready to go then.

@tkelman
Copy link
Contributor

tkelman commented Nov 1, 2015

@simonster et al, okay to merge this?

simonster added a commit that referenced this pull request Nov 1, 2015
@simonster simonster merged commit 44192cf into JuliaStats:master Nov 1, 2015
@yuyichao yuyichao deleted the 0.5-dev branch November 1, 2015 15:37
@tkelman
Copy link
Contributor

tkelman commented Nov 7, 2015

Note that this needs to be tagged before JuliaLang/julia#13803 can be considered for backporting. Not urgent as it's almost certainly going to be for 0.4.2, not 0.4.1.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants