[PATCH] D144522: [clang-tidy] Add readability-operators-representation check

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added inline comments.


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/readability/operators-representation.rst:16
+To configure the check to enforce the traditional token representation, you can
+set the `BinaryOperators` and `OverloadedOperators` options to ``&&,||,!``,
+respectively. This will cause the check to warn on any occurrences of ``and``,
----------------
carlosgalvezp wrote:
> Is there any reason why this distinction is needed? The usage of the operator happens in client code - it's unlikely one would want different style depending on whether the operator is overloaded or not?
> 
> ```
> bool x = some_bool and some_other_bool;
> bool y = some_object && some_other_object;
> ```
Consider code like this:

```
template<typename T>
void archive(T&& a, std::optional<int>& v)
{
    a & v;
}

and

bool test(unsigned value, unsigned mask)
{
    return value & mask;
}
```

Those are 2 different use-cases, probably you don't want to use bit_and, but you could use bit_and for value & mask.


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/readability/operators-representation.rst:28-29
+
+Alternative Token Representation
+--------------------------------
+
----------------
carlosgalvezp wrote:
> Personally I find this to be a matter of style, and arguments could be found for either style. As such I don't think this check should promote the style preference of the author of the check.
> 
> It's like having a written discussion in the clang-format option `ColumnLimit` explaining why N characters is a better choice than M characters. In the end this is a decision each project should take, and the check should not have a bias towards a preferred choice.
Following this way of thinking none of value is correct. Always will be group that is for it or against it. Initially this check had to be only to enforce alternative tokens but then made it configurable and change name of check. This check is for "readability" and alternative tokens are more readable. If someone don't like them, their choose, they can change config to their needs.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144522/new/

https://reviews.llvm.org/D144522