On Tue, 5 Jan 2021 03:44:10 GMT, Hao Sun <github.com+16932759+shqk...@openjdk.org> wrote:
>> 1. '-Wdeprecated-copy' >> As specified in C++11 [1], "the generation of the implicitly-defined >> copy constructor is deprecated if T has a user-defined destructor or >> user-defined copy assignment operator". The rationale behind is the >> well-known Rule of Three [2]. >> >> Introduced since gcc-9 [3] and clang-10 [4], flag '-Wdeprecated-copy' >> warns about the C++11 deprecation of implicitly declared copy >> constructor and assignment operator if one of them is user-provided. >> Defining an explicit copy constructor would suppress this warning. >> >> The main reason why debug build with gcc-9 or higher succeeds lies in >> the inconsistent warning behaviors between gcc and clang. See the >> reduced code example [5]. We suspect it might be return value >> optimization/copy elision [6] that drives gcc not to declare implicit >> copy constructor for this case. >> >> Note that flag '-Wdeprecated' in clang-8 and clang-9 would also raise >> warnings for deprecated defintions of copy constructors. However, >> '-Wdeprecated' is not enabled by '-Wall' or '-Wextra'. Hence, clang-8 >> and clang-9 are not affected. >> >> 2. '-Wimplicit-int-float-conversion' >> Making the conversion explicit would fix it. >> >> Flag '-Wimplicit-int-float-conversion' is first introduced in clang-10. >> Therefore clang-8 and clang-9 are not affected. The flag with similar >> functionality in gcc is '-Wfloat-conversion', but it is not enabled by >> '-Wall' or '-Wextra'. That's why this warning does not apprear when >> building with gcc. >> >> [1] https://en.cppreference.com/w/cpp/language/copy_constructor >> [2] https://en.cppreference.com/w/cpp/language/rule_of_three >> [3] https://www.gnu.org/software/gcc/gcc-9/changes.html >> [4] https://releases.llvm.org/10.0.0/tools/clang/docs/ReleaseNotes.html >> [5] https://godbolt.org/z/err4jM >> [6] https://en.wikipedia.org/wiki/Copy_elision#Return_value_optimization >> >> >> Note that we have tested with this patch, debug build succeeded with >> clang-10 on Linux X86/AArch64 machines. > > Hao Sun has updated the pull request incrementally with one additional commit > since the last revision: > > Update the copy constructors for class DUIterator, DUIterator_Fast and > DUIterator_Last > > 1. Update copyright year to 2021. > 2. Add the definition of copy constructor for class DUIterator. > Otherwise, gcc with '-fno-elide-constructors' would raise a warning. > 3. For the copy constructor of class DUIterator_Fast, we initialize > '_vdui' as false, otherwise UB is introduced. > 4. It's better to define the copy constructor of class DUIterator_Last > as explicitly-defaulted, instead of leaving it for compilers to > implicitly define. > > Change-Id: I3d2f5b396aa116d1832f52da361ff3172459a87e > CustomizedGitHooks: yes node.hpp changes seems fine. Passed tier1 builds and testing. ------------- Marked as reviewed by kvn (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/1874
