On Mon, 4 Jan 2021 06:59:54 GMT, Hao Sun
<github.com+16932759+shqk...@openjdk.org> wrote:
>> 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?
> Take the following code snippet as an example.
> DUIterator_Fast t1, t2;
> t2 = t1; // assignment operator
> DUIterator_Fast t3 = t1; // copy constructor. same as "t3(t1)"
> My point is that, the ctor for `t2` has already invoked, i.e. initializing
> `_vdui` as false. That's why `operator=` works well.
>
>
> 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)); }
That's true on the first assignment of `t2`. But what if `t2` is reassigned
to some other iterator. That assignment sees `_vdui` true, and keeps the old
value of `_last` rather than updating the value from that other iterator. Is
that really correct? It certainly seems strange. I'm trying to find someone
who understands this code to get involved, but holidays.
-------------
PR: https://git.openjdk.java.net/jdk/pull/1874
