On 16/06/20 11:42 +0100, Richard Sandiford wrote:
Jonathan Wakely <jwak...@redhat.com> writes:
+  template<typename ...Ts>
+  rtx_insn *operator() (Ts... args...) const

Why is this declared as a variadic template **and** a varargs function?

I think the second ellipsis is wrong, it should be just:

+  template<typename ...Ts>
+  rtx_insn *operator() (Ts... args) const

Oops, yes.

And this seems like a more direct way to say "a list of N rtx types"
where N is sizeof...(args):

diff --git a/gcc/recog.h b/gcc/recog.h
index 0a71a02c4a9..fd22c58c92a 100644
--- a/gcc/recog.h
+++ b/gcc/recog.h
@@ -294,10 +294,13 @@ struct insn_gen_fn
 {
   typedef void (*stored_funcptr) (void);

+  template<typename T> using rtx_t = rtx;
+
   template<typename ...Ts>
-  rtx_insn *operator() (Ts... args...) const
+  rtx_insn *operator() (Ts... args) const
   {
-    return ((rtx_insn *(*) (decltype(args, NULL_RTX)...)) func) (args...);
+    using funcptr = rtx_insn * (*) (rtx_t<Ts>...);
+    return ((funcptr) func) (args...);
   }

   // This is for compatibility of code that invokes functions like

The rtx_t<T> alias is the type 'rtx' for any T. The pack expansion
rtx_t<Ts>... is a list of rtx the same length as the pack Ts.

Yeah.  As mentioned on IRC, I'd originally done it like this, but didn't
like the ad-hoc type alias.  I can't remember what name I'd used, but the
problem in both cases is/was that the ad-hoc name looks odd when you're
used to seeing plain “rtx”.  It also felt weird to expose this kind of
internal, function-specific implementation detail at insn_gen_fn scope.

Using decltype also gave nicer error messages, e.g.:

   invalid conversion from ‘int’ to ‘rtx {aka rtx_def*}’

instead of:

   invalid conversion from ‘int’ to ‘insn_gen_fn::rtx_t<int> {aka rtx_def*}’

I think the latter is likely to confuse people when they see it
for the first time.

So in some ways I'd prefer to keep the decltype and just add a void cast
to suppress the warning.  (Seems odd to warn about expressions having no
effect in an unevaluated context.)

Yes, arguably a G++ bug.

But how about the below instead?  Hopefully the alias name is mnemonic
enough.

Works for me, thanks.

Reply via email to