Improve @perm macro

src/Groups/perm.jl

@@ -740,7 +740,7 @@
 #
 # The following code implements a new way to input permutations in Julia. For example
 # it is possible to create a permutation as follow
-# pi = Oscar.Permutations.@perm (1,2,3)(4,5)(6,7,8)
+# pi = @perm (1,2,3)(4,5)(6,7,8)
 # > (1,2,3)(4,5)(6,7,8)
 # For this we use macros to modify the syntax tree of (1,2,3)(4,5)(6,7,8) such that
 # Julia can deal with the expression.
@@ -750,7 +750,14 @@
     ex == :( () ) && return []
     ex isa Expr || error("Input is not a permutation expression")
 
-    res = []
+    if ex.head == :tuple && any(x -> x isa Expr && x.head == :macrocall && x.args[1] == Symbol("@perm"), ex.args)
+        error("""Encountered @perm macro inside of permutation expression.
+                Either set explicit parentheses for each @perm call (e.g. `@perm((1,2)), @perm((3,4))`),
+                or use the @perm variant that takes a list of permutations (e.g. `@perm n [(1,2), (3,4)]`).
+                Please refer to the docstring of @perm for more information.""")
+    end
+
+    res = Expr[]
     while ex isa Expr && ex.head == :call
         push!(res, Expr(:vect, ex.args[2:end]...))
         ex = ex.args[1]
@@ -760,7 +767,7 @@
         error("Input is not a permutation.")
     end
 
-    push!(res, Expr(:vect,ex.args...))
+    push!(res, Expr(:vect, ex.args...))
 
     # reverse `res` to match the original order; this ensures
     # the evaluation order is as the user expects
@@ -772,19 +779,38 @@
 
 ################################################################################
 #
-#   perm
+#   @perm
 #
+
 @doc raw"""
-    @perm ex
-
-Input a permutation in cycle notation. Supports arbitrary expressions for
-generating the integer entries of the cycles. The parent group is inferred 
-to be the symmetric group with a degree of the highest integer referenced 
-in the permutation.
+    @perm expr
+    @perm n expr
+    @perm G expr
+
+Input a permutation or a non-empty list of permutations in cycle notation.
+
+Supports arbitrary expressions for generating the integer entries of the cycles.
+
+`expr` may either be a single permutation, a non-empty list of permutations,
+or a non-empty tuple of permutations. **Note:** `@perm ()` denotes the identity
+permutation, NOT the empty tuple of permutations.
+
+If a group `G` is provided, the permutations are created as elements of `G`. This will
+raise an error if the permutations are not elements of `G`.
+
+If an integer `n` is provided, the permutations are created as elements of the
+symmetric group of degree `n`, i.e., `symmetric_group(n)`.
+
+In the remaining case, the parent group is inferred to be the symmetric group
+with a degree of the highest integer referenced in `expr`. This may result in
+evaluating the expressions in the cycle entries multiple times, so it is
+recommended to provide the parent group explicitly in cases of complex expressions.
 
 The actual work is done by [`cperm`](@ref). Thus, for the time being,
 cycles which are *not* disjoint actually are supported.
 
+See also [`cperm`](@ref) and [`perm`](@ref) for other ways to create permutations.
+
 # Examples
 ```jldoctest
 julia> x = @perm (1,2,3)(4,5)(factorial(3),7,8)
@@ -793,39 +819,18 @@
 julia> parent(x)
 Sym(8)
 
-julia> y = cperm([1,2,3],[4,5],[6,7,8])
-(1,2,3)(4,5)(6,7,8)
-
-julia> x == y
+julia> x == @perm 8 (1,2,3)(4,5)(factorial(3),7,8)
 true
 
-julia> z = perm(symmetric_group(8),[2,3,1,5,4,7,8,6])
-(1,2,3)(4,5)(6,7,8)
+julia> x == cperm([1,2,3],[4,5],[6,7,8])
+true
 
-julia> x == z
+julia> x == perm(symmetric_group(8),[2,3,1,5,4,7,8,6])
 true
 ```
-"""
-macro perm(ex)
-    res = _perm_helper(ex)
-    return esc(:(Oscar.cperm($(res...))))
-end
-
-
-################################################################################
-#
-#   perm(n,gens)
-#
-@doc raw"""
-    @perm n gens
-
-Input a list of permutations in cycle notation, created as elements of the
-symmetric group of degree `n`, i.e., `symmetric_group(n)`, by invoking
-[`cperm`](@ref) suitably.
 
-# Examples
 ```jldoctest
-julia> gens = @perm 14 [
+julia> gens = @perm [
               (1,10)
               (2,11)
               (3,12)
@@ -851,19 +856,76 @@
 Sym(14)
 ```
 """
-macro perm(n,gens)
+macro perm(expr)
+  type, res = _perm_parse(expr)
+  n = _perm_max_entry(type, res)
+  return _perm_format(type, n, res)
+end
 
-    ores = Expr[]
-    for ex in gens.args
-        res = _perm_helper(ex)
-        push!(ores, esc(:(  [$(res...)]  )))
+macro perm(n_or_G, expr)
+  type, res = _perm_parse(expr)
+  return _perm_format(type, esc(n_or_G), res)
+end
+
+function _perm_parse(expr::Expr)
+  # case: expr is a non-empty vector
+  if expr.head == :vect || expr.head == :vcat
+    @req length(expr.args) > 0 "empty vector not allowed"
+    return Val(:vector), [esc(:([$(_perm_helper(arg)...)])) for arg in expr.args]
+  end
+
+  # case: expr is a non-empty tuple of permutations
+  # to distinguish this from a single cycle (of arbitrary expressions), we walk through
+  # the expression tree, and look for a place where a tuple is called.
+  # This never happens inside a single cycle, so this is a safe way to distinguish.
+  if expr.head == :tuple && length(expr.args) > 0 && expr.args[1] isa Expr
+    ex = expr.args[1]
+    while true
+      if ex.head == :tuple
+        return Val(:tuple), [esc(:([$(_perm_helper(arg)...)])) for arg in expr.args]
+      end
+      if ex.head == :call && ex.args[1] isa Expr
+        ex = ex.args[1]
+      else
+        break
+      end
     end
+  end
 
-    return quote
-       let g = symmetric_group($n)
-           [ cperm(g, pi...) for pi in [$(ores...)] ]
-       end
+  # otherwise, we have a single permutation
+  return Val(:single), esc.(_perm_helper(expr))
+end
+
+function _perm_max_entry(::Val{:single}, res)
+  return :(mapreduce(maximum, max, [$(res...)]; init=1))
+end
+
+function _perm_max_entry(::Union{Val{:vector}, Val{:tuple}}, res)
+  return :(mapreduce(x -> mapreduce(maximum, max, x; init=1), max, [$(res...)]; init=1))
+end
+
+function _perm_format(::Val{:single}, n_or_G, res)
+  return quote
+    let n = $(n_or_G), G = n isa Int ? symmetric_group(n) : n
+      cperm(G, $(res...))
     end
+  end
+end
+
+function _perm_format(::Val{:vector}, n_or_G, res)
+  return quote
+    let n = $(n_or_G), G = n isa Int ? symmetric_group(n) : n
+      [cperm(G, p...) for p in [$(res...)]]
+    end
+  end
+end
+
+function _perm_format(::Val{:tuple}, n_or_G, res)
+  return quote
+    let n = $(n_or_G), G = n isa Int ? symmetric_group(n) : n
+      ((cperm(G, p...) for p in [$(res...)])...,)
+    end
+  end
 end