Hi,
On 2018-09-21 16:20:42 -0400, Tom Lane wrote:
> Thomas Munro <thomas.munro@enterprisedb.com> writes:
> > On Fri, Sep 21, 2018 at 5:52 PM Andres Freund <andres@anarazel.de> wrote:
> >> Here's a very quick-and-dirty implementation of this approach. Some very
> >> very brief testing seems to indicate it works, although I'm sure not
> >> perfectly.
>
> > And here is a quick-and-dirty variadic COMPLETE_WITH(...). Together:
>
> I'm a bit inclined to get rid of the need for PP_NARG() by instead making
> the macros add a trailing NULL argument, viz
>
> #define TailMatches(...) \
> CheckTailMatchesFor(previous_words_count, previous_words, __VA_ARGS__, NULL)
I don't think that'd *quite* work right now - MatchAny is also NULL. We
probably could make it work by redefining either MatchAny or the last
argument to CheckTailMatchesFor() to some other value however.
> This'd require (some of) the implementation functions to iterate over
> their variadic arguments twice, the first time merely counting how many
> there are.
Yea, leaving the above problem aside, I've a hard time to get excited
about that overhead.
> But we aren't exactly concerned about max runtime performance
> here, and the PP_NARG() thing seems like a crock that could easily blow
> out compilation time on some compilers.
It's not actually that complicated an expansion, and we've not
encountered many problems with expansions that are similarly complex,
e.g. ereport et al. It's pretty likely at least as cheap as the current
approach, where we sometimes have 9 deep recursion. So I'm not too
concerned about the compile-time performance. FWIW, I tested it with gcc
-E yesterday, and I couldn't measure any difference.
I think there's some argument to be made about the "mental" complexity
of the macros - if we went for them, we'd certainly need to add some
docs about how they work. One argument for having PP_NARGS (renamed) is
that it doesn't seem useful just here, but in a few other cases as well.
Greetings,
Andres Freund