[Sema] Remove a -Wrange warning from -Wall

[Mordante]

Summary

During the review of D73007 Aaron Puchert mentioned warn_for_range_variable_always_copy shouldn't be part of -Wall since some coding styles require for(const auto &bar : bars). This warning would cause false positives for these users. Based on Aaron's proposal refactored the warnings:

• -Wrange-loop-construct warns about possibly unintended constructor calls. This is part of -Wall. It contains
  • warn_for_range_copy: loop variable A of type B creates a copy from type C
  • warn_for_range_const_reference_copy: loop variable A is initialized with a value of a different type resulting in a copy
• -Wrange-loop-bind-reference warns about misleading use of reference types. This is not part of -Wall. It contains
  • warn_for_range_variable_always_copy: loop variable A is always a copy because the range of type B does not return a reference

Note the patch is intended to be backported to clang 11, thus the release notes aren't updated.

[merge_guards_bot]

Unit tests: pass. 62197 tests passed, 0 failed and 815 were skipped.

clang-tidy: pass.

clang-format: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[aaronpuchert]

Looks right to me, but someone else should approve.

[aaron.ballman]

LGTM!

[Mordante]

Created D73834 to cherry-pick the changes to the release branch and updates the release notes.

clang/include/clang/Basic/DiagnosticGroups.td

 def KNRPromotedParameter : DiagGroup<"knr-promoted-parameter">;
 def : DiagGroup<"init-self">;
 def : DiagGroup<"inline">;
 def : DiagGroup<"invalid-pch">;
 def GNULabelsAsValue : DiagGroup<"gnu-label-as-value">;
 def LiteralRange : DiagGroup<"literal-range">;
 def LocalTypeTemplateArgs : DiagGroup<"local-type-template-args",
                                       [CXX98CompatLocalTypeTemplateArgs]>;
-def RangeLoopAnalysis : DiagGroup<"range-loop-analysis">;
+def RangeLoopConstruct : DiagGroup<"range-loop-construct">;
+def RangeLoopBindReference : DiagGroup<"range-loop-bind-reference">;
+def RangeLoopAnalysis : DiagGroup<"range-loop-analysis",
+                                  [RangeLoopConstruct, RangeLoopBindReference]>;
 def ForLoopAnalysis : DiagGroup<"for-loop-analysis">;
 def LoopAnalysis : DiagGroup<"loop-analysis", [ForLoopAnalysis,
                                                RangeLoopAnalysis]>;
 def MalformedWarningCheck : DiagGroup<"malformed-warning-check">;
 def Main : DiagGroup<"main">;
 def MainReturnType : DiagGroup<"main-return-type">;
 def MaxUnsignedZero : DiagGroup<"max-unsigned-zero">;
 def MissingBraces : DiagGroup<"missing-braces">;
