From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andrew Gierth <andrew@tao11.riddles.org.uk>
cc: pgsql-hackers@postgresql.org
Subject: Re: A really subtle lexer bug
In-reply-to: <87r2ipylw4.fsf@news-spur.riddles.org.uk>
References: <87va851ppl.fsf@news-spur.riddles.org.uk>
 <87ftz90yma.fsf@news-spur.riddles.org.uk> <18470.1535036115@sss.pgh.pa.us>
 <871sap1658.fsf@news-spur.riddles.org.uk> <29360.1535039670@sss.pgh.pa.us>
 <87r2ipylw4.fsf@news-spur.riddles.org.uk>
Comments: In-reply-to Andrew Gierth <andrew@tao11.riddles.org.uk>
	message dated "Thu, 23 Aug 2018 19:43:50 +0100"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <4514.1535051153.1@sss.pgh.pa.us>
Date: Thu, 23 Aug 2018 15:05:53 -0400
Message-ID: <4515.1535051153@sss.pgh.pa.us>
Precedence: bulk

Andrew Gierth <andrew@tao11.riddles.org.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