[PATCH] D120188: Fix extraneous whitespace addition in line comments on clang-format directives

[Luis Penagos via Phabricator via cfe-commits]

penagos created this revision.
penagos requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120188

Files:
  clang/lib/Format/BreakableToken.cpp
  clang/unittests/Format/FormatTestComments.cpp


Index: clang/unittests/Format/FormatTestComments.cpp
===
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -91,6 +91,12 @@
"// line 2\n"
"void f() {}\n");
 
+  EXPECT_EQ("// comment\n"
+"// clang-format on\n",
+format("//comment\n"
+   "// clang-format on\n",
+   getLLVMStyleWithColumns(20)));
+
   verifyFormat("void f() {\n"
"  // Doesn't do anything\n"
"}");
Index: clang/lib/Format/BreakableToken.cpp
===
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -816,9 +816,10 @@
 assert(Lines[i].size() > IndentPrefix.size());
 const auto FirstNonSpace = Lines[i][IndentPrefix.size()];
 const auto AllowsSpaceChange =
-SpacesInPrefix != 0 ||
-(!NoSpaceBeforeFirstCommentChar() ||
- (FirstNonSpace == '}' && FirstLineSpaceChange != 0));
+(!LineTok || !switchesFormatting(*LineTok)) &&
+(SpacesInPrefix != 0 ||
+ (!NoSpaceBeforeFirstCommentChar() ||
+  (FirstNonSpace == '}' && FirstLineSpaceChange != 0)));
 
 if (PrefixSpaceChange[i] > 0 && AllowsSpaceChange) {
   Prefix[i] = IndentPrefix.str();


Index: clang/unittests/Format/FormatTestComments.cpp
===
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -91,6 +91,12 @@
"// line 2\n"
"void f() {}\n");
 
+  EXPECT_EQ("// comment\n"
+"// clang-format on\n",
+format("//comment\n"
+   "// clang-format on\n",
+   getLLVMStyleWithColumns(20)));
+
   verifyFormat("void f() {\n"
"  // Doesn't do anything\n"
"}");
Index: clang/lib/Format/BreakableToken.cpp
===
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -816,9 +816,10 @@
 assert(Lines[i].size() > IndentPrefix.size());
 const auto FirstNonSpace = Lines[i][IndentPrefix.size()];
 const auto AllowsSpaceChange =
-SpacesInPrefix != 0 ||
-(!NoSpaceBeforeFirstCommentChar() ||
- (FirstNonSpace == '}' && FirstLineSpaceChange != 0));
+(!LineTok || !switchesFormatting(*LineTok)) &&
+(SpacesInPrefix != 0 ||
+ (!NoSpaceBeforeFirstCommentChar() ||
+  (FirstNonSpace == '}' && FirstLineSpaceChange != 0)));
 
 if (PrefixSpaceChange[i] > 0 && AllowsSpaceChange) {
   Prefix[i] = IndentPrefix.str();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D120188: Fix extraneous whitespace addition in line comments on clang-format directives

[Luis Penagos via Phabricator via cfe-commits]

penagos updated this revision to Diff 410115.
penagos added a comment.

- Split AllowsSpaceChange conditional over multiple variables to enhance 
readability.
- Remove unnecessary column limit from unittest.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120188

Files:
  clang/lib/Format/BreakableToken.cpp
  clang/unittests/Format/FormatTestComments.cpp


Index: clang/unittests/Format/FormatTestComments.cpp
===
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -91,6 +91,11 @@
"// line 2\n"
"void f() {}\n");
 
+  EXPECT_EQ("// comment\n"
+"// clang-format on\n",
+format("//comment\n"
+   "// clang-format on\n"));
+
   verifyFormat("void f() {\n"
"  // Doesn't do anything\n"
"}");
Index: clang/lib/Format/BreakableToken.cpp
===
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -815,10 +815,13 @@
 
 assert(Lines[i].size() > IndentPrefix.size());
 const auto FirstNonSpace = Lines[i][IndentPrefix.size()];
-const auto AllowsSpaceChange =
-SpacesInPrefix != 0 ||
-(!NoSpaceBeforeFirstCommentChar() ||
- (FirstNonSpace == '}' && FirstLineSpaceChange != 0));
+const bool IsFormatComment = LineTok && switchesFormatting(*LineTok);
+const bool LineRequiresLeadingSpace =
+!NoSpaceBeforeFirstCommentChar() ||
+(FirstNonSpace == '}' && FirstLineSpaceChange != 0);
+const bool AllowsSpaceChange =
+!IsFormatComment &&
+(SpacesInPrefix != 0 || LineRequiresLeadingSpace);
 
 if (PrefixSpaceChange[i] > 0 && AllowsSpaceChange) {
   Prefix[i] = IndentPrefix.str();


Index: clang/unittests/Format/FormatTestComments.cpp
===
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -91,6 +91,11 @@
"// line 2\n"
"void f() {}\n");
 
