type-stable PyError constructor (#1022)

* fix test_build to use PyCall's python

* type-stable PyError constructor

Author: stevengj

src/PyCall.jl

@@ -299,7 +299,7 @@ function _getproperty(o::PyObject, s::Union{AbstractString,Symbol})
     ispynull(o) && throw(ArgumentError("ref of NULL PyObject"))
     p = ccall((@pysym :PyObject_GetAttrString), PyPtr, (PyPtr, Cstring), o, s)
     if p == C_NULL && pyerr_occurred()
-        e = pyerror("PyObject_GetAttrString")
+        e = PyError("PyObject_GetAttrString")
         if PyPtr(e.T) != @pyglobalobjptr(:PyExc_AttributeError)
             throw(e)
         end
@@ -336,7 +336,7 @@ function _setproperty!(o::PyObject, s::Union{Symbol,AbstractString}, v)
     ispynull(o) && throw(ArgumentError("assign of NULL PyObject"))
     p = ccall((@pysym :PyObject_SetAttrString), Cint, (PyPtr, Cstring, PyPtr), o, s, PyObject(v))
     if p == -1 && pyerr_occurred()
-        e = pyerror("PyObject_SetAttrString")
+        e = PyError("PyObject_SetAttrString")
         if PyPtr(e.T) != @pyglobalobjptr(:PyExc_AttributeError)
             throw(e)
         end
@@ -507,7 +507,7 @@ function pyimport(name::AbstractString)
     o = _pyimport(name)
     if ispynull(o)
         if pyerr_occurred()
-            e = pyerror("PyImport_ImportModule")
+            e = PyError("PyImport_ImportModule")
             if pyisinstance(e.val, @pyglobalobjptr(:PyExc_ImportError))
                 # Expand message to help with common user confusions.
                 msg = """
@@ -553,7 +553,7 @@ or alternatively you can use the Conda package directly (via
 `using Conda` followed by `Conda.add` etcetera).
 """
                 end
-                e = pyerror(string(e.msg, "\n\n", msg, "\n"), e)
+                e = PyError(string(e.msg, "\n\n", msg, "\n"), e)
             end
             throw(e)
         else

src/exception.jl

@@ -49,6 +49,19 @@ function show(io::IO, e::PyError)
     end
 end
 
+# like pyerror(msg) but type-stable: always returns PyError
+function PyError(msg::AbstractString)
+    ptype, pvalue, ptraceback = Ref{PyPtr}(), Ref{PyPtr}(), Ref{PyPtr}()
+    # equivalent of passing C pointers &exc[1], &exc[2], &exc[3]:
+    ccall((@pysym :PyErr_Fetch), Cvoid, (Ref{PyPtr},Ref{PyPtr},Ref{PyPtr}), ptype, pvalue, ptraceback)
+    ccall((@pysym :PyErr_NormalizeException), Cvoid, (Ref{PyPtr},Ref{PyPtr},Ref{PyPtr}), ptype, pvalue, ptraceback)
+    return PyError(msg, PyObject(ptype[]), PyObject(pvalue[]), PyObject(ptraceback[]))
+end
+
+# replace the message from another error
+PyError(msg::AbstractString, e::PyError) =
+    PyError(msg, e.T, e.val, e.traceback)
+
 #########################################################################
 # Conversion of Python exceptions into Julia exceptions
 
@@ -106,17 +119,12 @@ macro pycheckz(ex)
     :(@pycheckv $(esc(ex)) -1)
 end
 
-function pyerror(msg::AbstractString)
-    ptype, pvalue, ptraceback = Ref{PyPtr}(), Ref{PyPtr}(), Ref{PyPtr}()
-    # equivalent of passing C pointers &exc[1], &exc[2], &exc[3]:
-    ccall((@pysym :PyErr_Fetch), Cvoid, (Ref{PyPtr},Ref{PyPtr},Ref{PyPtr}), ptype, pvalue, ptraceback)
-    ccall((@pysym :PyErr_NormalizeException), Cvoid, (Ref{PyPtr},Ref{PyPtr},Ref{PyPtr}), ptype, pvalue, ptraceback)
-    pyerror(msg, PyObject(ptype[]), PyObject(pvalue[]), PyObject(ptraceback[]))
-end
+# like PyError(...) but type-unstable: may unwrap a PyJlError
+# if one was thrown by a nested pyjlwrap function.
 
-function pyerror(msg::AbstractString, e::PyError)
+pyerror(msg::AbstractString) = pyerror(msg, PyError(msg))
+pyerror(msg::AbstractString, e::PyError) =
     pyerror(msg, e.T, e.val, e.traceback)
-end
 
 function pyerror(msg::AbstractString, ptype::PyObject, pvalue::PyObject, ptraceback::PyObject)
     pargs = _getproperty(pvalue, "args")

test/test_build.jl

@@ -4,11 +4,11 @@ include(joinpath(dirname(@__FILE__), "..", "deps", "depsutils.jl"))
 include(joinpath(dirname(@__FILE__), "..", "deps", "buildutils.jl"))
 
 using Test
+using PyCall: python
 
 @testset "find_libpython" begin
-    for python in ["python", "python2", "python3"]
-        # TODO: In Windows, word size should also be checked.
-        Sys.iswindows() && break
+    # TODO: In Windows, word size should also be checked.
+    if !Sys.iswindows()
         if Sys.which(python) === nothing
             @info "$python not available; skipping test"
         else
@@ -37,7 +37,7 @@ using Test
     # Test the case `dlopen` failed to open the library.
     let err, msg
         @test try
-            find_libpython("python"; _dlopen = (_...) -> error("dummy"))
+            find_libpython(python; _dlopen = (_...) -> error("dummy"))
             false
         catch err
             err isa ErrorException