RFR: 8258010: Debug build failure with clang-10 due to -Wdeprecated-copy [v3]

[Hao Sun]

On Wed, 6 Jan 2021 03:14:48 GMT, Hao Sun <github.com+16932759+shqking at openjdk.org> wrote:

>> node.hpp changes seems fine.
>> Passed tier1 builds and testing.
>
>> > I think the two issues described here are distinct and should be dealt
>> > with in separate bugs and PRs. Their only relation is that both arise
>> > with using clang-10. But they are very different problems, in very
>> > different parts of the code, and probably ought to be reviewed by
>> > folks from different teams.
>> 
>> Thanks for your comment.
>> 
>> Warning message of '-Wimplicit-int-float-conversion' was further encountered after we fixed the build failure caused by '-Wdeprecated-copy' first. That's why we put them in one PR initially.
>> 
>> Yes. Your way is much better. But we suppose the issue of '-Wimplicit-int-float-conversion' is trivial and putting them in separate PRs might raise another internal review process (for our side) by which extra time is needed. I was wondering could we continue in one single PR. :)
> 
> Will split this PR.
> In this PR, we focus on the warnings caused by -Wdeprecated-copy.
> Will update the code soon. Will create a new PR to address JDK-8259288.

> Thanks for your comments. @kimbarrett and @navyxliu
> I updated the patch based on my understanding. Please check the latest commit.
> 
> As @kimbarrett mentioned, I suppose there still exist the following problems to be addressed.
> 
> 1. why clang-10 with '-Wdeprecated-copy' does NOT raise warning for class DUIterator. It's weird.
> 2. the assert failure when building with gcc '-fno-elide-constructors'. Might not be related to our patch. (JDK-8259036)
> 3. the implementation of 'operator=' for class DUIterator_Fast might be problematic.

@kimbarrett 
Regarding problem 1, I believe this is because there exists use-defined dtor for class DUIterator and the deprecated copy ctor warning message ought to be emitted by '-Wdeprecated-copy-dtor' (which is not enabled by '-Wall' or '-Wextra').

Please refer to https://github.com/llvm/llvm-project/blob/release/10.x/clang/lib/Sema/SemaDeclCXX.cpp#L13585
The main logic is user-defined dtor/copy constructor/assignment operator is checked in order when diagnosing implicit-defined copy constructor, and warning message will be emitted (lines 13619 to 13625).

In this case of class DUIterator, user-defined dtor is provided but '-Wdeprecated-copy-dtor' is not enabled, clang-10 does not further check whether '-Wdeprecated-copy' is on or not.

I further checked 
1) I built openjdk with "--with-extra-cxxflags='-Wdeprecated-copy-dtor'", but the building failed earlier before class DUIterator.
2) I removed the dtor of class DUIterator manually and built openjdk with clang-10. Then clang-10 would produce the warning as we expected, which I think verified my conjecture.
=== Output from failing command(s) repeated here ===
* For target hotspot_variant-server_libjvm_gtest_objs_test_mathexact.o:
In file included from /home/haosun01/jdk/jdk_src/test/hotspot/gtest/opto/test_mathexact.cpp:26:
In file included from /home/haosun01/jdk/jdk_src/src/hotspot/share/opto/mulnode.hpp:28:

  void operator=(const DUIterator& that)
       ^

  { return DUIterator(this, 0); }
           ^

  void operator=(const DUIterator_Fast& that)
       ^

  return DUIterator_Fast(this, 0);
         ^

   ... (rest of output omitted)
* For target support_gensrc_jdk.localedata__cldr-gensrc.marker:

* All command lines available in /home/haosun01/tmp/deprecated-copy/clang10-fast-build/make-support/failure-logs.
=== End of repeated output ===

-------------

PR: https://git.openjdk.java.net/jdk/pull/1874