from:"Paweł Żukowski via cfe\-commits"

[PATCH] D25439: Fixed column shift when formatting line containing bit shift operators

2016-11-03 Thread Paweł Żukowski via cfe-commits

idlecode added a comment.

No, not yet - I was about to ask someone to commit this for me :)


https://reviews.llvm.org/D25439



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

[PATCH] D26125: [clang-tidy] Fixed readability-else-after-return for cascade statements

2016-11-01 Thread Paweł Żukowski via cfe-commits

idlecode retitled this revision from "[clang-tidy] Fixed else-after-return 
warning in cascade if statement" to "[clang-tidy] Fixed 
readability-else-after-return for cascade statements".
idlecode updated the summary for this revision.
idlecode updated this revision to Diff 76604.
idlecode added a comment.

I have reverted matcher to the state before https://reviews.llvm.org/D23265 
(tests are passing with compoundStmt instead of stmt and this way is better 
than mine).


https://reviews.llvm.org/D26125

Files:
  clang-tidy/readability/ElseAfterReturnCheck.cpp
  test/clang-tidy/readability-else-after-return.cpp


Index: test/clang-tidy/readability-else-after-return.cpp
===
--- test/clang-tidy/readability-else-after-return.cpp
+++ test/clang-tidy/readability-else-after-return.cpp
@@ -29,32 +29,60 @@
   else if (a > 10)
 return;
   else // comment-2
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use 'else' after 'return'
-  // CHECK-FIXES: {{^}}  // comment-2
+  // CHECK-FIXES-NOT: {{^}}  // comment-2
 f(0);
+
+  if (a > 0)
+if (a < 10)
+  return;
+else // comment-3
+// CHECK-FIXES-NOT: {{^}}// comment-3
+  f(0);
+  else
+if (a > 10)
+  return;
+else // comment-4
+// CHECK-FIXES-NOT: {{^}}// comment-4
+  f(0);
+
+  if (a > 0) {
+if (a < 10)
+  return;
+else // comment-5
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use 'else' after 
'return'
+// CHECK-FIXES: {{^}}// comment-5
+  f(0);
+  } else {
+if (a > 10)
+  return;
+else // comment-6
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use 'else' after 
'return'
+// CHECK-FIXES: {{^}}// comment-6
+  f(0);
+  }
 }
 
 void foo() {
   for (unsigned x = 0; x < 42; ++x) {
 if (x) {
   continue;
-} else { // comment-3
+} else { // comment-7
 // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 
'continue'
-// CHECK-FIXES: {{^}}} // comment-3
+// CHECK-FIXES: {{^}}} // comment-7
   x++;
 }
 if (x) {
   break;
-} else { // comment-4
+} else { // comment-8
 // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 'break'
-// CHECK-FIXES: {{^}}} // comment-4
+// CHECK-FIXES: {{^}}} // comment-8
   x++;
 }
 if (x) {
   throw 42;
-} else { // comment-5
+} else { // comment-9
 // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 'throw'
-// CHECK-FIXES: {{^}}} // comment-5
+// CHECK-FIXES: {{^}}} // comment-9
   x++;
 }
   }
Index: clang-tidy/readability/ElseAfterReturnCheck.cpp
===
--- clang-tidy/readability/ElseAfterReturnCheck.cpp
+++ clang-tidy/readability/ElseAfterReturnCheck.cpp
@@ -23,7 +23,7 @@
   stmt(anyOf(returnStmt().bind("return"), continueStmt().bind("continue"),
  breakStmt().bind("break"), cxxThrowExpr().bind("throw")));
   Finder->addMatcher(
-  stmt(forEach(
+  compoundStmt(forEach(
   ifStmt(hasThen(stmt(
  anyOf(ControlFlowInterruptorMatcher,
compoundStmt(has(ControlFlowInterruptorMatcher),


Index: test/clang-tidy/readability-else-after-return.cpp
===
--- test/clang-tidy/readability-else-after-return.cpp
+++ test/clang-tidy/readability-else-after-return.cpp
@@ -29,32 +29,60 @@
   else if (a > 10)
 return;
   else // comment-2
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use 'else' after 'return'
-  // CHECK-FIXES: {{^}}  // comment-2
+  // CHECK-FIXES-NOT: {{^}}  // comment-2
 f(0);
+
+  if (a > 0)
+if (a < 10)
+  return;
+else // comment-3
+// CHECK-FIXES-NOT: {{^}}// comment-3
+  f(0);
+  else
+if (a > 10)
+  return;
+else // comment-4
+// CHECK-FIXES-NOT: {{^}}// comment-4
+  f(0);
+
+  if (a > 0) {
+if (a < 10)
+  return;
+else // comment-5
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use 'else' after 'return'
+// CHECK-FIXES: {{^}}// comment-5
+  f(0);
+  } else {
+if (a > 10)
+  return;
+else // comment-6
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use 'else' after 'return'
+// CHECK-FIXES: {{^}}// comment-6
+  f(0);
+  }
 }
 
 void foo() {
   for (unsigned x = 0; x < 42; ++x) {
 if (x) {
   continue;
-} else { // comment-3
+} else { // comment-7
 // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 'continue'
-// CHECK-FIXES: {{^}}} // comment-3
+// CHECK-FIXES: {{^}}} // comment-7
   x++;
 }
 if (x) {
   break;
-} else { // comment-4
+} else { // comment-8
 // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 'break'
-// CHECK-FIXES: {{^}}

[PATCH] D26125: [clang-tidy] Fixed else-after-return warning in cascade if statement

2016-10-30 Thread Paweł Żukowski via cfe-commits

idlecode added a comment.

@mgehre: Yes it does - in your case AST looks like `ifStmt(b) - CompoundStmt - 
ifStmt(c)`, so ifStmt's don't have direct parent-child relations.
But there is another problem:

  while(a)
if (b)
  return 1;
else
  doSomething(2);

In such case, generated fix (with and without this fix) will change semantics 
of program.
As @djasper suggested, I need to take a look at this check history and update 
patch accordingly.


https://reviews.llvm.org/D26125



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

[PATCH] D26125: [clang-tidy] Fixed else-after-return warning in cascade if statement

2016-10-30 Thread Paweł Żukowski via cfe-commits

idlecode planned changes to this revision.
idlecode added inline comments.



Comment at: clang-tidy/readability/ElseAfterReturnCheck.cpp:28
   stmt(forEach(
-  ifStmt(hasThen(stmt(
+  ifStmt(unless(hasParent(ifStmt())),
+ hasThen(stmt(

djasper wrote:
> idlecode wrote:
> > djasper wrote:
> > > idlecode wrote:
> > > > djasper wrote:
> > > > > I think this now effectively does:
> > > > > 
> > > > >   stmt(forEach(ifStmt(unless(hasParent(ifSttmt())), ...)
> > > > > 
> > > > > I think that's equivalent to:
> > > > > 
> > > > >   stmt(unless(ifStmt()), forEach(ifStmt(...)))
> > > > Apparently you are right - does that mean that this matcher actually 
> > > > matches node above ifStmt (and just binds ifStmt)?
> > > > If so maybe such expression would suffice? (it passes tests)
> > > > ```
> > > > Finder->addMatcher( // note lack of stmt(forEach(
> > > >   ifStmt(unless(hasParent(ifStmt())),
> > > >  hasThen(stmt(
> > > >  anyOf(ControlFlowInterruptorMatcher,
> > > >
> > > > compoundStmt(has(ControlFlowInterruptorMatcher),
> > > >  hasElse(stmt().bind("else")))
> > > >   .bind("if"),
> > > >   this);
> > > > 
> > > > ```
> > > > 
> > > > But I'm not sure if this would be any better than your version.
> > > Yeah. That makes sense to me. The difference is that we would start 
> > > binding ifStmts() that are direct children of declarations. But I don't 
> > > know whether we have excluded those intentionally. Do any tests break if 
> > > you make this change?
> > I have just ran `make check-clang-tools` on updated source tree - no new 
> > failures. Upload the change?
> > What do you mean by 'if statement as children of declaration'? (I'm new to 
> > compilers and just curious :) )
> I did some more digging. It appears this bug was introduced in 
> https://reviews.llvm.org/rL278257. The old code specifically did the forEach 
> stuff to ensure that the ifStmt was a direct child of a compound stmt. I 
> think we should investigate why this happened and probably undo it. There are 
> similar to this one here if the ifStmt is a child of a for or while loop 
> without a compound stmt.
Indeed, my fix fails for loops.
Also, do you see any reason why 'goto' was not included in interrupt statements?


https://reviews.llvm.org/D26125



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

[PATCH] D26125: [clang-tidy] Fixed else-after-return warning in cascade if statement

2016-10-30 Thread Paweł Żukowski via cfe-commits

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



Comment at: clang-tidy/readability/ElseAfterReturnCheck.cpp:28
   stmt(forEach(
-  ifStmt(hasThen(stmt(
+  ifStmt(unless(hasParent(ifStmt())),
+ hasThen(stmt(

djasper wrote:
> idlecode wrote:
> > djasper wrote:
> > > I think this now effectively does:
> > > 
> > >   stmt(forEach(ifStmt(unless(hasParent(ifSttmt())), ...)
> > > 
> > > I think that's equivalent to:
> > > 
> > >   stmt(unless(ifStmt()), forEach(ifStmt(...)))
> > Apparently you are right - does that mean that this matcher actually 
> > matches node above ifStmt (and just binds ifStmt)?
> > If so maybe such expression would suffice? (it passes tests)
> > ```
> > Finder->addMatcher( // note lack of stmt(forEach(
> >   ifStmt(unless(hasParent(ifStmt())),
> >  hasThen(stmt(
> >  anyOf(ControlFlowInterruptorMatcher,
> >
> > compoundStmt(has(ControlFlowInterruptorMatcher),
> >  hasElse(stmt().bind("else")))
> >   .bind("if"),
> >   this);
> > 
> > ```
> > 
> > But I'm not sure if this would be any better than your version.
> Yeah. That makes sense to me. The difference is that we would start binding 
> ifStmts() that are direct children of declarations. But I don't know whether 
> we have excluded those intentionally. Do any tests break if you make this 
> change?
I have just ran `make check-clang-tools` on updated source tree - no new 
failures. Upload the change?
What do you mean by 'if statement as children of declaration'? (I'm new to 
compilers and just curious :) )


https://reviews.llvm.org/D26125



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

[PATCH] D26125: [clang-tidy] Fixed else-after-return warning in cascade if statement

2016-10-30 Thread Paweł Żukowski via cfe-commits

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



Comment at: clang-tidy/readability/ElseAfterReturnCheck.cpp:28
   stmt(forEach(
-  ifStmt(hasThen(stmt(
+  ifStmt(unless(hasParent(ifStmt())),
+ hasThen(stmt(

djasper wrote:
> I think this now effectively does:
> 
>   stmt(forEach(ifStmt(unless(hasParent(ifSttmt())), ...)
> 
> I think that's equivalent to:
> 
>   stmt(unless(ifStmt()), forEach(ifStmt(...)))
Apparently you are right - does that mean that this matcher actually matches 
node above ifStmt (and just binds ifStmt)?
If so maybe such expression would suffice? (it passes tests)
```
Finder->addMatcher( // note lack of stmt(forEach(
  ifStmt(unless(hasParent(ifStmt())),
 hasThen(stmt(
 anyOf(ControlFlowInterruptorMatcher,
   compoundStmt(has(ControlFlowInterruptorMatcher),
 hasElse(stmt().bind("else")))
  .bind("if"),
  this);

```

But I'm not sure if this would be any better than your version.


https://reviews.llvm.org/D26125



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

[PATCH] D26125: [clang-tidy] Fixed else-after-return warning in cascade if statement

2016-10-30 Thread Paweł Żukowski via cfe-commits

idlecode created this revision.
idlecode added reviewers: djasper, malcolm.parsons, omtcyfz.
idlecode added a subscriber: cfe-commits.

Fix applied to else in a cascade if statements changed
program semantics in cases when not all previous branches
resulted in flow change.

Fixes PR30652


https://reviews.llvm.org/D26125

Files:
  clang-tidy/readability/ElseAfterReturnCheck.cpp
  test/clang-tidy/readability-else-after-return.cpp


Index: test/clang-tidy/readability-else-after-return.cpp
===
--- test/clang-tidy/readability-else-after-return.cpp
+++ test/clang-tidy/readability-else-after-return.cpp
@@ -29,32 +29,60 @@
   else if (a > 10)
 return;
   else // comment-2
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use 'else' after 'return'
-  // CHECK-FIXES: {{^}}  // comment-2
+  // CHECK-FIXES-NOT: {{^}}  // comment-2
 f(0);
+
+  if (a > 0)
+if (a < 10)
+  return;
+else // comment-3
+// CHECK-FIXES-NOT: {{^}}// comment-3
+  f(0);
+  else
+if (a > 10)
+  return;
+else // comment-4
+// CHECK-FIXES-NOT: {{^}}// comment-4
+  f(0);
+
+  if (a > 0) {
+if (a < 10)
+  return;
+else // comment-5
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use 'else' after 
'return'
+// CHECK-FIXES: {{^}}// comment-5
+  f(0);
+  } else {
+if (a > 10)
+  return;
+else // comment-6
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use 'else' after 
'return'
+// CHECK-FIXES: {{^}}// comment-6
+  f(0);
+  }
 }
 
 void foo() {
   for (unsigned x = 0; x < 42; ++x) {
 if (x) {
   continue;
-} else { // comment-3
+} else { // comment-7
 // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 
'continue'
-// CHECK-FIXES: {{^}}} // comment-3
+// CHECK-FIXES: {{^}}} // comment-7
   x++;
 }
 if (x) {
   break;
-} else { // comment-4
+} else { // comment-8
 // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 'break'
-// CHECK-FIXES: {{^}}} // comment-4
+// CHECK-FIXES: {{^}}} // comment-8
   x++;
 }
 if (x) {
   throw 42;
-} else { // comment-5
+} else { // comment-9
 // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 'throw'
-// CHECK-FIXES: {{^}}} // comment-5
+// CHECK-FIXES: {{^}}} // comment-9
   x++;
 }
   }
Index: clang-tidy/readability/ElseAfterReturnCheck.cpp
===
--- clang-tidy/readability/ElseAfterReturnCheck.cpp
+++ clang-tidy/readability/ElseAfterReturnCheck.cpp
@@ -22,9 +22,11 @@
   const auto ControlFlowInterruptorMatcher =
   stmt(anyOf(returnStmt().bind("return"), continueStmt().bind("continue"),
  breakStmt().bind("break"), cxxThrowExpr().bind("throw")));
+
   Finder->addMatcher(
   stmt(forEach(
-  ifStmt(hasThen(stmt(
+  ifStmt(unless(hasParent(ifStmt())),
+ hasThen(stmt(
  anyOf(ControlFlowInterruptorMatcher,
compoundStmt(has(ControlFlowInterruptorMatcher),
  hasElse(stmt().bind("else")))


Index: test/clang-tidy/readability-else-after-return.cpp
===
--- test/clang-tidy/readability-else-after-return.cpp
+++ test/clang-tidy/readability-else-after-return.cpp
@@ -29,32 +29,60 @@
   else if (a > 10)
 return;
   else // comment-2
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use 'else' after 'return'
-  // CHECK-FIXES: {{^}}  // comment-2
+  // CHECK-FIXES-NOT: {{^}}  // comment-2
 f(0);
+
+  if (a > 0)
+if (a < 10)
+  return;
+else // comment-3
+// CHECK-FIXES-NOT: {{^}}// comment-3
+  f(0);
+  else
+if (a > 10)
+  return;
+else // comment-4
+// CHECK-FIXES-NOT: {{^}}// comment-4
+  f(0);
+
+  if (a > 0) {
+if (a < 10)
+  return;
+else // comment-5
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use 'else' after 'return'
+// CHECK-FIXES: {{^}}// comment-5
+  f(0);
+  } else {
+if (a > 10)
+  return;
+else // comment-6
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use 'else' after 'return'
+// CHECK-FIXES: {{^}}// comment-6
+  f(0);
+  }
 }
 
 void foo() {
   for (unsigned x = 0; x < 42; ++x) {
 if (x) {
   continue;
-} else { // comment-3
+} else { // comment-7
 // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 'continue'
-// CHECK-FIXES: {{^}}} // comment-3
+// CHECK-FIXES: {{^}}} // comment-7
   x++;
 }
 if (x) {
   break;
-} else { // comment-4
+} else { // comment-8
 // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 'break'
-// CHECK-FIXES: {{^}}} // comment-4
+// CHECK-FIXES:

[PATCH] D25439: Fixed column shift when formatting line containing bit shift operators

2016-10-24 Thread Paweł Żukowski via cfe-commits

idlecode updated this revision to Diff 75635.

https://reviews.llvm.org/D25439

Files:
  lib/Format/FormatTokenLexer.cpp
  unittests/Format/FormatTest.cpp


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -5501,6 +5501,18 @@
   verifyFormat("< < < < < < < < < < < < < < < < < < < < < < < < < < < < < <");
 }
 
+TEST_F(FormatTest, BitshiftOperatorWidth) {
+  EXPECT_EQ("int a = 1 << 2; /* foo\n"
+"   bar */",
+format("inta=1<<2;  /* foo\n"
+   "   bar */"));
+
+  EXPECT_EQ("int b = 256 >> 1; /* foo\n"
+" bar */",
+format("int  b  =256>>1 ;  /* foo\n"
+   "  bar */"));
+}
+
 TEST_F(FormatTest, UnderstandsBinaryOperators) {
   verifyFormat("COMPARE(a, ==, b);");
   verifyFormat("auto s = sizeof...(Ts) - 1;");
Index: lib/Format/FormatTokenLexer.cpp
===
--- lib/Format/FormatTokenLexer.cpp
+++ lib/Format/FormatTokenLexer.cpp
@@ -525,10 +525,12 @@
   } else if (FormatTok->Tok.is(tok::greatergreater)) {
 FormatTok->Tok.setKind(tok::greater);
 FormatTok->TokenText = FormatTok->TokenText.substr(0, 1);
+++Column;
 StateStack.push(LexerState::TOKEN_STASHED);
   } else if (FormatTok->Tok.is(tok::lessless)) {
 FormatTok->Tok.setKind(tok::less);
 FormatTok->TokenText = FormatTok->TokenText.substr(0, 1);
+++Column;
 StateStack.push(LexerState::TOKEN_STASHED);
   }
 


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -5501,6 +5501,18 @@
   verifyFormat("< < < < < < < < < < < < < < < < < < < < < < < < < < < < < <");
 }
 
+TEST_F(FormatTest, BitshiftOperatorWidth) {
+  EXPECT_EQ("int a = 1 << 2; /* foo\n"
+"   bar */",
+format("inta=1<<2;  /* foo\n"
+   "   bar */"));
+
+  EXPECT_EQ("int b = 256 >> 1; /* foo\n"
+" bar */",
+format("int  b  =256>>1 ;  /* foo\n"
+   "  bar */"));
+}
+
 TEST_F(FormatTest, UnderstandsBinaryOperators) {
   verifyFormat("COMPARE(a, ==, b);");
   verifyFormat("auto s = sizeof...(Ts) - 1;");
Index: lib/Format/FormatTokenLexer.cpp
===
--- lib/Format/FormatTokenLexer.cpp
+++ lib/Format/FormatTokenLexer.cpp
@@ -525,10 +525,12 @@
   } else if (FormatTok->Tok.is(tok::greatergreater)) {
 FormatTok->Tok.setKind(tok::greater);
 FormatTok->TokenText = FormatTok->TokenText.substr(0, 1);
+++Column;
 StateStack.push(LexerState::TOKEN_STASHED);
   } else if (FormatTok->Tok.is(tok::lessless)) {
 FormatTok->Tok.setKind(tok::less);
 FormatTok->TokenText = FormatTok->TokenText.substr(0, 1);
+++Column;
 StateStack.push(LexerState::TOKEN_STASHED);
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D25439: Fixed column shift when formatting line containing bit shift operators

2016-10-24 Thread Paweł Żukowski via cfe-commits

idlecode added a comment.

Thanks for pointing it out, just a minute ago I found a proper document 
 mentioning it (I have no idea how I 
could miss it).
I hope to be more use in future :)




Comment at: unittests/Format/FormatTest.cpp:11365
+TEST_F(FormatTest, BitshiftOperatorWidth) {
+  std::string left = "int a = 1 << 2; /* foo\n"
+ "   bar */";

djasper wrote:
> It's always useful to have some other formatting being done in the same test. 
> We repeatedly ran into cases in the past where a test only passed because 
> some change effectively disabled formatting for a specific line. I suggest 
> writing these as:
> 
>   EXPECT_EQ("int a = 1 << 2; /* foo\n"
> "   bar */",
> format("inta=1<<2;  /* foo\n"
>"   bar */"));
Oh, that is worth mentioning, thanks :)



Comment at: unittests/Format/FormatTest.cpp:11365
+TEST_F(FormatTest, BitshiftOperatorWidth) {
+  std::string left = "int a = 1 << 2; /* foo\n"
+ "   bar */";

idlecode wrote:
> djasper wrote:
> > It's always useful to have some other formatting being done in the same 
> > test. We repeatedly ran into cases in the past where a test only passed 
> > because some change effectively disabled formatting for a specific line. I 
> > suggest writing these as:
> > 
> >   EXPECT_EQ("int a = 1 << 2; /* foo\n"
> > "   bar */",
> > format("inta=1<<2;  /* foo\n"
> >"   bar */"));
> Oh, that is worth mentioning, thanks :)
Oh, that is good to know; Done


https://reviews.llvm.org/D25439



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

[PATCH] D25439: Fixed column shift when formatting line containing bit shift operators

2016-10-12 Thread Paweł Żukowski via cfe-commits

idlecode updated this revision to Diff 74412.

https://reviews.llvm.org/D25439

Files:
  lib/Format/FormatTokenLexer.cpp
  unittests/Format/FormatTest.cpp


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -11361,6 +11361,16 @@
"llvm::outs()\n<<");
 }
 
+TEST_F(FormatTest, BitshiftOperatorWidth) {
+  std::string left = "int a = 1 << 2; /* foo\n"
+ "   bar */";
+  EXPECT_EQ(left, format(left));
+
+  std::string right = "int b = 256 >> 2; /* foo\n"
+  " bar */";
+  EXPECT_EQ(right, format(right));
+}
+
 TEST_F(FormatTest, HandleUnbalancedImplicitBracesAcrossPPBranches) {
   std::string code = "#if A\n"
  "#if B\n"
Index: lib/Format/FormatTokenLexer.cpp
===
--- lib/Format/FormatTokenLexer.cpp
+++ lib/Format/FormatTokenLexer.cpp
@@ -525,10 +525,12 @@
   } else if (FormatTok->Tok.is(tok::greatergreater)) {
 FormatTok->Tok.setKind(tok::greater);
 FormatTok->TokenText = FormatTok->TokenText.substr(0, 1);
+Column += 1;
 StateStack.push(LexerState::TOKEN_STASHED);
   } else if (FormatTok->Tok.is(tok::lessless)) {
 FormatTok->Tok.setKind(tok::less);
 FormatTok->TokenText = FormatTok->TokenText.substr(0, 1);
+Column += 1;
 StateStack.push(LexerState::TOKEN_STASHED);
   }
 


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -11361,6 +11361,16 @@
"llvm::outs()\n<<");
 }
 
+TEST_F(FormatTest, BitshiftOperatorWidth) {
+  std::string left = "int a = 1 << 2; /* foo\n"
+ "   bar */";
+  EXPECT_EQ(left, format(left));
+
+  std::string right = "int b = 256 >> 2; /* foo\n"
+  " bar */";
+  EXPECT_EQ(right, format(right));
+}
+
 TEST_F(FormatTest, HandleUnbalancedImplicitBracesAcrossPPBranches) {
   std::string code = "#if A\n"
  "#if B\n"
Index: lib/Format/FormatTokenLexer.cpp
===
--- lib/Format/FormatTokenLexer.cpp
+++ lib/Format/FormatTokenLexer.cpp
@@ -525,10 +525,12 @@
   } else if (FormatTok->Tok.is(tok::greatergreater)) {
 FormatTok->Tok.setKind(tok::greater);
 FormatTok->TokenText = FormatTok->TokenText.substr(0, 1);
+Column += 1;
 StateStack.push(LexerState::TOKEN_STASHED);
   } else if (FormatTok->Tok.is(tok::lessless)) {
 FormatTok->Tok.setKind(tok::less);
 FormatTok->TokenText = FormatTok->TokenText.substr(0, 1);
+Column += 1;
 StateStack.push(LexerState::TOKEN_STASHED);
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D25439: Fixed column shift when formatting line containing bit shift operators

2016-10-11 Thread Paweł Żukowski via cfe-commits

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

Sure, I will upload diff as soon as I test it


https://reviews.llvm.org/D25439



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

[PATCH] D25439: Fixed column shift when formatting line containing bit shift operators

2016-10-10 Thread Paweł Żukowski via cfe-commits

idlecode created this revision.
idlecode added a subscriber: cfe-commits.
Herald added a subscriber: klimek.

During clang-format source lexing >> and << operators are split and
treated as two less/greater operators but column position of following
tokens was not adjusted accordingly.


https://reviews.llvm.org/D25439

Files:
  lib/Format/FormatTokenLexer.cpp
  test/Format/bitshift-operator-width.cpp


Index: test/Format/bitshift-operator-width.cpp
===
--- /dev/null
+++ test/Format/bitshift-operator-width.cpp
@@ -0,0 +1,13 @@
+// RUN: grep -Ev "// *[A-Z-]+:" %s | clang-format -style=LLVM \
+// RUN:   | FileCheck -strict-whitespace %s
+
+// CHECK: {{^int a = 1 << 2; /\* foo$}}
+// CHECK: {{^   bar \*/$}}
+int a = 1 << 2; /* foo
+   bar */
+
+// CHECK: {{^int b = 256 >> 2; /\* foo$}}
+// CHECK: {{^ bar \*/$}}
+int b = 256 >> 2; /* foo
+ bar */
+
Index: lib/Format/FormatTokenLexer.cpp
===
--- lib/Format/FormatTokenLexer.cpp
+++ lib/Format/FormatTokenLexer.cpp
@@ -525,10 +525,12 @@
   } else if (FormatTok->Tok.is(tok::greatergreater)) {
 FormatTok->Tok.setKind(tok::greater);
 FormatTok->TokenText = FormatTok->TokenText.substr(0, 1);
+Column += 1;
 StateStack.push(LexerState::TOKEN_STASHED);
   } else if (FormatTok->Tok.is(tok::lessless)) {
 FormatTok->Tok.setKind(tok::less);
 FormatTok->TokenText = FormatTok->TokenText.substr(0, 1);
+Column += 1;
 StateStack.push(LexerState::TOKEN_STASHED);
   }
 


Index: test/Format/bitshift-operator-width.cpp
===
--- /dev/null
+++ test/Format/bitshift-operator-width.cpp
@@ -0,0 +1,13 @@
+// RUN: grep -Ev "// *[A-Z-]+:" %s | clang-format -style=LLVM \
+// RUN:   | FileCheck -strict-whitespace %s
+
+// CHECK: {{^int a = 1 << 2; /\* foo$}}
+// CHECK: {{^   bar \*/$}}
+int a = 1 << 2; /* foo
+   bar */
+
+// CHECK: {{^int b = 256 >> 2; /\* foo$}}
+// CHECK: {{^ bar \*/$}}
+int b = 256 >> 2; /* foo
+ bar */
+
Index: lib/Format/FormatTokenLexer.cpp
===
--- lib/Format/FormatTokenLexer.cpp
+++ lib/Format/FormatTokenLexer.cpp
@@ -525,10 +525,12 @@
   } else if (FormatTok->Tok.is(tok::greatergreater)) {
 FormatTok->Tok.setKind(tok::greater);
 FormatTok->TokenText = FormatTok->TokenText.substr(0, 1);
+Column += 1;
 StateStack.push(LexerState::TOKEN_STASHED);
   } else if (FormatTok->Tok.is(tok::lessless)) {
 FormatTok->Tok.setKind(tok::less);
 FormatTok->TokenText = FormatTok->TokenText.substr(0, 1);
+Column += 1;
 StateStack.push(LexerState::TOKEN_STASHED);
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D25439: Fixed column shift when formatting line containing bit shift operators

[PATCH] D26125: [clang-tidy] Fixed readability-else-after-return for cascade statements

[PATCH] D26125: [clang-tidy] Fixed else-after-return warning in cascade if statement

[PATCH] D26125: [clang-tidy] Fixed else-after-return warning in cascade if statement

[PATCH] D26125: [clang-tidy] Fixed else-after-return warning in cascade if statement

[PATCH] D26125: [clang-tidy] Fixed else-after-return warning in cascade if statement

[PATCH] D26125: [clang-tidy] Fixed else-after-return warning in cascade if statement

[PATCH] D25439: Fixed column shift when formatting line containing bit shift operators

[PATCH] D25439: Fixed column shift when formatting line containing bit shift operators

[PATCH] D25439: Fixed column shift when formatting line containing bit shift operators

[PATCH] D25439: Fixed column shift when formatting line containing bit shift operators

[PATCH] D25439: Fixed column shift when formatting line containing bit shift operators

12 matches

Site Navigation

Mail list logo

Footer information