RFR: 8258010: Debug build failure with clang-10 due to -Wdeprecated-copy and -Wimplicit-int-float-conversion [v2]

[Hao Sun]

On Mon, 4 Jan 2021 06:22:46 GMT, Kim Barrett <kbarrett at openjdk.org> 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