[PATCH] D43312: [clang-format] fix handling of consecutive unary operators
This revision was automatically updated to reflect the committed changes. Closed by commit rL326792: [clang-format] fix handling of consecutive unary operators (authored by krasimir, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D43312?vs=134511&id=137179#toc Repository: rL LLVM https://reviews.llvm.org/D43312 Files: cfe/trunk/lib/Format/TokenAnnotator.cpp cfe/trunk/unittests/Format/FormatTest.cpp cfe/trunk/unittests/Format/FormatTestJS.cpp Index: cfe/trunk/lib/Format/TokenAnnotator.cpp === --- cfe/trunk/lib/Format/TokenAnnotator.cpp +++ cfe/trunk/lib/Format/TokenAnnotator.cpp @@ -1531,10 +1531,8 @@ if (!PrevToken) return TT_UnaryOperator; -if (PrevToken->isOneOf(TT_CastRParen, TT_UnaryOperator) && -!PrevToken->is(tok::exclaim)) - // There aren't any trailing unary operators except for TypeScript's - // non-null operator (!). Thus, this must be squence of leading operators. +if (PrevToken->isOneOf(TT_CastRParen, TT_UnaryOperator)) + // This must be a sequence of leading unary operators. return TT_UnaryOperator; // Use heuristics to recognize unary operators. Index: cfe/trunk/unittests/Format/FormatTest.cpp === --- cfe/trunk/unittests/Format/FormatTest.cpp +++ cfe/trunk/unittests/Format/FormatTest.cpp @@ -5655,6 +5655,8 @@ verifyFormat("(a->f())++;"); verifyFormat("a[42]++;"); verifyFormat("if (!(a->f())) {\n}"); + verifyFormat("if (!+i) {\n}"); + verifyFormat("~&a;"); verifyFormat("a-- > b;"); verifyFormat("b ? -a : c;"); Index: cfe/trunk/unittests/Format/FormatTestJS.cpp === --- cfe/trunk/unittests/Format/FormatTestJS.cpp +++ cfe/trunk/unittests/Format/FormatTestJS.cpp @@ -2133,6 +2133,7 @@ verifyFormat("let x = foo!.bar();\n"); verifyFormat("let x = foo ? bar! : baz;\n"); verifyFormat("let x = !foo;\n"); + verifyFormat("if (!+a) {\n}"); verifyFormat("let x = foo[0]!;\n"); verifyFormat("let x = (foo)!;\n"); verifyFormat("let x = x(foo!);\n"); Index: cfe/trunk/lib/Format/TokenAnnotator.cpp === --- cfe/trunk/lib/Format/TokenAnnotator.cpp +++ cfe/trunk/lib/Format/TokenAnnotator.cpp @@ -1531,10 +1531,8 @@ if (!PrevToken) return TT_UnaryOperator; -if (PrevToken->isOneOf(TT_CastRParen, TT_UnaryOperator) && -!PrevToken->is(tok::exclaim)) - // There aren't any trailing unary operators except for TypeScript's - // non-null operator (!). Thus, this must be squence of leading operators. +if (PrevToken->isOneOf(TT_CastRParen, TT_UnaryOperator)) + // This must be a sequence of leading unary operators. return TT_UnaryOperator; // Use heuristics to recognize unary operators. Index: cfe/trunk/unittests/Format/FormatTest.cpp === --- cfe/trunk/unittests/Format/FormatTest.cpp +++ cfe/trunk/unittests/Format/FormatTest.cpp @@ -5655,6 +5655,8 @@ verifyFormat("(a->f())++;"); verifyFormat("a[42]++;"); verifyFormat("if (!(a->f())) {\n}"); + verifyFormat("if (!+i) {\n}"); + verifyFormat("~&a;"); verifyFormat("a-- > b;"); verifyFormat("b ? -a : c;"); Index: cfe/trunk/unittests/Format/FormatTestJS.cpp === --- cfe/trunk/unittests/Format/FormatTestJS.cpp +++ cfe/trunk/unittests/Format/FormatTestJS.cpp @@ -2133,6 +2133,7 @@ verifyFormat("let x = foo!.bar();\n"); verifyFormat("let x = foo ? bar! : baz;\n"); verifyFormat("let x = !foo;\n"); + verifyFormat("if (!+a) {\n}"); verifyFormat("let x = foo[0]!;\n"); verifyFormat("let x = (foo)!;\n"); verifyFormat("let x = x(foo!);\n"); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43312: [clang-format] fix handling of consecutive unary operators
kevinl added a comment. @krasimir ping? would appreciate you committing this for me. thanks! Repository: rC Clang https://reviews.llvm.org/D43312 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43312: [clang-format] fix handling of consecutive unary operators
kevinl added a comment. I do not have commit access. Would appreciate you committing it for me. Thanks! Repository: rC Clang https://reviews.llvm.org/D43312 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43312: [clang-format] fix handling of consecutive unary operators
krasimir accepted this revision. krasimir added a comment. This revision is now accepted and ready to land. Thank you! Do you have commit access or should I commit this for you? Repository: rC Clang https://reviews.llvm.org/D43312 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43312: [clang-format] fix handling of consecutive unary operators
kevinl added inline comments. Comment at: lib/Format/TokenAnnotator.cpp:1497 +!(PrevToken->is(tok::exclaim) && + Style.Language == FormatStyle::LK_JavaScript)) // There aren't any trailing unary operators except for TypeScript's krasimir wrote: > I think that TypeScript has both `if (!cond)` and `x!`. I'd expect that `if > (!+i) {\n}` is also handled in the TypeScript case. Could you add a test for > this for TypeScript please. Turns out the comment below was misleading. The `x!` operator is classified as `TT_JsNonNullAssertion` and not `TT_UnaryOperator`, so removing the check for `tok::exclaim` entirely fixes `if (!+i) {\n}` in TypeScript as well as C++. Tests are now added for both languages Repository: rC Clang https://reviews.llvm.org/D43312 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43312: [clang-format] fix handling of consecutive unary operators
kevinl updated this revision to Diff 134511. kevinl edited the summary of this revision. kevinl added a comment. Simplified the heuristic logic, which extends the fix to TypeScript as well Repository: rC Clang https://reviews.llvm.org/D43312 Files: lib/Format/TokenAnnotator.cpp unittests/Format/FormatTest.cpp unittests/Format/FormatTestJS.cpp Index: unittests/Format/FormatTestJS.cpp === --- unittests/Format/FormatTestJS.cpp +++ unittests/Format/FormatTestJS.cpp @@ -2132,6 +2132,7 @@ verifyFormat("let x = foo!.bar();\n"); verifyFormat("let x = foo ? bar! : baz;\n"); verifyFormat("let x = !foo;\n"); + verifyFormat("if (!+a) {\n}"); verifyFormat("let x = foo[0]!;\n"); verifyFormat("let x = (foo)!;\n"); verifyFormat("let x = x(foo!);\n"); Index: unittests/Format/FormatTest.cpp === --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -5655,6 +5655,8 @@ verifyFormat("(a->f())++;"); verifyFormat("a[42]++;"); verifyFormat("if (!(a->f())) {\n}"); + verifyFormat("if (!+i) {\n}"); + verifyFormat("~&a;"); verifyFormat("a-- > b;"); verifyFormat("b ? -a : c;"); Index: lib/Format/TokenAnnotator.cpp === --- lib/Format/TokenAnnotator.cpp +++ lib/Format/TokenAnnotator.cpp @@ -1492,10 +1492,8 @@ if (!PrevToken) return TT_UnaryOperator; -if (PrevToken->isOneOf(TT_CastRParen, TT_UnaryOperator) && -!PrevToken->is(tok::exclaim)) - // There aren't any trailing unary operators except for TypeScript's - // non-null operator (!). Thus, this must be squence of leading operators. +if (PrevToken->isOneOf(TT_CastRParen, TT_UnaryOperator)) + // This must be a sequence of leading unary operators. return TT_UnaryOperator; // Use heuristics to recognize unary operators. Index: unittests/Format/FormatTestJS.cpp === --- unittests/Format/FormatTestJS.cpp +++ unittests/Format/FormatTestJS.cpp @@ -2132,6 +2132,7 @@ verifyFormat("let x = foo!.bar();\n"); verifyFormat("let x = foo ? bar! : baz;\n"); verifyFormat("let x = !foo;\n"); + verifyFormat("if (!+a) {\n}"); verifyFormat("let x = foo[0]!;\n"); verifyFormat("let x = (foo)!;\n"); verifyFormat("let x = x(foo!);\n"); Index: unittests/Format/FormatTest.cpp === --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -5655,6 +5655,8 @@ verifyFormat("(a->f())++;"); verifyFormat("a[42]++;"); verifyFormat("if (!(a->f())) {\n}"); + verifyFormat("if (!+i) {\n}"); + verifyFormat("~&a;"); verifyFormat("a-- > b;"); verifyFormat("b ? -a : c;"); Index: lib/Format/TokenAnnotator.cpp === --- lib/Format/TokenAnnotator.cpp +++ lib/Format/TokenAnnotator.cpp @@ -1492,10 +1492,8 @@ if (!PrevToken) return TT_UnaryOperator; -if (PrevToken->isOneOf(TT_CastRParen, TT_UnaryOperator) && -!PrevToken->is(tok::exclaim)) - // There aren't any trailing unary operators except for TypeScript's - // non-null operator (!). Thus, this must be squence of leading operators. +if (PrevToken->isOneOf(TT_CastRParen, TT_UnaryOperator)) + // This must be a sequence of leading unary operators. return TT_UnaryOperator; // Use heuristics to recognize unary operators. ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43312: [clang-format] fix handling of consecutive unary operators
krasimir added inline comments. Comment at: lib/Format/TokenAnnotator.cpp:1497 +!(PrevToken->is(tok::exclaim) && + Style.Language == FormatStyle::LK_JavaScript)) // There aren't any trailing unary operators except for TypeScript's I think that TypeScript has both `if (!cond)` and `x!`. I'd expect that `if (!+i) {\n}` is also handled in the TypeScript case. Could you add a test for this for TypeScript please. Repository: rC Clang https://reviews.llvm.org/D43312 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43312: [clang-format] fix handling of consecutive unary operators
kevinl created this revision. kevinl added a reviewer: djasper. Herald added subscribers: cfe-commits, klimek. C++ code that used to be formatted as `if (! + object) {` is now formatted as `if (!+object) {` (we have a particular object in our codebase where unary `operator+` is overloaded to return the underlying value, which in this case is a `bool`) We still preserve the TypeScript behavior where `!` is a trailing non-null operator. (This is already tested by an existing unit test in `FormatTestJS.cpp`) It doesn't appear like handling of consecutive unary operators are tested in general? So I added another test for completeness Repository: rC Clang https://reviews.llvm.org/D43312 Files: lib/Format/TokenAnnotator.cpp unittests/Format/FormatTest.cpp Index: unittests/Format/FormatTest.cpp === --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -5655,6 +5655,8 @@ verifyFormat("(a->f())++;"); verifyFormat("a[42]++;"); verifyFormat("if (!(a->f())) {\n}"); + verifyFormat("if (!+i) {\n}"); + verifyFormat("~&a;"); verifyFormat("a-- > b;"); verifyFormat("b ? -a : c;"); Index: lib/Format/TokenAnnotator.cpp === --- lib/Format/TokenAnnotator.cpp +++ lib/Format/TokenAnnotator.cpp @@ -1493,7 +1493,8 @@ return TT_UnaryOperator; if (PrevToken->isOneOf(TT_CastRParen, TT_UnaryOperator) && -!PrevToken->is(tok::exclaim)) +!(PrevToken->is(tok::exclaim) && + Style.Language == FormatStyle::LK_JavaScript)) // There aren't any trailing unary operators except for TypeScript's // non-null operator (!). Thus, this must be squence of leading operators. return TT_UnaryOperator; Index: unittests/Format/FormatTest.cpp === --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -5655,6 +5655,8 @@ verifyFormat("(a->f())++;"); verifyFormat("a[42]++;"); verifyFormat("if (!(a->f())) {\n}"); + verifyFormat("if (!+i) {\n}"); + verifyFormat("~&a;"); verifyFormat("a-- > b;"); verifyFormat("b ? -a : c;"); Index: lib/Format/TokenAnnotator.cpp === --- lib/Format/TokenAnnotator.cpp +++ lib/Format/TokenAnnotator.cpp @@ -1493,7 +1493,8 @@ return TT_UnaryOperator; if (PrevToken->isOneOf(TT_CastRParen, TT_UnaryOperator) && -!PrevToken->is(tok::exclaim)) +!(PrevToken->is(tok::exclaim) && + Style.Language == FormatStyle::LK_JavaScript)) // There aren't any trailing unary operators except for TypeScript's // non-null operator (!). Thus, this must be squence of leading operators. return TT_UnaryOperator; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits