rebcabin / masr

Meta ASR: replacement for aging ASDL
MIT License
4 stars 0 forks source link

`IntrinsicFunction` and `call-args` versus `call-arg` #39

Open rebcabin opened 1 year ago

rebcabin commented 1 year ago

I've had call-args as a vector of lists working with SubroutineCall and FunctionCall for a while (see Issue #32) . Here is the spec and an example lifted from the lpython .stdout reference outputs:

(s/def ::call-args (s/coll-of ::call-arg)) ;; <~~~ top level of list/vector
(s/def ::call-arg ;; <~~~ second level of list
  (s/coll-of ::expr?
             :min-count 1   ;; Issue 32
             :max-count 1))
(FunctionCall
  2 pow__AT____lpython_overloaded_0__pow
  2 pow
  [((IntegerConstant 2 (Integer 4 []))) ;; <~~~ call-args HERE
   ((IntegerConstant 2 (Integer 4 [])))]
  (Real 8 [])
  (RealConstant 4.000000 (Real 8 [])  )  ())

However, I encountered the following usage in expr_14-6023c49:

(IntrinsicFunction
  Abs
  [(RealBinOp ;; <~~~ not enough nesting
   (Var 2 a3)
   Sub
   (RealConstant 9.000000 (Real 8 []))
   (Real 8 [])  ()  )]
  0
  (Real 8 [])
  ()  )

This has one too few levels of nesting for a call-args. I solved it with a spec like this:

(s/def ::call-arg-or-args
  (s/or :call-arg  ::call-arg
        :call-args ::call-args))

(defmasrtype
  IntrinsicFunction expr
  (intrinsic-ident    call-arg-or-args      overload-id
                      return-type           value?))

I guess this is OK, but it's a little "hinky."

certik commented 1 year ago

Here is a definition of IntrinsicFunction:

    | IntrinsicFunction(int intrinsic_id, expr* args, int overload_id,        
            ttype? type, expr? value)                                        

The arguments are just a list of expressions. They cannot be optional, so there is no double nesting. Everything seems clean? There is no call-arg or call-args for IntrinsicFunction.

rebcabin commented 1 year ago

The question is "why is it different to FunctionCall and SubroutineCall? I can handle the difference, just want to know why ... expr * may be too loose, not sure yet.

certik commented 1 year ago

We designed IntrinsicFunction to be more strict. No need to check for "nil", since it can't be nil.