Re: A really subtle lexer bug

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andrew Gierth <andrew(at)tao11(dot)riddles(dot)org(dot)uk>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: A really subtle lexer bug
Date: 2018-08-23 19:05:53
Message-ID: 4515.1535051153@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Andrew Gierth <andrew(at)tao11(dot)riddles(dot)org(dot)uk> writes:
> Here's what I will push unless there's something important I missed.

Stylistic nitpick: I don't especially like "continue" as the body of
a for-loop. How about instead of this:

for (nchars--;
nchars > 1 &&
(yytext[nchars - 1] == '+' ||
yytext[nchars - 1] == '-');
nchars--)
continue;

do this:

do {
nchars--;
} while (nchars > 1 &&
(yytext[nchars - 1] == '+' ||
yytext[nchars - 1] == '-'));

That's a clearer separation between loop action and loop test, and
it makes it more obvious that you're relying on the loop condition
to be true at the start.

Also, I'm not entirely convinced that replacing the strchr() with
a handmade equivalent is a good idea. Some versions of strchr()
are pretty quick.

No objections beyond that.

regards, tom lane

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2018-08-23 19:10:19 JIT explain output
Previous Message Tom Lane 2018-08-23 18:53:37 Re: [HACKERS] Optional message to user when terminating/cancelling backend