[clang][analyzer] Improve documentation of checker 'cplusplus.Move' (NFC)

[balazske]

No description provided.

[llvmbot]

@llvm/pr-subscribers-clang-static-analyzer-1

@llvm/pr-subscribers-clang

Author: Balázs Kéri (balazske)

Changes

---

Full diff: https://github.com/llvm/llvm-project/pull/96295.diff

2 Files Affected:

• (modified) clang/docs/analyzer/checkers.rst (+35-4)
• (modified) clang/include/clang/StaticAnalyzer/Checkers/Checkers.td (+5-16)

diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index b8d5f372bdf61..445f434e1e6ce 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -420,21 +420,52 @@ around, such as ``std::string_view``.
 
 cplusplus.Move (C++)
 """"""""""""""""""""
-Method calls on a moved-from object and copying a moved-from object will be reported.
-
+Find use-after-move bugs in C++. This includes method calls on moved-from
+objects, assignment of a moved-from object, and repeated move of a moved-from
+object.
 
 .. code-block:: cpp
 
-  struct A {
+ struct A {
    void foo() {}
  };
 
- void f() {
+ void f1() {
    A a;
    A b = std::move(a); // note: 'a' became 'moved-from' here
    a.foo();            // warn: method call on a 'moved-from' object 'a'
  }
 
+ void f2() {
+   A a;
+   A b = std::move(a);
+   A c(std::move(a)); // warn: move of an already moved-from object
+ }
+
+ void f3() {
+   A a;
+   A b = std::move(a);
+   b = a; // warn: copy of moved-from object
+ }
+
+The checker option ``WarnOn`` controls on what objects the use-after-move is
+checked. The most strict value is ``KnownsOnly``, in this mode only objects are
+checked whose type is known to be move-unsafe. These include most STL objects
+(but excluding move-safe ones) and smart pointers. With option value
+``KnownsAndLocals`` local variables (of any type) are additionally checked. The
+idea behind this is that local variables are usually not tempting to be re-used
+so an use after move is more likely a bug than with member variables. With
+option value ``All`` any use-after move condition is checked on all kinds of
+variables, excluding global variables and known move-safe cases. Default value
+is ``KnownsAndLocals``.
+
+Call of methods named ``empty()`` or ``isEmpty()`` are allowed on moved-from
+objects because these methods are considered as move-safe. Functions called
+``reset()``, ``destroy()``, ``clear()``, ``assign``, ``resize``,  ``shrink`` are
+treated as state-reset functions and are allowed on moved-from objects, these
+make the object valid again. This applies to any type of object (not only STL
+ones).
+
 .. _cplusplus-NewDelete:
 
 cplusplus.NewDelete (C++)
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 429c334a0b24b..6e224a4e098ad 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -686,22 +686,11 @@ def MoveChecker: Checker<"Move">,
   CheckerOptions<[
     CmdLineOption<String,
                   "WarnOn",
-                  "In non-aggressive mode, only warn on use-after-move of "
-                  "local variables (or local rvalue references) and of STL "
-                  "objects. The former is possible because local variables (or "
-                  "local rvalue references) are not tempting their user to "
-                  "re-use the storage. The latter is possible because STL "
-                  "objects are known to end up in a valid but unspecified "
-                  "state after the move and their state-reset methods are also "
-                  "known, which allows us to predict precisely when "
-                  "use-after-move is invalid. Some STL objects are known to "
-                  "conform to additional contracts after move, so they are not "
-                  "tracked. However, smart pointers specifically are tracked "
-                  "because we can perform extra checking over them. In "
-                  "aggressive mode, warn on any use-after-move because the "
-                  "user has intentionally asked us to completely eliminate "
-                  "use-after-move in his code. Values: \"KnownsOnly\", "
-                  "\"KnownsAndLocals\", \"All\".",
+                  "With setting \"KnownsOnly\" warn only on objects with known "
+                  "move semantics like smart pointers and other STL objects. "
+                  "With setting \"KnownsAndLocals\" warn additionally on local "
+                  "variables (or rvalue references). With setting \"All\" warn "
+                  "on all variables (excluding global variables).",
                   "KnownsAndLocals",
                   Released>
   ]>,

[NagyDonat]

LGTM, thanks for updating the docs!

I only have one minor inline remark about formatting.

[NagyDonat]

Suggested change

	The checker option ``WarnOn`` controls on what objects the use-after-move is
	checked. The most strict value is ``KnownsOnly``, in this mode only objects are
	checked whose type is known to be move-unsafe. These include most STL objects
	(but excluding move-safe ones) and smart pointers. With option value
	``KnownsAndLocals`` local variables (of any type) are additionally checked. The
	idea behind this is that local variables are usually not tempting to be re-used
	so an use after move is more likely a bug than with member variables. With
	option value ``All`` any use-after move condition is checked on all kinds of
	variables, excluding global variables and known move-safe cases. Default value
	is ``KnownsAndLocals``.
	The checker option ``WarnOn`` controls on what objects the use-after-move is
	checked.
	* The most strict value is ``KnownsOnly``, in this mode only objects are
	checked whose type is known to be move-unsafe. These include most STL objects
	(but excluding move-safe ones) and smart pointers.
	* With option value ``KnownsAndLocals`` local variables (of any type) are
	additionally checked. The idea behind this is that local variables are
	usually not tempting to be re-used so an use after move is more likely a bug
	than with member variables.
	* With option value ``All`` any use-after move condition is checked on all
	kinds of variables, excluding global variables and known move-safe cases.
	Default value is ``KnownsAndLocals``.

This paragraph was very long, re-flow it into a bullet point list.

[balazske]

I fixed a test that contained the entire option help description. I think this is not needed, removed it and only included the first line of the description.

[NagyDonat]

The new changes also LGTM, feel free to merge this when you want.