Re: strange case of "if ((a & b))"

At Thu, 9 Sep 2021 13:28:54 +0900, Michael Paquier <michael@paquier.xyz> wrote in 
> On Tue, Sep 07, 2021 at 02:59:58PM +0900, Michael Paquier wrote:
> > In 0002, everything is a boolean expression except for
> > SpGistPageStoresNulls() and GistPageIsLeaf().  So that's a good
> > cleanup overall.
> 
> I looked again at 0002 again yesterday, and that was an improvement
> for most of those locations, where we already use a boolean as
> expression, so done mostly as of fd0625c.
> 
> > -   pathnode->parallel_aware = parallel_workers > 0 ? true : false;
> > +   pathnode->parallel_aware = parallel_workers > 0;
> > I also prefer that we keep the parenthesis for such things.  That's
> > more readable and easier to reason about.
> 
> Adjusted these as well.

Maybe I'm missing something, but I can see several instances of the
"eval-bool ? true : false" pattern after fd0625c7a9 that are not in
the latest 0002.

./backend/nodes/readfuncs.c187:#define strtobool(x)  ((*(x) == 't') ? true : false)
./backend/tsearch/wparser_def.c1859:	return (item && (item->flags & A_BINGO)) ? true : false;

These are candidates to fix.

./backend/tsearch/ts_utils.c145:					sizeof(char *), pg_qsort_strcmp)) ? true : false;

This is a part of the following expression.

> 	return (s->stop && s->len > 0 &&
> 			bsearch(&key, s->stop, s->len,
> 					sizeof(char *), pg_qsort_strcmp)) ? true : false;

So this is also a candidate.

Also found !f(eval) equivalents.

./backend/access/gist/gistsplit.c424:	sv->spl_ldatum_exists = (v->spl_lisnull[attno]) ? false : true;
./backend/access/gist/gistsplit.c425:	sv->spl_rdatum_exists = (v->spl_risnull[attno]) ? false : true;
./backend/access/gist/gistsplit.c424:	sv->spl_ldatum_exists = (v->spl_lisnull[attno]) ? false : true;
./backend/access/gist/gistsplit.c425:	sv->spl_rdatum_exists = (v->spl_risnull[attno]) ? false : true;
./backend/access/gist/gistsplit.c454:		sv->spl_ldatum_exists = (v->spl_lisnull[attno]) ? false : true;
./backend/access/gist/gistsplit.c455:		sv->spl_rdatum_exists = (v->spl_risnull[attno]) ? false : true;
./backend/commands/tablecmds.c7466:					  newDefault == NULL ? false : true);
./backend/executor/spi.c146:	_SPI_current->atomic = (options & SPI_OPT_NONATOMIC ? false : true);
./backend/executor/nodeResult.c198:	resstate->rs_checkqual = (node->resconstantqual == NULL) ? false : true;
./backend/executor/nodeResult.c263:	node->rs_checkqual = (node->resconstantqual == NULL) ? false : true;
./backend/statistics/mcv.c1622:	memset(matches, (is_or) ? false : true,
./backend/tsearch/spell.c1708:						? false : true;

./interfaces/ecpg/ecpglib/execute.c124:			string = string ? false : true;
./interfaces/ecpg/ecpglib/prepare.c113:			string = string ? false : true;
./interfaces/ecpg/ecpglib/data.c959:						string = string ? false : true;
(Note: the "string" is a bool)

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center