On Mon, 4 Jan 2021 06:22:46 GMT, Kim Barrett <[email protected]> wrote:
>> Hao Sun has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Remove the unused assignment operator for DUIterator_Last
>>
>> Instead of adding the copy constructor, remove the assignment operator
>> of DUIterator_Last since it's never used.
>>
>> Change-Id: Idf5658e38861eb2b0e724b064d17e9ab4e93905f
>> CustomizedGitHooks: yes
>
> src/hotspot/share/opto/node.hpp line 1396:
>
>> 1394:
>> 1395: DUIterator_Fast(const DUIterator_Fast& that)
>> 1396: { _outp = that._outp; debug_only(reset(that)); }
>
> `reset` tests `_vdui`, but nothing here has set it, so it's uninitialized and
> that test wil be UB.
>
> I'm also not sure why it's okay for `operator=` to use whatever the current
> value of `_vdui` might be; that could leave `_last` as the old value rather
> than refreshing from `that`, which seems wrong. This is aabout pre-existing
> code that looks questionable to me.
I suppose the constructor would be invoked before the copy assignment operator.
That is `_vdui` gets initialized already in the ctor `DUIterator_Fast()` for
`operator=` case. Right?
Yes. For our newly-added copy constructor for `DUIterator_Fast`, we should
initialize `_vdui` as `false`. It may be defined as below.
DUIterator_Fast(const DUIterator_Fast& that)
{ _outp = that._outp; debug_only(_vdui = false; reset(that));
}
-------------
PR: https://git.openjdk.java.net/jdk/pull/1874
