Use Base.CoreLogging #78
Conversation
| @@ -1,6 +1,7 @@ | |||
| using Compat | |||
|
|
|||
| include(joinpath(@__DIR__, "..", "src", "util", "logging.jl")) | |||
| using Logging | |||
| parse(Bool, get(ENV, "DEBUG", "false")) && global_logger(SimpleLogger(global_logger().stream, Logging.Debug)) | |||
There was a problem hiding this comment.
Right, there's not a system for configuration yet, but what you're doing here is a very common pattern for this kind of thing which needs some kind of support. Actually I'm not sure I'll even have time for doing something nice for configuration in 1.0 - there's still a lot of other arguably-breaking-but-probably-not-feature-frozen things which need straightening out in Base, while configuration can be done in a package.
There was a problem hiding this comment.
Sure, I can work with an env flag and global logger for now.
| if after_exit[] | ||
| raw_print(msg...) | ||
| else | ||
| print_with_color(color, io, msg...) |
There was a problem hiding this comment.
Hmm, the current SimpleLogger doesn't support these kind of tricks I'm afraid. Though perhaps it should if it's to be useful in debugging Base.
It's a bit of a tricky issue: it would be nice if general users should expect to be able to install their own AbstractLoggers, but it's not reasonable to expect them to have a fallback path with raw IO.
Maybe the trick is to have a fallback logger in Base (a simplified SimpleLogger, perhaps) which is somehow protected and reinstalled "after" exit, and falls back to ccall somewhat like you have here.
There was a problem hiding this comment.
Given how plain print is now broken in this case too (ie. you can't print from finalizers), I wouldn't worry about high-level logging not supporting that scenario either. Might look into it when I have another finalizer issue to debug 🙂
| @@ -24,7 +24,7 @@ Context() do ctx | |||
| @test typeof(LLVM.ref(typ)) == LLVM.API.LLVMTypeRef # untyped | |||
|
|
|||
| @test typeof(LLVM.IntegerType(LLVM.ref(typ))) == LLVM.IntegerType # type reconstructed | |||
| if LLVM.DEBUG | |||
| @debug begin # TODO: does this correspond with the loglevel of the LLVM module? | |||
| @@ -9,11 +9,9 @@ identify(::Type{LLVMType}, ::Val{K}) where {K} = bug("Unknown type kind $K") | |||
|
|
|||
| @inline function check(::Type{T}, ref::API.LLVMTypeRef) where T<:LLVMType | |||
| ref==C_NULL && throw(NullException()) | |||
| @static if DEBUG | |||
| @debug begin | |||
There was a problem hiding this comment.
This seems like an odd use of @debug to contain an @assert.
Really @debug is for emitting verbose, low priority messages which may be ignored. The expression passed to @debug should evaluate to a human-readable message.
There was a problem hiding this comment.
Hmm, I wanted to avoid (DRY) another global DEBUG setting that determines whether costly checks are enabled, and (ab)using @debug seemed like a prime candidate for that. I see now how it still prints something.
|
Also, I didn't mean to ping you in order to get personalized comments; although I welcome them I'd be happy to wait until everything has settled a little 🙂 |
|
No problems, thanks for pinging me - having a lot of diverse use cases is really important for designing the right thing here I think. I learned something reading the tricks you had to pull here to debug finalizer issues. Hopefully somehow addressable in Base, we'll see. |
Use new logging infrastructure (supersede #78).
Testing out @c42f's new logging infrastructure. Maybe a little early, given how there doesn't seem to be a way to configure the system yet 🙂