[PATCH] D15456: [PATCH] New diagnostic for non-idiomatic copy or move operations (v2)

[Aaron Ballman via cfe-commits]

aaron.ballman created this revision.
aaron.ballman added reviewers: rsmith, dblaikie.
aaron.ballman added a subscriber: cfe-commits.

All copy operations were created equal (as far as the core language is concerned), but some copy operations are more equal than others (as far as the library is concerned). The same is true of move operations. This patch diagnoses copy and move operations (constructors and assignment operators) that are non-idiomatic so that users are not surprised when their copyable type cannot be placed in a standard library container or used with a standard library algorithm.

Idiomatic copy and move operations are (where T is the class type for the object):
Copy Constructor: takes a const T&
Move constructor: takes a T&&
Copy Assignment: takes a const T&, returns a T&, and has no ref-qualifiers
Move Assignment: takes a T&&, returns a T&, and has no ref-qualifiers

This patch does not diagnose *all* non-idiomatic uses because of false positives in practice. Specifically, it does not diagnose when a non-idiomatic operation is overloaded with an idiomatic one (T(const T&); T(T&);) or when there are multiple non-idiomatic operation overloads (T(volatile T&); T(T&);). In both cases, it is reasonable to assume that the user knows they have non-idiomatic signatures. This means that the diagnostic can always be suppressed by either converting the operation into an idiomatic one, or by adding an overload that is deleted (or is private, as in pre-C++11 code bases), depending on the behavior they desire.

One thing this patch does could have are fix-it hints to correct the types involved. However, I cannot find a way to get the full source range for the type, including qualifiers. For a parameter of type 'const volatile T&', the TypeLoc appears to only track the location of T&, but not the qualifiers. Assistance in understanding how to do this would be appreciated.


http://reviews.llvm.org/D15456

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaDeclCXX.cpp
  test/Analysis/NewDelete-checker-test.cpp
  test/Analysis/inlining/path-notes.cpp
  test/Analysis/operator-calls.cpp
  test/CXX/basic/basic.types/p10.cpp
  test/CXX/class.access/p4.cpp
  test/CXX/class/class.union/p1.cpp
  test/CXX/class/p6-0x.cpp
  test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p1.cpp
  test/CXX/dcl.decl/dcl.init/dcl.init.ref/p5-cxx03-extra-copy.cpp
  test/CXX/dcl.decl/dcl.init/dcl.init.ref/p5-cxx0x-no-extra-copy.cpp
  test/CXX/dcl.decl/dcl.meaning/dcl.fct/dcl.fct.def.default/p2.cpp
  test/CXX/drs/dr0xx.cpp
  test/CXX/drs/dr1xx.cpp
  test/CXX/drs/dr3xx.cpp
  test/CXX/drs/dr5xx.cpp
  test/CXX/expr/expr.prim/expr.prim.lambda/p14.cpp
  test/CXX/expr/expr.prim/expr.prim.lambda/p21.cpp
  test/CXX/special/class.copy/implicit-move.cpp
  test/CXX/special/class.copy/p11.0x.copy.cpp
  test/CXX/special/class.copy/p12-0x.cpp
  test/CXX/special/class.copy/p18-cxx11.cpp
  test/CXX/special/class.copy/p20.cpp
  test/CXX/special/class.copy/p23-cxx11.cpp
  test/CXX/special/class.copy/p25-0x.cpp
  test/CXX/special/class.copy/p8-cxx11.cpp
  test/CXX/special/class.copy/p9.cpp
  test/CXX/stmt.stmt/stmt.dcl/p3-0x.cpp
  test/CXX/stmt.stmt/stmt.dcl/p3.cpp
  test/CXX/temp/temp.fct.spec/temp.deduct/cwg1170.cpp
  test/CodeGenCXX/copy-assign-synthesis-3.cpp
  test/OpenMP/for_firstprivate_messages.cpp
  test/OpenMP/for_lastprivate_messages.cpp
  test/OpenMP/for_reduction_messages.cpp
  test/OpenMP/for_simd_lastprivate_messages.cpp
  test/OpenMP/for_simd_reduction_messages.cpp
  test/OpenMP/parallel_copyin_messages.cpp
  test/OpenMP/parallel_for_copyin_messages.cpp
  test/OpenMP/parallel_for_lastprivate_messages.cpp
  test/OpenMP/parallel_for_reduction_messages.cpp
  test/OpenMP/parallel_for_simd_copyin_messages.cpp
  test/OpenMP/parallel_for_simd_lastprivate_messages.cpp
  test/OpenMP/parallel_for_simd_reduction_messages.cpp
  test/OpenMP/parallel_reduction_messages.cpp
  test/OpenMP/parallel_sections_copyin_messages.cpp
  test/OpenMP/parallel_sections_lastprivate_messages.cpp
  test/OpenMP/parallel_sections_reduction_messages.cpp
  test/OpenMP/parallel_sections_shared_messages.cpp
  test/OpenMP/parallel_shared_messages.cpp
  test/OpenMP/sections_lastprivate_messages.cpp
  test/OpenMP/sections_reduction_messages.cpp
  test/OpenMP/simd_lastprivate_messages.cpp
  test/OpenMP/simd_reduction_messages.cpp
  test/OpenMP/single_copyprivate_messages.cpp
  test/OpenMP/target_map_messages.cpp
  test/OpenMP/task_shared_messages.cpp
  test/OpenMP/taskloop_firstprivate_messages.cpp
  test/OpenMP/taskloop_lastprivate_messages.cpp
  test/OpenMP/taskloop_simd_firstprivate_messages.cpp
  test/OpenMP/taskloop_simd_lastprivate_messages.cpp
  test/OpenMP/teams_reduction_messages.cpp
  test/OpenMP/teams_shared_messages.cpp
  test/Parser/cxx0x-ambig.cpp
  test/SemaCUDA/implicit-copy.cu
  test/SemaCUDA/implicit-member-target.cu
  test/SemaCXX/MicrosoftCompatibility.cpp
  test/SemaCXX/conditional-expr.cpp
  test/SemaCXX/conversion-function.cpp
  test/SemaCXX/copy-assignment.cpp
  test/SemaCXX/copy-initialization.cpp
  test/SemaCXX/coroutines.cpp
  test/SemaCXX/cxx0x-cursory-default-delete.cpp
  test/SemaCXX/cxx0x-initializer-stdinitializerlist.cpp
  test/SemaCXX/cxx98-compat-flags.cpp
  test/SemaCXX/cxx98-compat-pedantic.cpp
  test/SemaCXX/cxx98-compat.cpp
  test/SemaCXX/default-assignment-operator.cpp
  test/SemaCXX/lambda-expressions.cpp
  test/SemaCXX/overload-call-copycon.cpp
  test/SemaCXX/static-cast.cpp
  test/SemaCXX/type-traits.cpp
  test/SemaCXX/user-defined-conversions.cpp
  test/SemaCXX/warn-consumed-analysis.cpp
  test/SemaCXX/warn-non-idiomatic-copy-move.cpp
  test/SemaCXX/warn-unused-filescoped.cpp
  test/SemaObjCXX/property-synthesis-error.mm
  test/SemaTemplate/constructor-template.cpp
  test/SemaTemplate/instantiate-decl-init.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D15456.42537.patch
Type: text/x-patch
Size: 95467 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20151211/824da502/attachment-0001.bin>