[PATCH] D73434: [Sema] Remove a -Wrange warning from -Wall

[Mark de Wever via Phabricator via cfe-commits]

Mordante created this revision.
Mordante added reviewers: aaron.ballman, aaronpuchert, rtrieu, xbolva00.
Mordante added a project: clang.

During the review of D73007 <https://reviews.llvm.org/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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D73434

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/test/Misc/warning-wall.c
  clang/test/SemaCXX/warn-range-loop-analysis.cpp


Index: clang/test/SemaCXX/warn-range-loop-analysis.cpp
===================================================================
--- clang/test/SemaCXX/warn-range-loop-analysis.cpp
+++ clang/test/SemaCXX/warn-range-loop-analysis.cpp
@@ -1,4 +1,4 @@
-// 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
Index: clang/test/Misc/warning-wall.c
===================================================================
--- clang/test/Misc/warning-wall.c
+++ clang/test/Misc/warning-wall.c
@@ -15,14 +15,12 @@
 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
@@ -31,6 +29,7 @@
 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
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2381,17 +2381,17 @@
   "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 "
Index: clang/include/clang/Basic/DiagnosticGroups.td
===================================================================
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -384,7 +384,10 @@
 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]>;
@@ -858,14 +861,15 @@
     Comment,
     DeleteNonVirtualDtor,
     Format,
+    ForLoopAnalysis,
     Implicit,
     InfiniteRecursion,
     IntInBoolContext,
-    LoopAnalysis,
     MismatchedTags,
     MissingBraces,
     Move,
     MultiChar,
+    RangeLoopConstruct,
     Reorder,
     ReturnType,
     SelfAssignment,


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D73434.240447.patch
Type: text/x-patch
Size: 4284 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200126/701204d8/attachment-0001.bin>