From 84213fe0b024de98b7eb4a7c17ea7bc94c563cd0 Mon Sep 17 00:00:00 2001 From: Dheepak Krishnamurthy Date: Sat, 8 May 2021 11:10:09 -0600 Subject: [PATCH 1/7] Fix error message because of macro --- src/api.jl | 14 +++++++------- src/helper.jl | 4 ++++ src/utils.jl | 7 ++++++- 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/src/api.jl b/src/api.jl index 3c83a74b..a323a653 100644 --- a/src/api.jl +++ b/src/api.jl @@ -73,7 +73,7 @@ Initializes the Optimizer library. This must be called before any other library """ function init() - @checked Lib.XPRSinit(C_NULL) + Lib.XPRSinit(C_NULL) end """ @@ -83,7 +83,7 @@ Frees any allocated memory and closes all open files. """ function free() - @checked Lib.XPRSfree() + Lib.XPRSfree() end """ @@ -125,7 +125,7 @@ You can use this function to disable some of the checking and validation of func """ function setcheckedmode(checked_mode) - @checked Lib.XPRSsetcheckedmode(checked_mode) + Lib.XPRSsetcheckedmode(checked_mode) end """ @@ -135,7 +135,7 @@ You can use this function to interrogate whether checking and validation of all """ function getcheckedmode(r_checked_mode) - @checked Lib.XPRSgetcheckedmode(r_checked_mode) + Lib.XPRSgetcheckedmode(r_checked_mode) end function license(lic, path) @@ -143,11 +143,11 @@ function license(lic, path) end function beginlicensing(r_dontAlreadyHaveLicense) - @checked Lib.XPRSbeginlicensing(r_dontAlreadyHaveLicense) + Lib.XPRSbeginlicensing(r_dontAlreadyHaveLicense) end function endlicensing() - @checked Lib.XPRSendlicensing() + Lib.XPRSendlicensing() end """ @@ -165,7 +165,7 @@ function getlicerrmsg(; len = 1024) msg = Cstring(pointer(Array{Cchar}(undef, len*8))) Lib.XPRSgetlicerrmsg(msg, len) # TODO - @ checked version does not work - # @checked Lib.XPRSgetlicerrmsg(msg, len) + # Lib.XPRSgetlicerrmsg(msg, len) return unsafe_string(msg) end diff --git a/src/helper.jl b/src/helper.jl index 1394ec75..e887f784 100644 --- a/src/helper.jl +++ b/src/helper.jl @@ -62,6 +62,10 @@ mutable struct XpressProblem <: CWrapper end end +function get_xpress_error_message(prob::XpressProblem) + e = lstrip(Xpress.getlasterror(prob), ['?']) +end + function XpressProblem(; logfile = "") ref = Ref{Lib.XPRSprob}() createprob(ref) diff --git a/src/utils.jl b/src/utils.jl index 3969c6d4..e4e23586 100644 --- a/src/utils.jl +++ b/src/utils.jl @@ -88,15 +88,20 @@ macro invoke(expr) return f end +function get_xpress_error_message(xprs_ptr) + e = "(Unable to extract error message for $(typeof(xprs_ptr)).)" +end macro checked(expr) @assert expr.head == :call "Can only use @checked on function calls" @assert ( expr.args[1].head == :(.) ) && ( expr.args[1].args[1] == :Lib) "Can only use @checked on Lib.\$function" + @assert length(expr.args) >= 2 "Lib.\$function must be contain atleast one argument and the first argument must be of type XpressProblem" f = replace(String(expr.args[1].args[2].value), "XPRS" => "") return esc(quote r = $(expr) if r != 0 - e = lstrip(Xpress.getlasterror(prob), ['?']) + xprs_ptr = $(expr.args[2]) + e = get_xpress_error_message(xprs_ptr) throw(XpressError(r, "Unable to call `Xpress.$($f)`:\n\n$e.\n")) else nothing From 926970edea0edf83ea8e2a4965f3da4d26bcf290 Mon Sep 17 00:00:00 2001 From: Dheepak Krishnamurthy Date: Sat, 8 May 2021 11:36:15 -0600 Subject: [PATCH 2/7] Better error message --- src/api.jl | 4 +++- test/runtests.jl | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/api.jl b/src/api.jl index a323a653..461c182c 100644 --- a/src/api.jl +++ b/src/api.jl @@ -2787,7 +2787,9 @@ Returns the error message corresponding to the last error encountered by a libra """ function getlasterror(prob::XpressProblem) - @invoke Lib.XPRSgetlasterror(prob, _)::String + out = Cstring(pointer(Array{Cchar}(undef, 512))) + s = Lib.XPRSgetlasterror(prob, out) + s == 0 ? unsafe_string(out) : "Unable to get last error" end """ diff --git a/test/runtests.jl b/test/runtests.jl index 5a4005c5..c57346a6 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -16,4 +16,5 @@ end @test Xpress.getcontrol(prob, "HEURTHREADS") == 0 @test Xpress.getcontrol(prob, :HEURTHREADS) == 0 + @test_throws Xpress.XpressError Xpress.copyprob(prob, prob) end From 853c57fd9a3973630d901a19bf8fe6e50cee46e0 Mon Sep 17 00:00:00 2001 From: Dheepak Krishnamurthy Date: Wed, 19 May 2021 21:05:01 -0600 Subject: [PATCH 3/7] Update tests and documentation --- src/utils.jl | 30 ++++++++++++++++++++++++++++++ test/runtests.jl | 3 ++- 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/src/utils.jl b/src/utils.jl index e4e23586..4d8d64ea 100644 --- a/src/utils.jl +++ b/src/utils.jl @@ -92,6 +92,36 @@ function get_xpress_error_message(xprs_ptr) e = "(Unable to extract error message for $(typeof(xprs_ptr)).)" end +""" + @checked f(prob) + +Lets you invoke a lower level `Lib` function and check that Xpress does not error. +Use this macro to minimize repetition and increase readability. + +The first argument must be a object that can be cast into an Xpress pointer, e.g. `Ptr{XpressProblem}`. +This is passed to `get_xpress_error_message(xprs_ptr)` to get the error message. + +Examples: + + @checked Lib.XPRSsetprobname(prob, name) + +As an example of what @checked expands to: + +``` +julia> @macroexpand @checked Lib.XPRSsetprobname(prob, name) +quote + r = Lib.XPRSsetprobname(prob, name) + if r != 0 + xprs_ptr = prob + e = get_xpress_error_message(xprs_ptr) + throw(XpressError(r, "Unable to call `Xpress.setprobname`:\n\n\$(e).\n")) + else + nothing + end +end +``` + +""" macro checked(expr) @assert expr.head == :call "Can only use @checked on function calls" @assert ( expr.args[1].head == :(.) ) && ( expr.args[1].args[1] == :Lib) "Can only use @checked on Lib.\$function" diff --git a/test/runtests.jl b/test/runtests.jl index c57346a6..10e6b512 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -16,5 +16,6 @@ end @test Xpress.getcontrol(prob, "HEURTHREADS") == 0 @test Xpress.getcontrol(prob, :HEURTHREADS) == 0 - @test_throws Xpress.XpressError Xpress.copyprob(prob, prob) + msg = "Unable to call `Xpress.copyprob`:\n\n91 Error: No problem has been input.\n" + @test_throws Xpress.XpressError(32,msg) Xpress.copyprob(prob, prob) end From 8833da4ddfbb87ebade64a71fcf1939ba9a04a97 Mon Sep 17 00:00:00 2001 From: Dheepak Krishnamurthy Date: Wed, 19 May 2021 22:28:10 -0600 Subject: [PATCH 4/7] Fix potential GC issue --- src/api.jl | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/src/api.jl b/src/api.jl index 461c182c..b3bf56df 100644 --- a/src/api.jl +++ b/src/api.jl @@ -163,10 +163,12 @@ Retrieves an error message describing the last licensing error, if any occurred. """ function getlicerrmsg(; len = 1024) msg = Cstring(pointer(Array{Cchar}(undef, len*8))) - Lib.XPRSgetlicerrmsg(msg, len) - # TODO - @ checked version does not work - # Lib.XPRSgetlicerrmsg(msg, len) - return unsafe_string(msg) + buffer = Array{Cchar}(undef, len*8) + buffer_p = pointer(buffer) + GC.@preserve buffer begin + Lib.XPRSgetlicerrmsg(msg, len) + return unsafe_string(msg) + end end """ @@ -2787,10 +2789,14 @@ Returns the error message corresponding to the last error encountered by a libra """ function getlasterror(prob::XpressProblem) - out = Cstring(pointer(Array{Cchar}(undef, 512))) - s = Lib.XPRSgetlasterror(prob, out) - s == 0 ? unsafe_string(out) : "Unable to get last error" -end + @invoke Lib.XPRSgetlasterror(prob, _)::String + buffer = Array{Cchar}(undef, 512) + buffer_p = pointer(buffer) + GC.@preserve buffer begin + s = Lib.XPRSgetlasterror(prob, buffer_p) + return s == 0 ? unsafe_string(buffer_p) : "Unable to get last error" + end + end """ int XPRS_CC XPRSbasiscondition(XPRSprob prob, double *condnum, double *scondnum); From d3fe3421dc420bfea015e389dbd03cb34cf05de9 Mon Sep 17 00:00:00 2001 From: Dheepak Krishnamurthy Date: Wed, 19 May 2021 22:42:50 -0600 Subject: [PATCH 5/7] Fix other potential GC issues --- src/api.jl | 1 - src/license.jl | 7 +++++-- src/utils.jl | 16 +++++++++------- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/api.jl b/src/api.jl index b3bf56df..b70ab5e0 100644 --- a/src/api.jl +++ b/src/api.jl @@ -162,7 +162,6 @@ Retrieves an error message describing the last licensing error, if any occurred. """ function getlicerrmsg(; len = 1024) - msg = Cstring(pointer(Array{Cchar}(undef, len*8))) buffer = Array{Cchar}(undef, len*8) buffer_p = pointer(buffer) GC.@preserve buffer begin diff --git a/src/license.jl b/src/license.jl index 994b98a9..82a41642 100644 --- a/src/license.jl +++ b/src/license.jl @@ -74,8 +74,11 @@ function userlic(; verbose::Bool = true, liccheck::Function = emptyliccheck, xpa lic = liccheck(lic) # Send GIVEN LIC to XPRESS lib - slicmsg = Cstring(pointer(Array{Cchar}(undef, 1024*8))) - ierr = license(lic, slicmsg) + buffer = Array{Cchar}(undef, 1024*8) + buffer_p = pointer(buffer) + ierr = GC.@preserve buffer begin + license(lic, Cstring(buffer_p)) + end # check LIC TYPE if ierr == 16 diff --git a/src/utils.jl b/src/utils.jl index 4d8d64ea..2dcd1ceb 100644 --- a/src/utils.jl +++ b/src/utils.jl @@ -19,13 +19,15 @@ function invoke(f::Function, pos::Int, ::Type{Int}, args...) end function invoke(f::Function, pos::Int, ::Type{String}, args...) - out = Cstring(pointer(Array{Cchar}(undef, 1024))) - - args = collect(Any, args) - insert!(args, pos-1, out) - - r = f(args...) - r != 0 ? throw(XpressError(r, "Unable to invoke $f")) : unsafe_string(out) + buffer = Array{Cchar}(undef, 1024) + buffer_p = pointer(buffer) + GC.@preserve buffer begin + out = Cstring(buffer_p) + args = collect(Any, args) + insert!(args, pos-1, out) + r = f(args...) + r != 0 ? throw(XpressError(r, "Unable to invoke $f")) : unsafe_string(out) + end end """ From 047844cc397daec4b2effa532fe408d5f20638e0 Mon Sep 17 00:00:00 2001 From: Dheepak Krishnamurthy Date: Wed, 19 May 2021 22:44:50 -0600 Subject: [PATCH 6/7] Use explicit return in invoke for clarity --- src/utils.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils.jl b/src/utils.jl index 2dcd1ceb..723e899d 100644 --- a/src/utils.jl +++ b/src/utils.jl @@ -26,7 +26,7 @@ function invoke(f::Function, pos::Int, ::Type{String}, args...) args = collect(Any, args) insert!(args, pos-1, out) r = f(args...) - r != 0 ? throw(XpressError(r, "Unable to invoke $f")) : unsafe_string(out) + return r != 0 ? throw(XpressError(r, "Unable to invoke $f")) : unsafe_string(out) end end From 3d067d672e10a0cc7372809dcc28b4afbc196807 Mon Sep 17 00:00:00 2001 From: Dheepak Krishnamurthy Date: Wed, 19 May 2021 22:45:52 -0600 Subject: [PATCH 7/7] Remove unused variable --- src/helper.jl | 2 +- src/utils.jl | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/helper.jl b/src/helper.jl index e887f784..d6556c93 100644 --- a/src/helper.jl +++ b/src/helper.jl @@ -63,7 +63,7 @@ mutable struct XpressProblem <: CWrapper end function get_xpress_error_message(prob::XpressProblem) - e = lstrip(Xpress.getlasterror(prob), ['?']) + lstrip(Xpress.getlasterror(prob), ['?']) end function XpressProblem(; logfile = "") diff --git a/src/utils.jl b/src/utils.jl index 723e899d..bc0ab024 100644 --- a/src/utils.jl +++ b/src/utils.jl @@ -91,7 +91,7 @@ macro invoke(expr) end function get_xpress_error_message(xprs_ptr) - e = "(Unable to extract error message for $(typeof(xprs_ptr)).)" + "(Unable to extract error message for $(typeof(xprs_ptr)).)" end """