+  EXPECT_EQ("// comment\n"
+"// clang-format on\n",
+format("//comment\n"
+   "// clang-format on\n"));
+
   verifyFormat("void f() {\n"
"  // Doesn't do anything\n"
"}");
Index: clang/lib/Format/BreakableToken.cpp
===
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -815,10 +815,13 @@
 
 assert(Lines[i].size() > IndentPrefix.size());
 const auto FirstNonSpace = Lines[i][IndentPrefix.size()];
-const auto AllowsSpaceChange =
-SpacesInPrefix != 0 ||
-(!NoSpaceBeforeFirstCommentChar() ||
- (FirstNonSpace == '}' && FirstLineSpaceChange != 0));
+const bool IsFormatComment = LineTok && switchesFormatting(*LineTok);
+const bool LineRequiresLeadingSpace =
+!NoSpaceBeforeFirstCommentChar() ||
+(FirstNonSpace == '}' && FirstLineSpaceChange != 0);
+const bool AllowsSpaceChange =
+!IsFormatComment &&
+(SpacesInPrefix != 0 || LineRequiresLeadingSpace);
 
 if (PrefixSpaceChange[i] > 0 && AllowsSpaceChange) {
   Prefix[i] = IndentPrefix.str();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D120188: Fix extraneous whitespace addition in line comments on clang-format directives

[Luis Penagos via Phabricator via cfe-commits]

penagos marked 3 inline comments as done.
penagos added inline comments.



Comment at: clang/lib/Format/BreakableToken.cpp:818-822
 const auto AllowsSpaceChange =
-SpacesInPrefix != 0 ||
-(!NoSpaceBeforeFirstCommentChar() ||
- (FirstNonSpace == '}' && FirstLineSpaceChange != 0));
+(!LineTok || !switchesFormatting(*LineTok)) &&
+(SpacesInPrefix != 0 ||
+ (!NoSpaceBeforeFirstCommentChar() ||
+  (FirstNonSpace == '}' && FirstLineSpaceChange != 0)));

curdeius wrote:
> curdeius wrote:
> > HazardyKnusperkeks wrote:
> > > This slowly gets hard to read. Could you maybe split it up? Either in 
> > > giving the multiple parts their own name. Or to keep the short circuiting 
> > > have something like:
> > > ```
> > > auto AllowsSpaceChange = ...;
> > > AllowsSpaceChange = AllowsSpaceChange || ...;
> > > AllowsSpaceChange = AllowsSpaceChange || ...;
> > > ```
> > Or use a lambda (with a series of ifs for instance) to initialize it.
> While here, please change `auto` to `bool`.
I've split the conditional over a couple of smaller boolean conditionals with 
the intent of enhancing readability.



Comment at: clang/unittests/Format/FormatTestComments.cpp:98
+   "// clang-format on\n",
+   getLLVMStyleWithColumns(20)));
+

HazardyKnusperkeks wrote:
> Why the limit?
This was a copy-paste artifact and wasn't intended to be included in the 
original diff; the column limit has no bearing on this specific test addition.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120188

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D120188: Fix extraneous whitespace addition in line comments on clang-format directives

[Luis Penagos via Phabricator via cfe-commits]

penagos marked an inline comment as done.
penagos added a comment.

In D120188#674 , 
@HazardyKnusperkeks wrote:

> Thanks for working on it.

Happy to help!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120188

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D120188: Fix extraneous whitespace addition in line comments on clang-format directives

[Luis Penagos via Phabricator via cfe-commits]

penagos added a comment.

I don’t have commit rights; if someone could commit on my behalf that’d be 
great.

- Luis Penagos
- l...@penagos.co

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120188

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D120374: [clang-format] Do not insert space after new/delete keywords in C function declarations

[Luis Penagos via Phabricator via cfe-commits]

penagos created this revision.
penagos added reviewers: curdeius, MyDeveloperDay, HazardyKnusperkeks.
penagos updated this revision to Diff 410700.
penagos added a comment.
penagos published this revision for review.
penagos added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The windows build failure appears to be unrelated to this change. I found the 
need to include `!Left.Previous` in the conditional so as to account for the 
existing unittest in the form of:

  new (expr)

To not wind up formatting this case as:

  new(expr)

Though it's still unclear to me whether or not it'd make more sense to continue 
to pursue a change to introduce `C` as a language. Thoughts?


Fixes https://github.com/llvm/llvm-project/issues/46915.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120374

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -9919,6 +9919,11 @@
   verifyFormat("void operator new(void *foo) ATTRIB;");
   verifyFormat("void operator delete[](void *foo) ATTRIB;");
   verifyFormat("void operator delete(void *ptr) noexcept;");
+
+  EXPECT_EQ("void new(link p);\n"
+"void delete(link p);\n",
+format("void new (link p);\n"
+   "void delete(link p);\n"));
 }
 
 TEST_F(FormatTest, UnderstandsUsesOfStarAndAmp) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3299,11 +3299,15 @@
   if (Left.isOneOf(tok::kw_try, Keywords.kw___except, tok::kw_catch))
 return Style.SpaceBeforeParensOptions.AfterControlStatements ||
