Code that seems to do nothing

[Dimitry Sibiryakov]

Hello All,

in dsql/ExprNodes.cpp:ParameterNode::pass2() I see following code:

> dsc desc;
> getDesc(tdbb, csb, &desc);

But variable desc is not used anywhere below and getDesc() function has no
side-effects. What does this code do?

--
WBR, SD.

[Mark Rotteveel]

Are you sure it has no side-effects? I see potential paths where it can
throw an exception, and it potentially does things in `arg1` and `arg2`
that have side-effects depending on the implementation of those nodes
(I didn't check, but I guess the could perform operations which might
changes something in those nodes).

Mark
--
Mark Rotteveel

[Dimitry Sibiryakov]

'Mark Rotteveel' via firebird-devel wrote 07.06.2024 18:11:
>>>     dsc desc;
>>>     getDesc(tdbb, csb, &desc);
>>
>>    But variable desc is not used anywhere below and getDesc() function has no
>> side-effects. What does this code do?
>
> Are you sure it has no side-effects? I see potential paths where it can throw an
> exception, and it potentially does things in `arg1` and `arg2` that have
> side-effects depending on the implementation of those nodes (I didn't check, but
> I guess the could perform operations which might changes something in those nodes).

Which function do you look at? I see only this:

> void ParameterNode::getDesc(thread_db* /*tdbb*/, CompilerScratch* /*csb*/, dsc* desc)
> {
> *desc = message->format->fmt_desc[argNumber];
> // Must reset dsc_address because it's used in others places to read literals, but here it was
> // an offset in the message.
> desc->dsc_address = NULL;
> }


--
WBR, SD.

[Mark Rotteveel]

On 07/06/2024 18:15, 'Dimitry Sibiryakov' via firebird-devel wrote:
>   Which function do you look at? I see only this:
>
>> void ParameterNode::getDesc(thread_db* /*tdbb*/, CompilerScratch*
>> /*csb*/, dsc* desc)
>> {
>>     *desc = message->format->fmt_desc[argNumber];
>>     // Must reset dsc_address because it's used in others places to
>> read literals, but here it was
>>     // an offset in the message.
>>     desc->dsc_address = NULL;
>> }

You're right, I was looking at some other method. However, keep in mind
that ParameterNode::getDesc can also be called from other locations than
that pass2 (e.g. it could be called from ArithmeticNode::getDesc if
either arg1 or arg2 is a ParameterNode). So, to be able to call it,
ParameterNode::pass2 *must* also pass a descriptor.

Mark
--
Mark Rotteveel

[Dimitry Sibiryakov]

'Mark Rotteveel' via firebird-devel wrote 07.06.2024 18:23:
> You're right, I was looking at some other method. However, keep in mind that
> ParameterNode::getDesc can also be called from other locations than that pass2
> (e.g. it could be called from ArithmeticNode::getDesc if either arg1 or arg2 is
> a ParameterNode). So, to be able to call it, ParameterNode::pass2 *must* also
> pass a descriptor.

Yes, but what puzzles me is WHY pass2() calls it. ParameterNode is final, no
descendant can override getDesc() to do something useful.

--
WBR, SD.

[Mark Rotteveel]

BTW, this doesn't look like something that is without side-effects:

*desc = message->format->fmt_desc[argNumber];

desc->dsc_address = NULL;

i.e., it assigns the value of `message->format->fmt_desc[argNumber]` to
`*desc`, and then sets its `dsc_address` to null, which to me would seem
that it has a side-effect on whatever is held in
`message->format->fmt_desc[argNumber]`, as I assume there are no
copy-semantics in this case (but my C++ knowledge is really bad, so I
might be wrong).

Mark
--
Mark Rotteveel

[Dimitry Sibiryakov]

'Mark Rotteveel' via firebird-devel wrote 07.06.2024 18:27:
> BTW, this doesn't look like something that is without side-effects:
>
> *desc = message->format->fmt_desc[argNumber];
> desc->dsc_address = NULL;
>
> i.e., it assigns the value of `message->format->fmt_desc[argNumber]` to `*desc`,
> and then sets its `dsc_address` to null, which to me would seem that it has a
> side-effect on whatever is held in `message->format->fmt_desc[argNumber]`, as I
> assume there are no copy-semantics in this case (but my C++ knowledge is really
> bad, so I might be wrong).

I may be wrong, but for what I know first line really copies whole context of
descriptor from format to target variable and second line changes member of this
target variable, not original format's descriptor.

--
WBR, SD.