[PATCH] D17244: [clang-tidy] readability-ternary-operator new check

[Richard via cfe-commits]

LegalizeAdulthood added inline comments.

================
Comment at: docs/clang-tidy/checks/readability-ternary-operator.rst:10
@@ +9,3 @@
+And it gets transformed into::
+  (condition ? expression0 : expression1);
+
----------------
Why add the parentheses around the entire statement?  They seem to serve no purpose.

What happens if the `condition` is an expression with the same or lower precedence than the ternary operator?

Most people seem to agree that `(a || b) ? e1 : e2;` is more readable than `(a || b ? e1 : e2);`

================
Comment at: docs/clang-tidy/checks/readability-ternary-operator.rst:13
@@ +12,3 @@
+Another situation when ternary operator would be suggested::
+  if (condition) return literal0; else return literal1;
+
----------------
[[ http://clang.llvm.org/extra/clang-tidy/checks/readability-simplify-boolean-expr.html | readability-simplify-boolean-expr ]] does this when the return type is `bool` and the return statements are returning `true` and `false`.

It also handles conditional assignment via ternary operator when the value assigned is of type `bool` and the resulting expressions are boolean constants.

If this check also starts modifying the same ternary operators, then we can have two checks fighting to make changes if both checks are applied on the same run of `clang-tidy`.  I'm unsure how best to resolve the conflict.  I don't know what `clang-tidy` would do in such instances.

================
Comment at: docs/clang-tidy/checks/readability-ternary-operator.rst:18
@@ +17,3 @@
+
+The autofixes are only suggested when no comments would be lost.
+
----------------
Are you also checking for intermediate preprocessor lines?

```
if (cond) {
#define BLAH 1
  return BLAH;
} else {
  return 0;
}
```

================
Comment at: docs/clang-tidy/checks/readability-ternary-operator.rst:24
@@ +23,2 @@
+both comments will be lost, therefore there will be no autofix suggested for
+such case. If you want the check to perform an autofix just remove the comments.
----------------
A slightly better wording:

If you want the check to perform an autofix, either reposition or remove the comments.

================
Comment at: test/clang-tidy/readability-ternary-operator.cpp:3
@@ +2,3 @@
+
+bool Condition = true;
+int Variable = 0;
----------------
You don't have any test cases where the condition is an expression with operators.  Please add some tests for this, particularly operators of the same or lower precedence than the ternary operator.


http://reviews.llvm.org/D17244