def Extra : DiagGroup<"extra", [
     EmptyInitStatement
   ]>;
 
 def Most : DiagGroup<"most", [
     CharSubscript,
     Comment,
     DeleteNonVirtualDtor,
     Format,
+    ForLoopAnalysis,
     Implicit,
     InfiniteRecursion,
     IntInBoolContext,
-    LoopAnalysis,
     MismatchedTags,
     MissingBraces,
     Move,
     MultiChar,
+    RangeLoopConstruct,
     Reorder,
     ReturnType,
     SelfAssignment,
     SelfMove,
     SizeofArrayArgument,
     SizeofArrayDecay,
     StringPlusInt,
     TautologicalCompare,

clang/include/clang/Basic/DiagnosticSemaKinds.td

 def note_for_range_invalid_iterator : Note <
   "in implicit call to 'operator%select{!=|*|++}0' for iterator of type %1">;
 def note_for_range_begin_end : Note<
   "selected '%select{begin|end}0' %select{function|template }1%2 with iterator type %3">;
 def warn_for_range_const_reference_copy : Warning<
   "loop variable %0 "
   "%diff{has type $ but is initialized with type $"
   "| is initialized with a value of a different type}1,2 resulting in a copy">,
-  InGroup<RangeLoopAnalysis>, DefaultIgnore;
+  InGroup<RangeLoopConstruct>, DefaultIgnore;
 def note_use_type_or_non_reference : Note<
   "use non-reference type %0 to keep the copy or type %1 to prevent copying">;
 def warn_for_range_variable_always_copy : Warning<
   "loop variable %0 is always a copy because the range of type %1 does not "
   "return a reference">,
-  InGroup<RangeLoopAnalysis>, DefaultIgnore;
+  InGroup<RangeLoopBindReference>, DefaultIgnore;
 def note_use_non_reference_type : Note<"use non-reference type %0">;
 def warn_for_range_copy : Warning<
   "loop variable %0 of type %1 creates a copy from type %2">,
-  InGroup<RangeLoopAnalysis>, DefaultIgnore;
+  InGroup<RangeLoopConstruct>, DefaultIgnore;
 def note_use_reference_type : Note<"use reference type %0 to prevent copying">;
 def err_objc_for_range_init_stmt : Error<
   "initialization statement is not supported when iterating over Objective-C "
   "collection">;
 
 // C++11 constexpr
 def warn_cxx98_compat_constexpr : Warning<
   "'constexpr' specifier is incompatible with C++98">,

clang/test/Misc/warning-wall.c

 RUN: diagtool tree -Wall > %t 2>&1
 RUN: FileCheck --input-file=%t %s
 
      CHECK:-Wall
 CHECK-NEXT:  -Wmost
 CHECK-NEXT:    -Wchar-subscripts
 CHECK-NEXT:    -Wcomment
 CHECK-NEXT:    -Wdelete-non-virtual-dtor
 CHECK-NEXT:      -Wdelete-non-abstract-non-virtual-dtor
 CHECK-NEXT:      -Wdelete-abstract-non-virtual-dtor
 CHECK-NEXT:    -Wformat
 CHECK-NEXT:      -Wformat-extra-args
 CHECK-NEXT:      -Wformat-zero-length
 CHECK-NEXT:      -Wnonnull
 CHECK-NEXT:      -Wformat-security
 CHECK-NEXT:      -Wformat-y2k
 CHECK-NEXT:      -Wformat-invalid-specifier
+CHECK-NEXT:    -Wfor-loop-analysis
 CHECK-NEXT:    -Wimplicit
 CHECK-NEXT:      -Wimplicit-function-declaration
 CHECK-NEXT:      -Wimplicit-int
 CHECK-NEXT:    -Winfinite-recursion
 CHECK-NEXT:    -Wint-in-bool-context
-CHECK-NEXT:    -Wloop-analysis
-CHECK-NEXT:      -Wfor-loop-analysis
-CHECK-NEXT:      -Wrange-loop-analysis
 CHECK-NEXT:    -Wmismatched-tags
 CHECK-NEXT:    -Wmissing-braces
 CHECK-NEXT:    -Wmove
 CHECK-NEXT:      -Wpessimizing-move
 CHECK-NEXT:      -Wredundant-move
 CHECK-NEXT:      -Wreturn-std-move
 CHECK-NEXT:      -Wself-move
 CHECK-NEXT:    -Wmultichar
+CHECK-NEXT:    -Wrange-loop-construct
 CHECK-NEXT:    -Wreorder
 CHECK-NEXT:      -Wreorder-ctor
 CHECK-NEXT:      -Wreorder-init-list
 CHECK-NEXT:    -Wreturn-type
 CHECK-NEXT:      -Wreturn-type-c-linkage
 CHECK-NEXT:    -Wself-assign
 CHECK-NEXT:      -Wself-assign-overloaded
 CHECK-NEXT:      -Wself-assign-field

 CHECK-NEXT:    -Wshift-op-parentheses
 CHECK-NEXT:    -Woverloaded-shift-op-parentheses
 CHECK-NEXT:    -Wparentheses-equality
 CHECK-NEXT:    -Wdangling-else
 CHECK-NEXT:  -Wswitch
 CHECK-NEXT:  -Wswitch-bool
 CHECK-NEXT:  -Wmisleading-indentation
 
 
 CHECK-NOT:-W

clang/test/SemaCXX/warn-range-loop-analysis.cpp

-// RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wall -Wno-unused -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wall -Wrange-loop-bind-reference -Wno-unused -verify %s
 // RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wloop-analysis -verify %s
 // RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wrange-loop-analysis -verify %s
 // RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wloop-analysis -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
 
 template <typename return_type>
 struct Iterator {
   return_type operator*();
   Iterator operator++();