[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

PiotrZSL abandoned this revision. PiotrZSL added a comment. Obsolete by D144522 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D31308/new/ https://reviews.llvm.org/D31308 ___ cfe-commits mailing list cfe-commi

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. Herald added a subscriber: xazax.hun. As noted above, my concern with the current implementation is that the use of AST in this check seems to be superfluous. It should be enough to h

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

Prazek added a comment. Yep, I am also a fan of "and","or" and "not". The other tokens doesn't make it more readable imho https://reviews.llvm.org/D31308 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

aaron.ballman added inline comments. Comment at: clang-tidy/readability/OperatorsRepresentationCheck.cpp:34 + + if (const auto *B = Result.Nodes.getNodeAs("binary")) { +switch (B->getOpcode()) { alexfh wrote: > Eugene.Zelenko wrote: > > aaron.ballman wrote:

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

alexfh added inline comments. Comment at: clang-tidy/readability/OperatorsRepresentationCheck.cpp:34 + + if (const auto *B = Result.Nodes.getNodeAs("binary")) { +switch (B->getOpcode()) { Eugene.Zelenko wrote: > aaron.ballman wrote: > > alexfh wrote: > > > a

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

Eugene.Zelenko added inline comments. Comment at: clang-tidy/readability/OperatorsRepresentationCheck.cpp:34 + + if (const auto *B = Result.Nodes.getNodeAs("binary")) { +switch (B->getOpcode()) { aaron.ballman wrote: > alexfh wrote: > > aaron.ballman wrote:

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

aaron.ballman added inline comments. Comment at: clang-tidy/readability/OperatorsRepresentationCheck.cpp:34 + + if (const auto *B = Result.Nodes.getNodeAs("binary")) { +switch (B->getOpcode()) { alexfh wrote: > aaron.ballman wrote: > > mgehre wrote: > > > aa

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

mgehre updated this revision to Diff 93519. mgehre added a comment. only check C++ code; only match operators that can have alternative representations https://reviews.llvm.org/D31308 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/OperatorsRepresentationCheck.cpp cla

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

alexfh added inline comments. Comment at: clang-tidy/readability/OperatorsRepresentationCheck.cpp:34 + + if (const auto *B = Result.Nodes.getNodeAs("binary")) { +switch (B->getOpcode()) { aaron.ballman wrote: > mgehre wrote: > > aaron.ballman wrote: > > > al

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

aaron.ballman added inline comments. Comment at: clang-tidy/readability/OperatorsRepresentationCheck.cpp:34 + + if (const auto *B = Result.Nodes.getNodeAs("binary")) { +switch (B->getOpcode()) { mgehre wrote: > aaron.ballman wrote: > > alexfh wrote: > > > aa

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

mgehre updated this revision to Diff 93309. mgehre added a comment. Improved diagnostic; updated documentation; added test for overloaded operator https://reviews.llvm.org/D31308 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/OperatorsRepresentationCheck.cpp clang-tid

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

mgehre added inline comments. Comment at: clang-tidy/readability/OperatorsRepresentationCheck.cpp:34 + + if (const auto *B = Result.Nodes.getNodeAs("binary")) { +switch (B->getOpcode()) { aaron.ballman wrote: > alexfh wrote: > > aaron.ballman wrote: > > > I

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

aaron.ballman added inline comments. Comment at: clang-tidy/readability/OperatorsRepresentationCheck.cpp:34 + + if (const auto *B = Result.Nodes.getNodeAs("binary")) { +switch (B->getOpcode()) { alexfh wrote: > aaron.ballman wrote: > > I think this would mak

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

alexfh added inline comments. Comment at: clang-tidy/readability/OperatorsRepresentationCheck.cpp:34 + + if (const auto *B = Result.Nodes.getNodeAs("binary")) { +switch (B->getOpcode()) { aaron.ballman wrote: > I think this would make more sense lifted into

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

aaron.ballman added inline comments. Comment at: clang-tidy/readability/OperatorsRepresentationCheck.cpp:34 + + if (const auto *B = Result.Nodes.getNodeAs("binary")) { +switch (B->getOpcode()) { I think this would make more sense lifted into an AST matcher -

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

mgehre updated this revision to Diff 93057. mgehre added a comment. Rename check to readability-operators-representation https://reviews.llvm.org/D31308 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/OperatorsRepresentationCheck.cpp clang-tidy/readability/OperatorsRep

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

gnzlbg added a comment. Thanks for working on this! > I'm not sure if it would be helpful to have this check in both ways. I did a > code search for "not_eq", "bitand" and "and_eq" on github, and their usage > seems to be a clear minority. I actually was requesting the opposite version of this

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

alexfh added a comment. FWIW, I'm pretty sure this can and should be done on the lexer level - it will be faster and more universal. Repository: rL LLVM https://reviews.llvm.org/D31308 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

alexfh added a comment. > So I would propose to keep the features as-is for now, > change the name to readability-operators-representation, and then later > (someone else?) might also add an option > for making this work the other way around. Would that be ok for you? Fine by me. Repository:

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

alexfh added a comment. In https://reviews.llvm.org/D31308#709709, @mgehre wrote: > Thanks for your feedback, Eugene! > > I'm not sure if it would be helpful to have this check in both ways. I did a > code search for "not_eq", "bitand" and "and_eq" > on github, and their usage seems to be a cle

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

mgehre added a comment. Thanks for your feedback, Eugene! I'm not sure if it would be helpful to have this check in both ways. I did a code search for "not_eq", "bitand" and "and_eq" on github, and their usage seems to be a clear minority. So I would propose to keep the features as-is for now,

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

Eugene.Zelenko added a comment. I think will be good idea to make this check working in both ways (standard or alternative operator representations), depending on option. Probably readability-operators-representation will be better name for check. See also PR25195. https://reviews.llvm.org/D3

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

mgehre created this revision. Herald added a subscriber: mgorny. Flags (and replaces) the alternative tokens for binary and unary operators, such as ``not`` (for ``!``), ``bitand`` (for ``&``), ``or`` (for ``||``) or ``not_eq`` (for ``!=``). https://reviews.llvm.org/D31308 Files: clang-tidy/