spaceRequiredBeforeParens(Right);
-  if (Left.isOneOf(tok::kw_new, tok::kw_delete) ||
-  (Left.is(tok::r_square) && Left.MatchingParen &&
-   Left.MatchingParen->Previous &&
-   Left.MatchingParen->Previous->is(tok::kw_delete)))
-return Style.SpaceBeforeParens != FormatStyle::SBPO_Never ||
+  if (Left.isOneOf(tok::kw_new, tok::kw_delete))
+return ((!Line.MightBeFunctionDecl || !Left.Previous) &&
+Style.SpaceBeforeParens != FormatStyle::SBPO_Never) ||
+   spaceRequiredBeforeParens(Right);
+
+  if (Left.is(tok::r_square) && Left.MatchingParen &&
+  Left.MatchingParen->Previous &&
+  Left.MatchingParen->Previous->is(tok::kw_delete))
+return (Style.SpaceBeforeParens != FormatStyle::SBPO_Never) ||
spaceRequiredBeforeParens(Right);
 }
 if (Line.Type != LT_PreprocessorDirective &&


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -9919,6 +9919,11 @@
   verifyFormat("void operator new(void *foo) ATTRIB;");
   verifyFormat("void operator delete[](void *foo) ATTRIB;");
   verifyFormat("void operator delete(void *ptr) noexcept;");
+
+  EXPECT_EQ("void new(link p);\n"
+"void delete(link p);\n",
+format("void new (link p);\n"
+   "void delete(link p);\n"));
 }
 
 TEST_F(FormatTest, UnderstandsUsesOfStarAndAmp) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3299,11 +3299,15 @@
   if (Left.isOneOf(tok::kw_try, Keywords.kw___except, tok::kw_catch))
 return Style.SpaceBeforeParensOptions.AfterControlStatements ||
spaceRequiredBeforeParens(Right);
-  if (Left.isOneOf(tok::kw_new, tok::kw_delete) ||
-  (Left.is(tok::r_square) && Left.MatchingParen &&
-   Left.MatchingParen->Previous &&
-   Left.MatchingParen->Previous->is(tok::kw_delete)))
-return Style.SpaceBeforeParens != FormatStyle::SBPO_Never ||
+  if (Left.isOneOf(tok::kw_new, tok::kw_delete))
+return ((!Line.MightBeFunctionDecl || !Left.Previous) &&
+Style.SpaceBeforeParens != FormatStyle::SBPO_Never) ||
+   spaceRequiredBeforeParens(Right);
+
+  if (Left.is(tok::r_square) && Left.MatchingParen &&
+  Left.MatchingParen->Previous &&
+  Left.MatchingParen->Previous->is(tok::kw_delete))
+return (Style.SpaceBeforeParens != FormatStyle::SBPO_Never) ||
spaceRequiredBeforeParens(Right);
 }
 if (Line.Type != LT_PreprocessorDirective &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101322: [clang-format] Add comment justification for https://reviews.llvm.org/D100778

[Luis Penagos via Phabricator via cfe-commits]

penagos abandoned this revision.
penagos added a comment.

Closing old review request.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101322

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D120374: [clang-format] Do not insert space after new/delete keywords in C function declarations

[Luis Penagos via Phabricator via cfe-commits]

penagos updated this revision to Diff 410788.
penagos added a comment.

Add missing space in unittest.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120374

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -9919,6 +9919,11 @@
   verifyFormat("void operator new(void *foo) ATTRIB;");
   verifyFormat("void operator delete[](void *foo) ATTRIB;");
   verifyFormat("void operator delete(void *ptr) noexcept;");
+
+  EXPECT_EQ("void new(link p);\n"
+"void delete(link p);\n",
+format("void new (link p);\n"
+   "void delete (link p);\n"));
 }
 
 TEST_F(FormatTest, UnderstandsUsesOfStarAndAmp) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3299,11 +3299,15 @@
   if (Left.isOneOf(tok::kw_try, Keywords.kw___except, tok::kw_catch))
 return Style.SpaceBeforeParensOptions.AfterControlStatements ||
spaceRequiredBeforeParens(Right);
-  if (Left.isOneOf(tok::kw_new, tok::kw_delete) ||
-  (Left.is(tok::r_square) && Left.MatchingParen &&
-   Left.MatchingParen->Previous &&
-   Left.MatchingParen->Previous->is(tok::kw_delete)))
-return Style.SpaceBeforeParens != FormatStyle::SBPO_Never ||
+  if (Left.isOneOf(tok::kw_new, tok::kw_delete))
+return ((!Line.MightBeFunctionDecl || !Left.Previous) &&
+Style.SpaceBeforeParens != FormatStyle::SBPO_Never) ||
+   spaceRequiredBeforeParens(Right);
+
+  if (Left.is(tok::r_square) && Left.MatchingParen &&
+  Left.MatchingParen->Previous &&
+  Left.MatchingParen->Previous->is(tok::kw_delete))
+return (Style.SpaceBeforeParens != FormatStyle::SBPO_Never) ||
spaceRequiredBeforeParens(Right);
 }
 if (Line.Type != LT_PreprocessorDirective &&


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -9919,6 +9919,11 @@
   verifyFormat("void operator new(void *foo) ATTRIB;");
   verifyFormat("void operator delete[](void *foo) ATTRIB;");
   verifyFormat("void operator delete(void *ptr) noexcept;");
+
+  EXPECT_EQ("void new(link p);\n"
+"void delete(link p);\n",
+format("void new (link p);\n"
+   "void delete (link p);\n"));
 }
 
 TEST_F(FormatTest, UnderstandsUsesOfStarAndAmp) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3299,11 +3299,15 @@
   if (Left.isOneOf(tok::kw_try, Keywords.kw___except, tok::kw_catch))
 return Style.SpaceBeforeParensOptions.AfterControlStatements ||
spaceRequiredBeforeParens(Right);
-  if (Left.isOneOf(tok::kw_new, tok::kw_delete) ||
-  (Left.is(tok::r_square) && Left.MatchingParen &&
-   Left.MatchingParen->Previous &&
-   Left.MatchingParen->Previous->is(tok::kw_delete)))
-return Style.SpaceBeforeParens != FormatStyle::SBPO_Never ||
+  if (Left.isOneOf(tok::kw_new, tok::kw_delete))
+return ((!Line.MightBeFunctionDecl || !Left.Previous) &&
+Style.SpaceBeforeParens != FormatStyle::SBPO_Never) ||
+   spaceRequiredBeforeParens(Right);
+
+  if (Left.is(tok::r_square) && Left.MatchingParen &&
+  Left.MatchingParen->Previous &&
+  Left.MatchingParen->Previous->is(tok::kw_delete))
+return (Style.SpaceBeforeParens != FormatStyle::SBPO_Never) ||
spaceRequiredBeforeParens(Right);
 }
 if (Line.Type != LT_PreprocessorDirective &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D120374: [clang-format] Do not insert space after new/delete keywords in C function declarations

[Luis Penagos via Phabricator via cfe-commits]

penagos marked an inline comment as done.
penagos added a comment.

In D120374#3339554 , @curdeius wrote:

> In D120374#3339337 , @penagos wrote:
>
>> Though it's still unclear to me whether or not it'd make more sense to 
>> continue to pursue a change to introduce `C` as a language. Thoughts?
>
> I prefer your current approach (and there should be not that many cases that 
> we need a special treatment for C).

Makes sense. I'll also need someone to commit on my behalf:

- Luis Penagos
- l...@penagos.co

Thanks!




Comment at: clang/unittests/Format/FormatTest.cpp:9926
+format("void new (link p);\n"
+   "void delete(link p);\n"));
 }

curdeius wrote:
> Why there's no space after `delete` in the input?
Whoops, good catch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120374

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D120445: [clang-format] Treat && followed by noexcept operator as a binary operator inside template arguments

[Luis Penagos via Phabricator via cfe-commits]

penagos created this revision.
penagos added reviewers: curdeius, MyDeveloperDay.
penagos published this revision for review.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fixes https://github.com/llvm/llvm-project/issues/44544.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120445

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -9512,6 +9512,7 @@
   verifyFormat("f(aa\n"
"  .template operator()());",
getLLVMStyleWithColumns(35));
+  verifyFormat("bool_constant");
 
   // Not template parameters.
   verifyFormat("return a < b && c > d;");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2066,6 +2066,10 @@
   return TT_UnaryOperator;
 
 const FormatToken *NextToken = Tok.getNextNonComment();
+
+if (InTemplateArgument && NextToken && NextToken->is(tok::kw_noexcept))
+  return TT_BinaryOperator;
+
 if (!NextToken ||
 NextToken->isOneOf(tok::arrow, tok::equal, tok::kw_noexcept) ||
 NextToken->canBePointerOrReferenceQualifier() ||


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -9512,6 +9512,7 @@
   verifyFormat("f(aa\n"
"  .template operator()());",
getLLVMStyleWithColumns(35));
+  verifyFormat("bool_constant");
 
   // Not template parameters.
   verifyFormat("return a < b && c > d;");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2066,6 +2066,10 @@
   return TT_UnaryOperator;
 
 const FormatToken *NextToken = Tok.getNextNonComment();
+
+if (InTemplateArgument && NextToken && NextToken->is(tok::kw_noexcept))
+  return TT_BinaryOperator;
+
 if (!NextToken ||
 NextToken->isOneOf(tok::arrow, tok::equal, tok::kw_noexcept) ||
 NextToken->canBePointerOrReferenceQualifier() ||
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D120445: [clang-format] Treat && followed by noexcept operator as a binary operator inside template arguments

[Luis Penagos via Phabricator via cfe-commits]

penagos updated this revision to Diff 411272.
penagos added a comment.

Rebase to trigger CI.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120445

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -9512,6 +9512,7 @@
   verifyFormat("f(aa\n"
"  .template operator()());",
getLLVMStyleWithColumns(35));
+  verifyFormat("bool_constant");
 
   // Not template parameters.
   verifyFormat("return a < b && c > d;");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2066,6 +2066,10 @@
   return TT_UnaryOperator;
 
 const FormatToken *NextToken = Tok.getNextNonComment();
+
+if (InTemplateArgument && NextToken && NextToken->is(tok::kw_noexcept))
+  return TT_BinaryOperator;
+
 if (!NextToken ||
 NextToken->isOneOf(tok::arrow, tok::equal, tok::kw_noexcept) ||
 NextToken->canBePointerOrReferenceQualifier() ||


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -9512,6 +9512,7 @@
   verifyFormat("f(aa\n"
"  .template operator()());",
getLLVMStyleWithColumns(35));
+  verifyFormat("bool_constant");
 
   // Not template parameters.
   verifyFormat("return a < b && c > d;");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2066,6 +2066,10 @@
   return TT_UnaryOperator;
 
 const FormatToken *NextToken = Tok.getNextNonComment();
+
+if (InTemplateArgument && NextToken && NextToken->is(tok::kw_noexcept))
+  return TT_BinaryOperator;
+
 if (!NextToken ||
 NextToken->isOneOf(tok::arrow, tok::equal, tok::kw_noexcept) ||
 NextToken->canBePointerOrReferenceQualifier() ||
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D120445: [clang-format] Treat && followed by noexcept operator as a binary operator inside template arguments

[Luis Penagos via Phabricator via cfe-commits]

penagos added a comment.

I went ahead and did a git rebase but CI still appears to be failing (though 
tests pass in my local clone). Also, I’ll need someone to commit this on my 
behalf:

- Luis Penagos
- l...@penagos.co

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120445

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100778: [clang-format] Prevent extraneous space insertion in bitshift operators

[Luis Penagos via Phabricator via cfe-commits]

penagos added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:125
+ 
CurrentToken->Next->getStartOfNonWhitespace().getLocWithOffset(
+ -1)))
   return false;

krasimir wrote:
> penagos wrote:
> > MyDeveloperDay wrote:
> > > I don't really understand what we are saying here? 
> > Effectively we are checking that, barring intervening whitespace, we are 
> > analyzing 2 consecutive '>' tokens. If so, we treat such sequence as a 
> > binary op in lieu of a closing template angle bracket. If there's another 
> > more straightforward way of accomplishing this check, I'm open to that, but 
> > this seemed to be the most straightforward way at the time.
> I'm worried that this may regress template code. How does this account for 
> cases where two consecutive `>`-s are really two closing template brackets, 
> e.g.,
> `std::vector> v;`?
> 
> In particular, one added test case is ambiguous: `>>` could really be two 
> closing template brackets:
> https://godbolt.org/z/v19hj9vKn
> 
> I have to say that my general feeling about trying to disambiguate between 
> bitshifts and template closers is: don't try too hard inside clang-format as 
> the heuristics are generally quite brittle and make the code harder to 
> maintain; in cases where clang-format wrongly detects bitshift as templates, 
> users should add parens around the bitshift, which IMO improves readability.
As this patch currently stands, it does not disambiguate between bitshift '>>' 
operators and 2 closing template brackets, so in your snippet, we would no 
longer insert a space between the '>' characters (despite arguably being the 
better formatting decision in this case).

I agree with your feeling that user guided disambiguation between bitshift 
operators and template closing brackets via parens is the ideal solution and 
also improves readability, but IMO the approach taken by clang-format to format 
the '>' token should be conservative in that any change made should be 
non-semantic altering, which is not presently the case. While the case you 
mentioned would regress, we would no longer potentially alter program 
semantics. Thinking about this more, would it make sense to modify the actual 
white-space change generation later on in the analysis to not break up >> 
sequences of characters in lieu of annotating the tokens differently as the 
proposed patch is currently doing?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100778

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101322: [clang-format] Add comment justification for https://reviews.llvm.org/D100778

[Luis Penagos via Phabricator via cfe-commits]

penagos created this revision.
penagos requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D101322

Files:
  clang/lib/Format/TokenAnnotator.cpp


Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -116,7 +116,11 @@
 while (CurrentToken) {
   if (CurrentToken->is(tok::greater)) {
 // Try to do a better job at looking for ">>" within the condition of
-// a statement.
+// a statement. Conservatively insert spaces between consecutive ">"
+// tokens to prevent splitting right bitshift operators and potentially
+// altering program semantics. This check is overly conservative and
+// will prevent spaces from being inserted in select nested template
+// parameter cases, but should not alter program semantics.
 if (CurrentToken->Next && CurrentToken->Next->is(tok::greater) &&
 Left->ParentBracket != tok::less &&
 (isKeywordWithCondition(*Line.First) ||


Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -116,7 +116,11 @@
 while (CurrentToken) {
   if (CurrentToken->is(tok::greater)) {
 // Try to do a better job at looking for ">>" within the condition of
-// a statement.
+// a statement. Conservatively insert spaces between consecutive ">"
+// tokens to prevent splitting right bitshift operators and potentially
+// altering program semantics. This check is overly conservative and
+// will prevent spaces from being inserted in select nested template
+// parameter cases, but should not alter program semantics.
 if (CurrentToken->Next && CurrentToken->Next->is(tok::greater) &&
 Left->ParentBracket != tok::less &&
 (isKeywordWithCondition(*Line.First) ||
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101322: [clang-format] Add comment justification for https://reviews.llvm.org/D100778

[Luis Penagos via Phabricator via cfe-commits]

penagos added a comment.

Please ignore this revision.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101322

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100778: [clang-format] Prevent extraneous space insertion in bitshift operators

[Luis Penagos via Phabricator via cfe-commits]

penagos updated this revision to Diff 340656.
penagos added a comment.

Add justification comment for changes in parseAngle()


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100778

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -7649,6 +7649,8 @@
   verifyFormat("a = 1;", Style);
   verifyFormat("a >>= 1;", Style);
 
+  verifyFormat("test < a | b >> c;");
+  verifyFormat("test> c;");
   verifyFormat("test >> a >> b;");
   verifyFormat("test << a >> b;");
 
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -116,10 +116,17 @@
 while (CurrentToken) {
   if (CurrentToken->is(tok::greater)) {
 // Try to do a better job at looking for ">>" within the condition of
-// a statement.
+// a statement. Conservatively insert spaces between consecutive ">"
+// tokens to prevent splitting right bitshift operators and potentially
+// altering program semantics. This check is overly conservative and
+// will prevent spaces from being inserted in select nested template
+// parameter cases, but should not alter program semantics.
 if (CurrentToken->Next && CurrentToken->Next->is(tok::greater) &&
 Left->ParentBracket != tok::less &&
-isKeywordWithCondition(*Line.First))
+(isKeywordWithCondition(*Line.First) ||
+ CurrentToken->getStartOfNonWhitespace() ==
+ 
CurrentToken->Next->getStartOfNonWhitespace().getLocWithOffset(
+ -1)))
   return false;
 Left->MatchingParen = CurrentToken;
 CurrentToken->MatchingParen = Left;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -7649,6 +7649,8 @@
   verifyFormat("a = 1;", Style);
   verifyFormat("a >>= 1;", Style);
 
+  verifyFormat("test < a | b >> c;");
+  verifyFormat("test> c;");
   verifyFormat("test >> a >> b;");
   verifyFormat("test << a >> b;");
 
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -116,10 +116,17 @@
 while (CurrentToken) {
   if (CurrentToken->is(tok::greater)) {
 // Try to do a better job at looking for ">>" within the condition of
-// a statement.
+// a statement. Conservatively insert spaces between consecutive ">"
+// tokens to prevent splitting right bitshift operators and potentially
+// altering program semantics. This check is overly conservative and
+// will prevent spaces from being inserted in select nested template
+// parameter cases, but should not alter program semantics.
 if (CurrentToken->Next && CurrentToken->Next->is(tok::greater) &&
 Left->ParentBracket != tok::less &&
-isKeywordWithCondition(*Line.First))
+(isKeywordWithCondition(*Line.First) ||
+ CurrentToken->getStartOfNonWhitespace() ==
+ CurrentToken->Next->getStartOfNonWhitespace().getLocWithOffset(
+ -1)))
   return false;
 Left->MatchingParen = CurrentToken;
 CurrentToken->MatchingParen = Left;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100778: [clang-format] Prevent extraneous space insertion in bitshift operators

[Luis Penagos via Phabricator via cfe-commits]

penagos added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:125
+ 
CurrentToken->Next->getStartOfNonWhitespace().getLocWithOffset(
+ -1)))
   return false;

krasimir wrote:
> penagos wrote:
> > krasimir wrote:
> > > penagos wrote:
> > > > MyDeveloperDay wrote:
> > > > > I don't really understand what we are saying here? 
> > > > Effectively we are checking that, barring intervening whitespace, we 
> > > > are analyzing 2 consecutive '>' tokens. If so, we treat such sequence 
> > > > as a binary op in lieu of a closing template angle bracket. If there's 
> > > > another more straightforward way of accomplishing this check, I'm open 
> > > > to that, but this seemed to be the most straightforward way at the time.
> > > I'm worried that this may regress template code. How does this account 
> > > for cases where two consecutive `>`-s are really two closing template 
> > > brackets, e.g.,
> > > `std::vector> v;`?
> > > 
> > > In particular, one added test case is ambiguous: `>>` could really be two 
> > > closing template brackets:
> > > https://godbolt.org/z/v19hj9vKn
> > > 
> > > I have to say that my general feeling about trying to disambiguate 
> > > between bitshifts and template closers is: don't try too hard inside 
> > > clang-format as the heuristics are generally quite brittle and make the 
> > > code harder to maintain; in cases where clang-format wrongly detects 
> > > bitshift as templates, users should add parens around the bitshift, which 
> > > IMO improves readability.
> > As this patch currently stands, it does not disambiguate between bitshift 
> > '>>' operators and 2 closing template brackets, so in your snippet, we 
> > would no longer insert a space between the '>' characters (despite arguably 
> > being the better formatting decision in this case).
> > 
> > I agree with your feeling that user guided disambiguation between bitshift 
> > operators and template closing brackets via parens is the ideal solution 
> > and also improves readability, but IMO the approach taken by clang-format 
> > to format the '>' token should be conservative in that any change made 
> > should be non-semantic altering, which is not presently the case. While the 
> > case you mentioned would regress, we would no longer potentially alter 
> > program semantics. Thinking about this more, would it make sense to modify 
> > the actual white-space change generation later on in the analysis to not 
> > break up >> sequences of characters in lieu of annotating the tokens 
> > differently as the proposed patch is currently doing?
> I tried and can't make this misinterpret two consecutive template `>` as a 
> bit shift, IMO because this check is guarded by the `Left->ParentBracket != 
> tok::less` condition. Both `std::vector> v;` and 
> `test> c;` below are handled correctly.
> I'm less worried about regressions in common template cases now.
> Thank you for pointing out altering program semantics, I agree.
> Please add a comment about this tradeoff and and a bit of the reasoning 
> behind it in code for future reference.
I had come to the same conclusion when modifying the conditional here; namely 
the ParentBracket predicate is what catches the case you were alluding to 
earlier.
I've added a brief comment to `parseAngle()` to document the need for the 
change, explaining the conservative nature of the change w.r.t. nested template 
cases; thank you for the suggestion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100778

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100778: [clang-format] Prevent extraneous space insertion in bitshift operators

[Luis Penagos via Phabricator via cfe-commits]

penagos added a comment.

Friendly reminder that I need someone to land this for me as I do not have 
commit access.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100778

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100778: [clang-format] Prevent extraneous space insertion in bitshift operators

[Luis Penagos via Phabricator via cfe-commits]

penagos created this revision.
penagos added reviewers: Saldivarcher, MyDeveloperDay, JakeMerdichAMD, krasimir.
penagos requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This serves to augment the improvements made in 
https://reviews.llvm.org/D86581. It prevents clang-format from interpreting 
bitshift operators as template arguments in certain circumstances.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D100778

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -7649,6 +7649,7 @@
   verifyFormat("a = 1;", Style);
   verifyFormat("a >>= 1;", Style);
 
+  verifyFormat("test < a - 1 >> 1;");
   verifyFormat("test >> a >> b;");
   verifyFormat("test << a >> b;");
 
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -119,7 +119,10 @@
 // a statement.
 if (CurrentToken->Next && CurrentToken->Next->is(tok::greater) &&
 Left->ParentBracket != tok::less &&
-isKeywordWithCondition(*Line.First))
+(isKeywordWithCondition(*Line.First) ||
+ CurrentToken->getStartOfNonWhitespace() ==
+ 
CurrentToken->Next->getStartOfNonWhitespace().getLocWithOffset(
+ -1)))
   return false;
 Left->MatchingParen = CurrentToken;
 CurrentToken->MatchingParen = Left;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -7649,6 +7649,7 @@
   verifyFormat("a = 1;", Style);
   verifyFormat("a >>= 1;", Style);
 
+  verifyFormat("test < a - 1 >> 1;");
   verifyFormat("test >> a >> b;");
   verifyFormat("test << a >> b;");
 
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -119,7 +119,10 @@
 // a statement.
 if (CurrentToken->Next && CurrentToken->Next->is(tok::greater) &&
 Left->ParentBracket != tok::less &&
-isKeywordWithCondition(*Line.First))
+(isKeywordWithCondition(*Line.First) ||
+ CurrentToken->getStartOfNonWhitespace() ==
+ CurrentToken->Next->getStartOfNonWhitespace().getLocWithOffset(
+ -1)))
   return false;
 Left->MatchingParen = CurrentToken;
 CurrentToken->MatchingParen = Left;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100778: [clang-format] Prevent extraneous space insertion in bitshift operators

[Luis Penagos via Phabricator via cfe-commits]

penagos updated this revision to Diff 338641.

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

https://reviews.llvm.org/D100778

Files:
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -7649,7 +7649,8 @@
   verifyFormat("a = 1;", Style);
   verifyFormat("a >>= 1;", Style);
 
-  verifyFormat("test < a - 1 >> 1;");
+  verifyFormat("test < a | b >> c;");
+  verifyFormat("test> c;");
   verifyFormat("test >> a >> b;");
   verifyFormat("test << a >> b;");
 


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -7649,7 +7649,8 @@
   verifyFormat("a = 1;", Style);
   verifyFormat("a >>= 1;", Style);
 
-  verifyFormat("test < a - 1 >> 1;");
+  verifyFormat("test < a | b >> c;");
+  verifyFormat("test> c;");
   verifyFormat("test >> a >> b;");
   verifyFormat("test << a >> b;");
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100778: [clang-format] Prevent extraneous space insertion in bitshift operators

[Luis Penagos via Phabricator via cfe-commits]

penagos marked an inline comment as done.
penagos added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:7652
 
+  verifyFormat("test < a - 1 >> 1;");
   verifyFormat("test >> a >> b;");

Quuxplusone wrote:
> Quuxplusone wrote:
> > IMO you should use `"test < a | b >> c;"` as your test case here, to 
> > reassure the reader that it doesn't depend on the fact that `... 1;` is 
> > visibly not a variable declaration.
> > Personally I'd also like to see `"test> c;"` tested on the very 
> > next line, to show off the intended difference between the two. (Assuming 
> > that I understand the intent of this patch correctly.)
> > (I also switched to a bitwise operator just for the heck of it; that makes 
> > this expression just a //very tiny bit// less implausible — but still 
> > highly implausible, to the point where I question why we're special-casing 
> > it.)
> Btw, a much-bigger-scope way to fix this would be to teach clang-format about 
> "input encoding" versus "output encoding." The only time clang-format should 
> //ever// be inserting space in the middle of `>>` is if it's translating 
> C++11-encoded input into C++03-encoded output. If the input is known to 
> already be C++03-encoded, then breaking up an `>>` token into a pair of `> >` 
> tokens is //guaranteed// to introduce a bug.
> Right now, my impression is that clang-format has a concept of "output 
> encoding" (i.e. "language mode") but has no way of knowing the "input 
> encoding."
Thanks for the feedback. Your 2 test suggestions make sense to me; I've updated 
the patch diff. I hadn't considered teaching clang-format input encoding, but 
that does sound like the preferable long term solution. This patch is intended 
to be a lightweight fix to fix a very narrow use case.


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

https://reviews.llvm.org/D100778

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100778: [clang-format] Prevent extraneous space insertion in bitshift operators

[Luis Penagos via Phabricator via cfe-commits]

penagos updated this revision to Diff 338648.
penagos added a comment.

Update Format test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100778

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -7649,6 +7649,8 @@
   verifyFormat("a = 1;", Style);
   verifyFormat("a >>= 1;", Style);
 
+  verifyFormat("test < a | b >> c;");
+  verifyFormat("test> c;");
   verifyFormat("test >> a >> b;");
   verifyFormat("test << a >> b;");
 
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -119,7 +119,10 @@
 // a statement.
 if (CurrentToken->Next && CurrentToken->Next->is(tok::greater) &&
 Left->ParentBracket != tok::less &&
-isKeywordWithCondition(*Line.First))
+(isKeywordWithCondition(*Line.First) ||
+ CurrentToken->getStartOfNonWhitespace() ==
+ 
CurrentToken->Next->getStartOfNonWhitespace().getLocWithOffset(
+ -1)))
   return false;
 Left->MatchingParen = CurrentToken;
 CurrentToken->MatchingParen = Left;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -7649,6 +7649,8 @@
   verifyFormat("a = 1;", Style);
   verifyFormat("a >>= 1;", Style);
 
+  verifyFormat("test < a | b >> c;");
+  verifyFormat("test> c;");
   verifyFormat("test >> a >> b;");
   verifyFormat("test << a >> b;");
 
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -119,7 +119,10 @@
 // a statement.
 if (CurrentToken->Next && CurrentToken->Next->is(tok::greater) &&
 Left->ParentBracket != tok::less &&
-isKeywordWithCondition(*Line.First))
+(isKeywordWithCondition(*Line.First) ||
+ CurrentToken->getStartOfNonWhitespace() ==
+ CurrentToken->Next->getStartOfNonWhitespace().getLocWithOffset(
+ -1)))
   return false;
 Left->MatchingParen = CurrentToken;
 CurrentToken->MatchingParen = Left;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100778: [clang-format] Prevent extraneous space insertion in bitshift operators

[Luis Penagos via Phabricator via cfe-commits]

penagos added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:125
+ 
CurrentToken->Next->getStartOfNonWhitespace().getLocWithOffset(
+ -1)))
   return false;

MyDeveloperDay wrote:
> I don't really understand what we are saying here? 
Effectively we are checking that, barring intervening whitespace, we are 
analyzing 2 consecutive '>' tokens. If so, we treat such sequence as a binary 
op in lieu of a closing template angle bracket. If there's another more 
straightforward way of accomplishing this check, I'm open to that, but this 
seemed to be the most straightforward way at the time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100778

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100778: [clang-format] Prevent extraneous space insertion in bitshift operators

[Luis Penagos via Phabricator via cfe-commits]

penagos added a comment.

Additionally; barring any other feedback, I'll need someone to land this change 
as I do not have commit access.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100778

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits