Fix loading of precompiled JSON when DataStructures is not present #126
Conversation
without DataStructures present
| catch | ||
| false | ||
| function __init__() | ||
| global _HAVE_DATASTRUCTURES = try |
There was a problem hiding this comment.
I think Jameson hates it JuliaLang/julia#12010
There was a problem hiding this comment.
I don't see any harm in allowing this to be modified at runtime if someone really wants to. Would there be a performance impact?
There was a problem hiding this comment.
Well, globals are usually slower than constant globals because of the type instability... though maybe it doesn't matter here.
There was a problem hiding this comment.
It's only ever used as a boolean afaict - https://github.com/JuliaLang/JSON.jl/blob/0716a1ccc00e63461e03851387390fdcd43d0c35/src/Parser.jl#L322
There was a problem hiding this comment.
You could annotate all usages, e.g., _HAVE_DATASTRUCTURES::Bool.
There was a problem hiding this comment.
Even annotating them is not nearly as efficient as a const, last I checked the generated code. You could make it a const _HAVE_DATASTRUCTURES = [false].
Again, it could be that _HAVE_DATASTRUCTURES is not used in any performance-critical situations, so maybe we shouldn't worry.
There was a problem hiding this comment.
I'm going to merge this then, fix what's broken. Adjusting constness can be done separately if it makes a difference.
Fix loading of precompiled JSON when DataStructures is not present
closes #123
first commit adds a test to demonstrate failure case on travis, then second commit moves the conditional dependency on DataStructures into an
__init__function.