some dead code in functioncmds.c

Started by Pavel Stehuleabout 16 years ago6 messages
#1Pavel Stehule
pavel.stehule@gmail.com

Hello

I found some strange code. Is code in "ELSE" part dead, or its bug?

if (stmt->returnType)
{
/* explicit RETURNS clause */
compute_return_type(stmt->returnType, languageOid,
&prorettype, &returnsSet);
if (OidIsValid(requiredResultType) && prorettype != requiredResultType)
ereport(ERROR,
(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
errmsg("function result type must be %s because of OUT parameters",
format_type_be(requiredResultType))));
}
else if (OidIsValid(requiredResultType))
{
/* default RETURNS clause from OUT parameters */
prorettype = requiredResultType;
returnsSet = false;
}
else
{
ereport(ERROR,
(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
errmsg("function result type must be specified")));
/* Alternative possibility: default to RETURNS VOID */

/* WHY FOLOWING LINES? */
prorettype = VOIDOID;
returnsSet = false;
}

Regards
Pavel

#2Heikki Linnakangas
heikki.linnakangas@enterprisedb.com
In reply to: Pavel Stehule (#1)
Re: some dead code in functioncmds.c

Pavel Stehule wrote:

else
{
ereport(ERROR,
(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
errmsg("function result type must be specified")));
/* Alternative possibility: default to RETURNS VOID */

/* WHY FOLOWING LINES? */
prorettype = VOIDOID;
returnsSet = false;
}

To keep the compiler quiet about using the variables uninitialized. The
compiler doesn't know that ereport(ERROR) never returns.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

#3Pavel Stehule
pavel.stehule@gmail.com
In reply to: Heikki Linnakangas (#2)
Re: some dead code in functioncmds.c

2009/10/30 Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>:

Pavel Stehule wrote:

      else
      {
              ereport(ERROR,
                              (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
                               errmsg("function result type must be specified")));
              /* Alternative possibility: default to RETURNS VOID */

/* WHY FOLOWING LINES? */
              prorettype = VOIDOID;
              returnsSet = false;
      }

To keep the compiler quiet about using the variables uninitialized. The
compiler doesn't know that ereport(ERROR) never returns.

Should be similar code little bit commented?

Pavel

Show quoted text

--
 Heikki Linnakangas
 EnterpriseDB   http://www.enterprisedb.com

#4Heikki Linnakangas
heikki.linnakangas@enterprisedb.com
In reply to: Pavel Stehule (#3)
Re: some dead code in functioncmds.c

Pavel Stehule wrote:

2009/10/30 Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>:

Pavel Stehule wrote:

else
{
ereport(ERROR,
(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
errmsg("function result type must be specified")));
/* Alternative possibility: default to RETURNS VOID */

/* WHY FOLOWING LINES? */
prorettype = VOIDOID;
returnsSet = false;
}

To keep the compiler quiet about using the variables uninitialized. The
compiler doesn't know that ereport(ERROR) never returns.

Should be similar code little bit commented?

*shrug*, maybe, often we do put a "/* keep compiler quiet */" comment on
such places.

On closer look, the "Alternative possibility: default to RETURNS VOID"
comment suggests that besides keeping the compiler quiet, those lines
demonstrate an alternative behavior that was considered.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

#5Pavel Stehule
pavel.stehule@gmail.com
In reply to: Heikki Linnakangas (#4)
Re: some dead code in functioncmds.c

2009/10/30 Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>:

Pavel Stehule wrote:

2009/10/30 Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>:

Pavel Stehule wrote:

      else
      {
              ereport(ERROR,
                              (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
                               errmsg("function result type must be specified")));
              /* Alternative possibility: default to RETURNS VOID */

/* WHY FOLOWING LINES? */
              prorettype = VOIDOID;
              returnsSet = false;
      }

To keep the compiler quiet about using the variables uninitialized. The
compiler doesn't know that ereport(ERROR) never returns.

Should be similar code little bit commented?

*shrug*, maybe, often we do put a "/* keep compiler quiet */" comment on
such places.

On closer look, the "Alternative possibility: default to RETURNS VOID"
comment suggests that besides keeping the compiler quiet, those lines
demonstrate an alternative behavior that was considered.

I prefere "keep compiler quiet". It is cleaner - and it is used more times.

Pavel

Show quoted text

--
 Heikki Linnakangas
 EnterpriseDB   http://www.enterprisedb.com

#6Peter Eisentraut
peter_e@gmx.net
In reply to: Pavel Stehule (#5)
Re: some dead code in functioncmds.c

On fre, 2009-10-30 at 19:08 +0100, Pavel Stehule wrote:

2009/10/30 Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>:

Pavel Stehule wrote:

2009/10/30 Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>:

To keep the compiler quiet about using the variables uninitialized. The
compiler doesn't know that ereport(ERROR) never returns.

Should be similar code little bit commented?

*shrug*, maybe, often we do put a "/* keep compiler quiet */" comment on
such places.

On closer look, the "Alternative possibility: default to RETURNS VOID"
comment suggests that besides keeping the compiler quiet, those lines
demonstrate an alternative behavior that was considered.

I prefere "keep compiler quiet". It is cleaner - and it is used more times.

A while ago a patch was discussed (in the "clang checker" threat) that
would teach the compiler that elog(>=ERROR) does not return, but I think
it's not portable without further work.