Re: Converting tab-complete.c's else-if chain to a switch

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Converting tab-complete.c's else-if chain to a switch
Date: 2024-07-15 19:42:40
Message-ID: 3193170.1721072560@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Andres Freund <andres(at)anarazel(dot)de> writes:
> On 2024-07-13 13:16:14 -0400, Tom Lane wrote:
>> Of course the main point is the hope that it will work at all with MSVC, but
>> I'm not in a position to test that.

> It does work with just your patches applied - very nice.

Thanks for testing!

> The only obvious thing that doesn't is that ctrl-c without a running query
> terminates psql - interrupting a running query works without terminating psql,
> which is a bit weird. But that seems independent of this series.

Yeah, whatever's going on there is surely orthogonal to this.

>> +# The input is a C file that contains some case labels that are not
>> +# constants, but look like function calls, for example:
>> +# case Matches("A", "B"):

> Hm, the fact that we are continuing to use the same macros in the switch makes
> it a bit painful to edit the .in.c in an editor with compiler-warnings
> integration - I see a lot of reported errors ("Expression is not an integer
> constant expression") due to case statements not being something that the
> normal C switch can handle.

Ugh, good point.

> Perhaps we could use a distinct macro for use in the switch, which generates
> valid (but nonsensical) code, so we avoid warnings?

Dunno. I'd explicitly wanted to not have different notations for
patterns implemented in switch labels and those used in ad-hoc tests.

Thinking a bit further outside the box ... maybe the original source
code could be like it is now (or more likely, like it is after 0002,
with the switch-to-be stuff all in a separate function), and we
could make the preprocessor perform the else-to-switch conversion
every time instead of viewing that as a one-time conversion?
That would make it a bit more fragile, but perhaps not impossibly so.

>> +# These lines are replaced by "case N:" where N is a unique number
>> +# for each such case label. (For convenience, we use the line number
>> +# of the case label, although other values would work just as well.)

> Hm, using the line number seems to make it a bit harder to compare the output
> of the script after making changes to the input.

OK. I'd thought this choice might be helpful for debugging, but I'm
not wedded to it. The obvious alternative is to use sequential
numbers for case labels, but wouldn't that also be a bit problematic
if "making changes" includes adding or removing rules?

> I don't think we want to install tab-complete.c?

Good point --- I borrowed that rule from somewhere else, and
failed to study it closely enough.

regards, tom lane

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Arjan Marku 2024-07-15 19:59:24 Re: [PATCH v1] Fix parsing of a complex type that has an array of complex types
Previous Message Andres Freund 2024-07-15 19:37:54 Re: Upgrade Debian CI images to Bookworm