@@ -1,7 +1,8 @@
 /**
- * @name Ambiguous assignment of comparison in condition
- * @description Assigning the result of an unparenthesized comparison in a condition may indicate
- *              that the assignment and comparison are grouped incorrectly.
+ * @name Ambiguous assignment of comparison used as truth value
+ * @description Assigning the result of an unparenthesized comparison when the assignment is used
+ *              as a truth value may indicate that the assignment and comparison are grouped
+ *              incorrectly.
  * @kind problem
  * @problem.severity warning
  * @precision high
@@ -14,23 +15,36 @@
 
 import cpp
 
-/** Gets a condition that controls branching. */
-private Expr getACondition() {
-  result = any(IfStmt s).getCondition()
+/** Holds if the value of `expression` is directly used as a truth value. */
+private predicate isDirectlyUsedAsTruthValue(Expr expression) {
+  expression.isCondition()
   or
-  result = any(Loop s).getCondition()
+  expression = any(UnaryLogicalOperation operation).getAnOperand()
   or
-  result = any(ConditionalExpr e).getCondition()
+  expression = any(BinaryLogicalOperation operation).getAnOperand()
 }
 
 /**
- * Holds if `assignment` occurs within a condition that controls branching.
- *
- * This includes nested expressions, such as function arguments and either operand of a comma
- * expression, because the ambiguous syntax still occurs within the condition.
+ * Holds if the value of `expression` is used as a truth value, possibly after contributing to a
+ * comma, conditional, or comparison expression.
  */
-private predicate occursInCondition(Assignment assignment) {
-  assignment.getParent*() = getACondition()
+private predicate isUsedAsTruthValue(Expr expression) {
+  isDirectlyUsedAsTruthValue(expression)
+  or
+  exists(CommaExpr comma |
+    expression = comma.getRightOperand() and
+    isUsedAsTruthValue(comma)
+  )
+  or
+  exists(ConditionalExpr conditional |
+    expression = [conditional.getThen(), conditional.getElse()] and
+    isUsedAsTruthValue(conditional)
+  )
+  or
+  exists(ComparisonOperation comparison |
+    expression = comparison.getAnOperand() and
+    isUsedAsTruthValue(comparison)
+  )
 }
 
 /**
@@ -46,11 +60,13 @@ from Assignment assignment, ComparisonOperation comparison
 where
   assignment.getRValue() = comparison and
   not isExplicitlyGrouped(comparison) and
-  occursInCondition(assignment) and
-  // Assigning a comparison result to a Boolean is normally intentional.
+  isUsedAsTruthValue(assignment) and
+  // A Boolean lvalue makes assigning the comparison result type-appropriate and normally
+  // intentional.
   not assignment.getLValue().getUnspecifiedType() instanceof BoolType and
   not assignment.isUnevaluated() and
   not assignment.isFromUninstantiatedTemplate(_)
 select assignment,
   "The '" + assignment.getOperator() +
-    "' operation assigns the result of an unparenthesized comparison used in a condition."
+    "' operation assigns the result of an unparenthesized comparison, and its result is used as " +
+    "a truth value."
@@ -2,4 +2,4 @@
 category: newQuery
 ---
 * Added a new query, `cpp/ambiguous-assignment-of-comparison`, to detect assignments of
-  unparenthesized comparison results in conditions.
+  unparenthesized comparison results when the assignment is used as a truth value.
@@ -1,24 +1,36 @@
-| test.c:6:8:6:31 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison used in a condition. |
-| test.c:13:30:13:54 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison used in a condition. |
-| test.c:20:8:20:33 | ... /= ... | The '/=' operation assigns the result of an unparenthesized comparison used in a condition. |
-| test.c:27:8:27:33 | ... %= ... | The '%=' operation assigns the result of an unparenthesized comparison used in a condition. |
-| test.c:34:8:34:32 | ... \|= ... | The '\|=' operation assigns the result of an unparenthesized comparison used in a condition. |
-| test.c:41:8:41:33 | ... >>= ... | The '>>=' operation assigns the result of an unparenthesized comparison used in a condition. |
-| test.c:51:3:51:26 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison used in a condition. |
-| test.cpp:8:8:8:30 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison used in a condition. |
-| test.cpp:15:11:15:34 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison used in a condition. |
-| test.cpp:22:7:22:29 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison used in a condition. |
-| test.cpp:29:11:29:40 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison used in a condition. |
-| test.cpp:36:8:36:30 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison used in a condition. |
-| test.cpp:43:11:43:33 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison used in a condition. |
-| test.cpp:48:8:48:33 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison used in a condition. |
-| test.cpp:55:8:55:32 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison used in a condition. |
-| test.cpp:64:13:64:35 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison used in a condition. |
-| test.cpp:70:29:70:51 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison used in a condition. |
-| test.cpp:77:8:77:30 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison used in a condition. |
-| test.cpp:84:8:84:38 | ... <<= ... | The '<<=' operation assigns the result of an unparenthesized comparison used in a condition. |
-| test.cpp:91:9:91:31 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison used in a condition. |
-| test.cpp:101:3:101:20 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison used in a condition. |
-| test.cpp:253:8:253:24 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison used in a condition. |
-| test.cpp:280:16:280:38 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison used in a condition. |
-| test.cpp:287:8:287:30 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison used in a condition. |
+| test.c:6:8:6:31 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.c:13:30:13:54 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.c:20:8:20:33 | ... /= ... | The '/=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.c:27:8:27:33 | ... %= ... | The '%=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.c:34:8:34:32 | ... \|= ... | The '\|=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.c:41:8:41:33 | ... >>= ... | The '>>=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.c:51:3:51:26 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.c:102:28:102:51 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.c:109:15:109:38 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.c:116:49:116:72 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.c:130:11:130:34 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.cpp:8:8:8:30 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.cpp:15:11:15:34 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.cpp:22:7:22:29 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.cpp:29:11:29:40 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.cpp:36:8:36:30 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.cpp:43:11:43:33 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.cpp:48:8:48:33 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.cpp:55:8:55:32 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.cpp:64:13:64:35 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.cpp:70:29:70:51 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.cpp:77:8:77:30 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.cpp:84:8:84:38 | ... <<= ... | The '<<=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.cpp:91:9:91:31 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.cpp:101:3:101:20 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.cpp:253:8:253:24 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.cpp:294:27:294:49 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.cpp:301:27:301:49 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.cpp:308:9:308:31 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.cpp:315:15:315:37 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.cpp:322:23:322:45 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.cpp:329:47:329:69 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.cpp:343:47:343:69 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.cpp:350:11:350:33 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.cpp:355:12:355:34 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.cpp:360:14:360:36 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
@@ -82,10 +82,55 @@ int c_boolean_result_assignment(void) {
 
 int c_switch_expression(void) {
   int value;
-  switch (value = read_value() < 0) { // GOOD: This query only covers branching conditions.
+  switch (value = read_value() < 0) { // GOOD: The switch operand is not used as a truth value.
   case 0:
     return value;
   default:
     return 0;
   }
 }
+
+int c_discarded_assignment_in_comma_expression(void) {
+  int value;
+  if ((value = read_value() < 0, read_other_value())) // GOOD: The assignment result is discarded.
+    return value;
+  return 0;
+}
+
+int c_truth_valued_assignment_in_comma_expression(void) {
+  int value;
+  if ((read_other_value(), value = read_value() < 0)) // $ Alert // BAD
+    return value;
+  return 0;
+}
+
+int c_truth_valued_conditional_then_arm(int flag) {
+  int value;
+  if (flag ? (value = read_value() < 0) : 0) // $ Alert // BAD
+    return value;
+  return 0;
+}
+
+int c_nested_truth_valued_comma_expression(void) {
+  int value;
+  if ((read_other_value(), (read_other_value(), value = read_value() < 0))) // $ Alert // BAD
+    return value;
+  return 0;
+}
+
+int c_nested_discarded_comma_expression(void) {
+  int value;
+  if (((read_other_value(), value = read_value() < 0), read_other_value())) // GOOD: Discarded.
+    return value;
+  return 0;
+}
+
+int c_logical_value_outside_branch(void) {
+  int value;
+  return (value = read_value() < 0) && read_other_value(); // $ Alert // BAD
+}
+
+int c_returned_assignment(void) {
+  int value;
+  return value = read_value() < 0; // GOOD: The assignment result is not used as a truth value.
+}
@@ -1,7 +1,7 @@
 int get_value();
 int get_other_value();
 void *get_pointer();
-bool identity(bool value);
+bool check(int value);
 
 int direct_if() {
   int value;
@@ -74,7 +74,7 @@ int logical_or() {
 
 int nested_comparison() {
   int value;
-  if ((value = get_value() < 0) == 1) // $ Alert // BAD
+  if ((value = get_value() < 0) == false) // $ Alert // BAD
     return value;
   return 0;
 }
@@ -173,7 +173,7 @@ int compound_assignment_without_comparison() {
 
 int switch_expression() {
   int value;
-  switch (value = get_value() < 0) { // GOOD: This query only covers branching conditions.
+  switch (value = get_value() < 0) { // GOOD: The switch operand is not used as a truth value.
   case 0:
     return value;
   default:
@@ -277,14 +277,87 @@ int overloaded_comparison() {
 
 int assignment_as_call_argument() {
   int value;
-  if (identity(value = get_value() < 0)) // $ Alert // BAD: The ambiguous syntax is still in a condition.
+  if (check(value = get_value() < 0)) // GOOD: The assignment only computes an integer argument.
     return value;
   return 0;
 }
 
 int discarded_assignment_in_comma_expression() {
   int value;
-  if ((value = get_value() < 0, get_other_value())) // $ Alert // BAD: Still ambiguous syntax in a condition.
+  if ((value = get_value() < 0, get_other_value())) // GOOD: The assignment result is discarded.
+    return value;
+  return 0;
+}
+
+int truth_valued_assignment_in_comma_expression() {
+  int value;
+  if ((get_other_value(), value = get_value() < 0)) // $ Alert // BAD
+    return value;
+  return 0;
+}
+
+int comma_assignment_under_comparison() {
+  int value;
+  if ((get_other_value(), value = get_value() < 0) == false) // $ Alert // BAD
+    return value;
+  return 0;
+}
+
+int nested_truth_valued_comparisons() {
+  int value;
+  if (((value = get_value() < 0) == false) == false) // $ Alert // BAD
+    return value;
+  return 0;
+}
+
+int truth_valued_conditional_then_arm(bool flag) {
+  int value;
+  if (flag ? (value = get_value() < 0) : false) // $ Alert // BAD
+    return value;
+  return 0;
+}
+
+int truth_valued_conditional_else_arm(bool flag) {
+  int value;
+  if (flag ? false : (value = get_value() < 0)) // $ Alert // BAD
+    return value;
+  return 0;
+}
+
+int nested_truth_valued_comma_expression() {
+  int value;
+  if ((get_other_value(), (get_other_value(), value = get_value() < 0))) // $ Alert // BAD
+    return value;
+  return 0;
+}
+
+int nested_discarded_comma_expression() {
+  int value;
+  if (((get_other_value(), value = get_value() < 0), get_other_value())) // GOOD: Discarded.
+    return value;
+  return 0;
+}
+
+int nested_comma_assignment_under_comparison() {
+  int value;
+  if ((get_other_value(), (get_other_value(), value = get_value() < 0)) == false) // $ Alert // BAD
+    return value;
+  return 0;
+}
+
+bool logical_value_outside_branch() {
+  int value;
+  return (value = get_value() < 0) && get_other_value(); // $ Alert // BAD
+}
+
+bool logical_not_outside_branch() {
+  int value;
+  return !(value = get_value() < 0); // $ Alert // BAD
+}
+
+int truth_valued_assignment_inside_call_argument() {
+  int value;
+  if (check((value = get_value() < 0) && get_other_value())) // $ Alert // BAD
     return value;
   return 0;
 }