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

At Thu, 9 Sep 2021 13:28:54 +0900, Michael Paquier <michael(at)paquier(dot)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