[PATCH] recog: Use parameter packs for operator()

[Jonathan Wakely]

On 18/06/20 10:02 +0100, Jonathan Wakely wrote:
>On 18/06/20 08:55 +0100, Richard Sandiford wrote:
>>Sebastian Huber <sebastian.huber@embedded-brains.de> writes:
>>>On 18/06/2020 09:09, Richard Sandiford wrote:
>>>
>>>>Sebastian Huber <sebastian.huber@embedded-brains.de> writes:
>>>>>On 16/06/2020 12:42, Richard Sandiford wrote:
>>>>>
>>>>>>[...]
>>>>>>2020-06-16  Richard Sandiford  <richard.sandiford@arm.com>
>>>>>>
>>>>>>gcc/
>>>>>>	* coretypes.h (first_type): New alias template.
>>>>>>	* recog.h (insn_gen_fn::operator()): Use it instead of a decltype.
>>>>>>	Remove spurious “...” and split the function type out into a typedef.
>>>>>>---
>>>>>>   gcc/coretypes.h | 4 ++++
>>>>>>   gcc/recog.h     | 5 +++--
>>>>>>   2 files changed, 7 insertions(+), 2 deletions(-)
>>>>>>
>>>>>>diff --git a/gcc/coretypes.h b/gcc/coretypes.h
>>>>>>index cda22697cc3..01ec2e23ce2 100644
>>>>>>--- a/gcc/coretypes.h
>>>>>>+++ b/gcc/coretypes.h
>>>>>>@@ -359,6 +359,10 @@ struct kv_pair
>>>>>>     const ValueType value;	/* the value of the name */
>>>>>>   };
>>>>>>
>>>>>>+/* Alias of the first type, ignoring the second.  */
>>>>>>+template<typename T1, typename T2>
>>>>>>+using first_type = T1;
>>>>>>+
>>>>>>   #else
>>>>>>
>>>>>>   struct _dont_use_rtx_here_;
>>>>>>diff --git a/gcc/recog.h b/gcc/recog.h
>>>>>>index 0a71a02c4a9..d674d384723 100644
>>>>>>--- a/gcc/recog.h
>>>>>>+++ b/gcc/recog.h
>>>>>>@@ -295,9 +295,10 @@ struct insn_gen_fn
>>>>>>     typedef void (*stored_funcptr) (void);
>>>>>>
>>>>>>     template<typename ...Ts>
>>>>>>-  rtx_insn *operator() (Ts... args...) const
>>>>>>+  rtx_insn *operator() (Ts... args) const
>>>>>>     {
>>>>>>-    return ((rtx_insn *(*) (decltype(args, NULL_RTX)...)) func) (args...);
>>>>>>+    typedef rtx_insn *(*funcptr) (first_type<rtx, Ts>...);
>>>>>>+    return ((funcptr) func) (args...);
>>>>>>     }
>>>>>>
>>>>>>     // This is for compatibility of code that invokes functions like
>>>>>I get this error on FreeBSD 12.1 with
>>>>>
>>>>>c++ --version
>>>>>FreeBSD clang version 8.0.1 (tags/RELEASE_801/final 366581) (based on
>>>>>LLVM 8.0.1)
>>>>>Target: x86_64-unknown-freebsd12.1
>>>>>Thread model: posix
>>>>>InstalledDir: /usr/bin
>>>>>
>>>>>In file included from ../../gnu-mirror-gcc-aff95ee/gcc/function.c:51:
>>>>>../../gnu-mirror-gcc-aff95ee/gcc/recog.h:301:30: error: too many
>>>>>arguments to function call, expected 1, have 2
>>>>>      return ((funcptr) func) (args...);
>>>>>             ~~~~~~~~~~~~~~~~  ^~~~
>>>>>../../gnu-mirror-gcc-aff95ee/gcc/function.c:3315:29: note: in
>>>>>instantiation of function template specialization
>>>>>'insn_gen_fn::operator()<rtx_def *, rtx_def *>' requested here
>>>>>          emit_insn (GEN_FCN (icode) (parmreg, validated_mem));
>>>>Thanks for the report.  Was clang OK with the earlier version
>>>>(i.e. before 4e49b994de060d4a6c9318d0ed52ef038153426e)?
>>>Yes, the last version I built successfully was
>>>b952c2cfcd74c284970e1b9bf1fca58f5f69ab23 on 10th of June. I build GCC
>>>roughly once per week.
>>
>>Hmm, yeah, can reproduce with a recentish build from clang master:
>>
>>template<typename T> using int_t = int;
>>void (*f)();
>>template<typename ...Ts>
>>void g(Ts... args) {
>>#if A
>> ((void (*)(int_t<Ts>...)) f)(args...);
>>#elif B
>> typedef void (*funcptr)(decltype((void)args, 0)...);
>> ((funcptr) f)(args...);
>>#else
>> typedef void (*funcptr)(int_t<Ts>...);
>> ((funcptr) f)(args...);
>>#endif
>>}
>>void h(int x, int y) { g(x, y); }
>>
>>Works with -DA and -DB, but fails with the final version.
>>
>>So it looks like the obvious choices are:
>>
>>(1) Remove the typedef.
>>(2) Go back to using decltype.
>>(3) Revert the whole thing and go back to the overloads.
>>
>>Not sure about (1), because it leads to awkward formatting, and would
>>need a comment to say “Don't use a typedef!”.  The above error shows
>>that we're probably close to the edge of what compilers will support
>>reliably.
>
>That's extremely surprising. It's basic C++11 and so Clang should have
>been able to do it 10+ years ago. I'll look into it.
>
>>(2) seems to work more reliably, and gives nicer error messages,
>>but was unpopular for being unidiomatic C++.
>>
>>I guess at this point, (3) is the way to go.
>
>Yuck, (2) is better than (3).
>
>I'm not massively fond of (2) but (3) is horrible and just unnecessary
>with variadics.

I reported https://bugs.llvm.org/show_bug.cgi?id=46377 because it
doesn't seem to be known (which is almost as surprising as the fact it
fails in the first place). It used to work with Clang 3.0 back in 2011
but fails with every release since.

I thought I'd seen it before, but I was thinking of
https://bugs.llvm.org/show_bug.cgi?id=14858 which got fixed years ago.