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.

Sign up for GitHub

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

Jump to bottom

Fix #19655 (TypeError on empty stacktrace) #19656

Merged
merged 2 commits into from
Jan 4, 2017
Merged

Fix #19655 (TypeError on empty stacktrace) #19656

merged 2 commits into from
Jan 4, 2017

Conversation

iamed2
Copy link
Contributor

@iamed2 iamed2 commented Dec 19, 2016

The issue was that vcat(StackFrame[]...) gives Any[], not StackFrame[].

@iamed2
Copy link
Contributor Author

iamed2 commented Dec 19, 2016

This should probably be backported

@kshyatt kshyatt added backport pending 0.5 types and dispatch Types, subtyping and method dispatch labels Dec 19, 2016
vtjnash
vtjnash reviewed Dec 19, 2016
View reviewed changes
base/stacktraces.jl
@@ -144,7 +144,7 @@ doesn't return C functions, but this can be enabled.) When called without specif
trace, `stacktrace` first calls `backtrace`.
"""
function stacktrace(trace::Vector{Ptr{Void}}, c_funcs::Bool=false)
stack = vcat(map(lookup, trace)...)::StackTrace
stack = vcat(StackTrace(), map(lookup, trace)...)::StackTrace
Copy link
Member

Choose a reason for hiding this comment

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

Would: StackTrace[ lookup(x) for x in trace ] make more sense?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That wouldn't concatenate the results of lookup together.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is essentially a "flatten" operation.

@tkelman tkelman added the needs tests Unit tests are required for this change label Dec 19, 2016
@iamed2
Copy link
Contributor Author

iamed2 commented Dec 19, 2016

AppVeyor failure seems like an unrelated network issue.

@StefanKarpinski
Copy link
Member

Bump.

@iamed2
Copy link
Contributor Author

iamed2 commented Jan 3, 2017

Wrote a test but setting up a new computer so I have to wait for a Julia build.

@iamed2
Copy link
Contributor Author

iamed2 commented Jan 3, 2017

I don't understand what's happening with the CI here.

@tkelman
Copy link
Contributor

tkelman commented Jan 3, 2017

doc build failure from #19268

@tkelman
Copy link
Contributor

tkelman commented Jan 4, 2017

Ah but that was only on Travis, the AppVeyor failures look like the newly-added tests didn't work there.

@iamed2
Copy link
Contributor Author

iamed2 commented Jan 4, 2017

Dang; I decided to use Base.runtests instead of make test for the first time and forgot to rebuild before testing locally.

@tkelman tkelman removed the needs tests Unit tests are required for this change label Jan 4, 2017
tkelman
tkelman reviewed Jan 4, 2017
View reviewed changes
test/stacktraces.jl
let
# not in a `catch`, so should return an empty StackTrace
st = stacktrace(empty!(backtrace()))
println(st)
Copy link
Contributor

@tkelman tkelman Jan 4, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

should probably take this part out?
edit: or print to an IOBuffer and compare the contents to an expected string

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Dammit. Sorry.

No need to print and check, the tests below that check for all relevant information.

@tkelman
Copy link
Contributor

tkelman commented Jan 4, 2017

this is getting overly familiar: appveyor failure was #16555, travis 32 bit linux failure was #19803

@tkelman tkelman merged commit cfa2312 into JuliaLang:master Jan 4, 2017
@iamed2 iamed2 deleted the empty-stacktrace branch January 4, 2017 22:08
andyferris pushed a commit to andyferris/julia that referenced this pull request Jan 4, 2017
@iamed2
Fix JuliaLang#19655 (TypeError on empty stacktrace) (JuliaLang#19656)
b446919 
* Fix JuliaLang#19655

* Test for JuliaLang#19655
tkelman pushed a commit that referenced this pull request Mar 1, 2017
@iamed2 @tkelman
Fix #19655 (TypeError on empty stacktrace) (#19656)
73052e6 
* Fix #19655

* Test for #19655

(cherry picked from commit cfa2312)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tkelman tkelman tkelman left review comments

@vtjnash vtjnash vtjnash left review comments

Assignees
No one assigned
Labels
types and dispatch Types, subtyping and method dispatch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@iamed2 @StefanKarpinski @tkelman @vtjnash @kshyatt