From 13196ae7c8984336a32cee295d4bc69151235d08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lars=20G=C3=B6ttgens?= Date: Tue, 14 May 2024 12:36:12 +0200 Subject: [PATCH 1/7] Small refactoring and input sanitation --- src/macros.jl | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/src/macros.jl b/src/macros.jl index f9395bb8f..ee793d14c 100644 --- a/src/macros.jl +++ b/src/macros.jl @@ -359,16 +359,32 @@ macro wrap(ex) fullargs = ex.args[2:length(ex.args)] - # strip type annotation from arguments for use in the call to GAP - args = [x isa Symbol ? x : x.args[1] for x in fullargs] - + # splits the arguments with type annotations into expressions for the lhs and rhs + # of the call of the form `func(args) = GAP.Globals.func(args)`, e.g. type annotations + # have to be removed for the rhs + tempargs = [ + begin + if x isa Symbol + (x, x) + elseif x.head == :(::) && length(x.args) == 2 + var = x.args[1] + typeannot = x.args[2] + (x, var) + else + error("unknown argument syntax around `$x`") + end + end for x in fullargs + ] + lhsargs = map(first, tempargs) + rhsargs = map(last, tempargs) + # the "outer" part of the body body = quote global $newsym if !isassigned($newsym) $newsym[] = GAP.Globals.$name::GapObj end - return $newsym[]($(args...))::$retval + return $newsym[]($(rhsargs...))::$retval end # insert the correct line number @@ -376,6 +392,6 @@ macro wrap(ex) return esc(quote @eval const $newsym = Ref{GapObj}() - Base.@__doc__ $ex = $body + Base.@__doc__ $(Expr(:call, name, lhsargs...)) = $body end) end From aded7a1032300490ed891882c6d9ef81ccbb45bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lars=20G=C3=B6ttgens?= Date: Tue, 14 May 2024 16:53:19 +0200 Subject: [PATCH 2/7] Adapt line number handling --- Project.toml | 2 +- src/macros.jl | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/Project.toml b/Project.toml index af6748c18..5ceaae64a 100644 --- a/Project.toml +++ b/Project.toml @@ -30,7 +30,7 @@ GAP_lib_jll = "~400.1300.000" GAP_pkg_juliainterface_jll = "=0.900.000" InteractiveUtils = "1.6" Libdl = "1.6" -MacroTools = "0.5" +MacroTools = "0.5.13" Markdown = "1.6" Ncurses_jll = "6.4.1" Pidfile = "1.3" diff --git a/src/macros.jl b/src/macros.jl index ee793d14c..a02d702f5 100644 --- a/src/macros.jl +++ b/src/macros.jl @@ -379,7 +379,7 @@ macro wrap(ex) rhsargs = map(last, tempargs) # the "outer" part of the body - body = quote + body = MacroTools.@qq begin global $newsym if !isassigned($newsym) $newsym[] = GAP.Globals.$name::GapObj @@ -387,10 +387,8 @@ macro wrap(ex) return $newsym[]($(rhsargs...))::$retval end - # insert the correct line number - body.args[1] = __source__ - return esc(quote + return esc(MacroTools.@qq begin @eval const $newsym = Ref{GapObj}() Base.@__doc__ $(Expr(:call, name, lhsargs...)) = $body end) From 3a080c5b2a4cacf2fa8c3de9e1ffd25611f71c88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lars=20G=C3=B6ttgens?= Date: Tue, 14 May 2024 16:26:28 +0200 Subject: [PATCH 3/7] Add special case for `::GapObj` --- src/macros.jl | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/src/macros.jl b/src/macros.jl index a02d702f5..61ea010fb 100644 --- a/src/macros.jl +++ b/src/macros.jl @@ -328,6 +328,9 @@ However, the generated function actually caches the GAP object `GAP.Globals.NAME This minimizes the call overhead. So @wrap typically is used to provide an optimized way to call certain GAP functions. +If an argument is annotated as `::GapObj`, the resulting function accepts arguments +of any type and wraps them in `GapObj(...)` before passing them to the GAP function. + Another use case for this macro is to improve type stability of code calling into GAP, via the type annotations for the arguments and return value contained in the function declaration. @@ -343,6 +346,12 @@ Jacobi (generic function with 1 method) julia> Jacobi(11,35) 1 + +julia> GAP.@wrap IsString(x::GapObj)::Bool +IsString (generic function with 1 method) + +julia> IsString("abc") +true ``` """ macro wrap(ex) @@ -360,8 +369,9 @@ macro wrap(ex) fullargs = ex.args[2:length(ex.args)] # splits the arguments with type annotations into expressions for the lhs and rhs - # of the call of the form `func(args) = GAP.Globals.func(args)`, e.g. type annotations - # have to be removed for the rhs + # of the call of the form `func(args) = GAP.Globals.func(args)`: + # - type annotations have to be removed for the rhs + # - if the type annotation is `GapObj`, the rhs has to be wrapped in `GAP.GapObj(...)` and the lhs has no type annotation tempargs = [ begin if x isa Symbol @@ -369,7 +379,11 @@ macro wrap(ex) elseif x.head == :(::) && length(x.args) == 2 var = x.args[1] typeannot = x.args[2] - (x, var) + if typeannot in [:GapObj, :(GAP.GapObj)] + (var, :(GAP.GapObj($var))) + else + (x, var) + end else error("unknown argument syntax around `$x`") end From be4252dc55456bbf6e3e058552068d854d9326fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lars=20G=C3=B6ttgens?= Date: Fri, 17 May 2024 15:52:25 +0200 Subject: [PATCH 4/7] Update src/macros.jl --- src/macros.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/macros.jl b/src/macros.jl index 61ea010fb..68f8bc1a4 100644 --- a/src/macros.jl +++ b/src/macros.jl @@ -380,7 +380,7 @@ macro wrap(ex) var = x.args[1] typeannot = x.args[2] if typeannot in [:GapObj, :(GAP.GapObj)] - (var, :(GAP.GapObj($var))) + (var, :(GAP.GapObj($var)::GAP.GapObj)) else (x, var) end From 861160c22241719706848267a79ded3d45c7f654 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lars=20G=C3=B6ttgens?= Date: Wed, 22 May 2024 09:44:19 +0200 Subject: [PATCH 5/7] Update src/macros.jl --- src/macros.jl | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/macros.jl b/src/macros.jl index 68f8bc1a4..3845561ad 100644 --- a/src/macros.jl +++ b/src/macros.jl @@ -381,6 +381,10 @@ macro wrap(ex) typeannot = x.args[2] if typeannot in [:GapObj, :(GAP.GapObj)] (var, :(GAP.GapObj($var)::GAP.GapObj)) + elseif typeannot == :(GAP.Obj) + (var, :(GAP.Obj($var)::GAP.Obj)) + elseif typeannot in [:GapInt, :(GAP.GapInt)] + (var, :(GAP.GapInt($var)::GAP.GapInt)) else (x, var) end From 85dba1bac0ec210f331f82d6f4574bd63c6dda08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lars=20G=C3=B6ttgens?= Date: Mon, 27 May 2024 16:20:48 +0200 Subject: [PATCH 6/7] Add some better comments --- src/macros.jl | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/macros.jl b/src/macros.jl index 3845561ad..4cb63b0c7 100644 --- a/src/macros.jl +++ b/src/macros.jl @@ -369,23 +369,27 @@ macro wrap(ex) fullargs = ex.args[2:length(ex.args)] # splits the arguments with type annotations into expressions for the lhs and rhs - # of the call of the form `func(args) = GAP.Globals.func(args)`: - # - type annotations have to be removed for the rhs - # - if the type annotation is `GapObj`, the rhs has to be wrapped in `GAP.GapObj(...)` and the lhs has no type annotation + # of the call of the form `func(args) = GAP.Globals.func(args)` (see below) tempargs = [ begin if x isa Symbol + # no type annotations -> use x for lhs and rhs (x, x) elseif x.head == :(::) && length(x.args) == 2 + # type annotations -> split and decide what to do var = x.args[1] typeannot = x.args[2] if typeannot in [:GapObj, :(GAP.GapObj)] + # the lhs has no type annotation, rhs gets wrapped in `GAP.GapObj(...)::GAP.GapObj` (var, :(GAP.GapObj($var)::GAP.GapObj)) elseif typeannot == :(GAP.Obj) + # the lhs has no type annotation, rhs gets wrapped in `GAP.Obj(...)::GAP.Obj` (var, :(GAP.Obj($var)::GAP.Obj)) elseif typeannot in [:GapInt, :(GAP.GapInt)] + # the lhs has no type annotation, rhs gets wrapped in `GAP.GapInt(...)::GAP.GapInt` (var, :(GAP.GapInt($var)::GAP.GapInt)) else + # remove type annotation on the rhs (x, var) end else From 88a9e3b4b78c208170087f1c2478cba877c00126 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lars=20G=C3=B6ttgens?= Date: Mon, 27 May 2024 17:57:21 +0200 Subject: [PATCH 7/7] Enhance test Co-authored-by: Max Horn --- src/macros.jl | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/macros.jl b/src/macros.jl index 4cb63b0c7..79d2c9833 100644 --- a/src/macros.jl +++ b/src/macros.jl @@ -341,17 +341,20 @@ reference the original GAP object. # Examples ```jldoctest +julia> GAP.@wrap IsString(x::GapObj)::Bool +IsString (generic function with 1 method) + +julia> IsString("abc") +true + julia> GAP.@wrap Jacobi(x::GapInt, y::GapInt)::Int Jacobi (generic function with 1 method) julia> Jacobi(11,35) 1 -julia> GAP.@wrap IsString(x::GapObj)::Bool -IsString (generic function with 1 method) - -julia> IsString("abc") -true +julia> Jacobi(big(35)^100+11, 35) +1 ``` """ macro wrap(ex)