[PATCH] D43303: [Format] Fix for bug 35641
This revision was automatically updated to reflect the committed changes.
Closed by commit rL338578: [Format] Fix for bug 35641 (authored by ibiryukov,
committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D43303?vs=134830=158546#toc
Repository:
rL LLVM
https://reviews.llvm.org/D43303
Files:
cfe/trunk/lib/Format/WhitespaceManager.cpp
cfe/trunk/unittests/Format/FormatTest.cpp
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -9804,6 +9804,14 @@
"});\n",
Alignment);
Alignment.PointerAlignment = FormatStyle::PAS_Right;
+
+ // See llvm.org/PR35641
+ Alignment.AlignConsecutiveDeclarations = true;
+ verifyFormat("int func() { //\n"
+ " int b;\n"
+ " unsigned c;\n"
+ "}",
+ Alignment);
}
TEST_F(FormatTest, LinuxBraceBreaking) {
Index: cfe/trunk/lib/Format/WhitespaceManager.cpp
===
--- cfe/trunk/lib/Format/WhitespaceManager.cpp
+++ cfe/trunk/lib/Format/WhitespaceManager.cpp
@@ -255,8 +255,14 @@
Changes[ScopeStack.back()].indentAndNestingLevel())
ScopeStack.pop_back();
+// Compare current token to previous non-comment token to ensure whether
+// it is in a deeper scope or not.
+unsigned PreviousNonComment = i - 1;
+while (PreviousNonComment > Start &&
+ Changes[PreviousNonComment].Tok->is(tok::comment))
+ PreviousNonComment--;
if (i != Start && Changes[i].indentAndNestingLevel() >
- Changes[i - 1].indentAndNestingLevel())
+ Changes[PreviousNonComment].indentAndNestingLevel())
ScopeStack.push_back(i);
bool InsideNestedScope = ScopeStack.size() != 0;
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -9804,6 +9804,14 @@
"});\n",
Alignment);
Alignment.PointerAlignment = FormatStyle::PAS_Right;
+
+ // See llvm.org/PR35641
+ Alignment.AlignConsecutiveDeclarations = true;
+ verifyFormat("int func() { //\n"
+ " int b;\n"
+ " unsigned c;\n"
+ "}",
+ Alignment);
}
TEST_F(FormatTest, LinuxBraceBreaking) {
Index: cfe/trunk/lib/Format/WhitespaceManager.cpp
===
--- cfe/trunk/lib/Format/WhitespaceManager.cpp
+++ cfe/trunk/lib/Format/WhitespaceManager.cpp
@@ -255,8 +255,14 @@
Changes[ScopeStack.back()].indentAndNestingLevel())
ScopeStack.pop_back();
+// Compare current token to previous non-comment token to ensure whether
+// it is in a deeper scope or not.
+unsigned PreviousNonComment = i - 1;
+while (PreviousNonComment > Start &&
+ Changes[PreviousNonComment].Tok->is(tok::comment))
+ PreviousNonComment--;
if (i != Start && Changes[i].indentAndNestingLevel() >
- Changes[i - 1].indentAndNestingLevel())
+ Changes[PreviousNonComment].indentAndNestingLevel())
ScopeStack.push_back(i);
bool InsideNestedScope = ScopeStack.size() != 0;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43303: [Format] Fix for bug 35641
kadircet added a comment. ping Repository: rC Clang https://reviews.llvm.org/D43303 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43303: [Format] Fix for bug 35641
kadircet updated this revision to Diff 134830.
kadircet added a comment.
Rebased and uploaded diff to the master. Sorry for the inconvenience.
Repository:
rC Clang
https://reviews.llvm.org/D43303
Files:
lib/Format/WhitespaceManager.cpp
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -9449,6 +9449,14 @@
"});\n",
Alignment);
Alignment.PointerAlignment = FormatStyle::PAS_Right;
+
+ // See llvm.org/PR35641
+ Alignment.AlignConsecutiveDeclarations = true;
+ verifyFormat("int func() { //\n"
+ " int b;\n"
+ " unsigned c;\n"
+ "}",
+ Alignment);
}
TEST_F(FormatTest, LinuxBraceBreaking) {
Index: lib/Format/WhitespaceManager.cpp
===
--- lib/Format/WhitespaceManager.cpp
+++ lib/Format/WhitespaceManager.cpp
@@ -255,8 +255,14 @@
Changes[ScopeStack.back()].indentAndNestingLevel())
ScopeStack.pop_back();
+// Compare current token to previous non-comment token to ensure whether
+// it is in a deeper scope or not.
+unsigned PreviousNonComment = i - 1;
+while (PreviousNonComment > Start &&
+ Changes[PreviousNonComment].Tok->is(tok::comment))
+ PreviousNonComment--;
if (i != Start && Changes[i].indentAndNestingLevel() >
- Changes[i - 1].indentAndNestingLevel())
+ Changes[PreviousNonComment].indentAndNestingLevel())
ScopeStack.push_back(i);
bool InsideNestedScope = ScopeStack.size() != 0;
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -9449,6 +9449,14 @@
"});\n",
Alignment);
Alignment.PointerAlignment = FormatStyle::PAS_Right;
+
+ // See llvm.org/PR35641
+ Alignment.AlignConsecutiveDeclarations = true;
+ verifyFormat("int func() { //\n"
+ " int b;\n"
+ " unsigned c;\n"
+ "}",
+ Alignment);
}
TEST_F(FormatTest, LinuxBraceBreaking) {
Index: lib/Format/WhitespaceManager.cpp
===
--- lib/Format/WhitespaceManager.cpp
+++ lib/Format/WhitespaceManager.cpp
@@ -255,8 +255,14 @@
Changes[ScopeStack.back()].indentAndNestingLevel())
ScopeStack.pop_back();
+// Compare current token to previous non-comment token to ensure whether
+// it is in a deeper scope or not.
+unsigned PreviousNonComment = i - 1;
+while (PreviousNonComment > Start &&
+ Changes[PreviousNonComment].Tok->is(tok::comment))
+ PreviousNonComment--;
if (i != Start && Changes[i].indentAndNestingLevel() >
- Changes[i - 1].indentAndNestingLevel())
+ Changes[PreviousNonComment].indentAndNestingLevel())
ScopeStack.push_back(i);
bool InsideNestedScope = ScopeStack.size() != 0;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43303: [Format] Fix for bug 35641
lebedev.ri added a comment. The diff looks wrong, the `lib/Format/WhitespaceManager.cpp` change gone missing. You need to upload the diff to the `git master`/svn trunk/etc, *NOT* to the previous commit (previous diff). Repository: rC Clang https://reviews.llvm.org/D43303 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43303: [Format] Fix for bug 35641
kadircet added a comment. Btw, I cannot commit the change myself, don't have commit rights. Repository: rC Clang https://reviews.llvm.org/D43303 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43303: [Format] Fix for bug 35641
kadircet updated this revision to Diff 134414.
kadircet added a comment.
Changed description of unit test to include direct link to bug.
Used different variable types to make sure alignment happens.
Repository:
rC Clang
https://reviews.llvm.org/D43303
Files:
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -9450,11 +9450,11 @@
Alignment);
Alignment.PointerAlignment = FormatStyle::PAS_Right;
- // Bug 35641
+ // See llvm.org/PR35641
Alignment.AlignConsecutiveDeclarations = true;
verifyFormat("int func() { //\n"
- " int b;\n"
- " int c;\n"
+ " int b;\n"
+ " unsigned c;\n"
"}",
Alignment);
}
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -9450,11 +9450,11 @@
Alignment);
Alignment.PointerAlignment = FormatStyle::PAS_Right;
- // Bug 35641
+ // See llvm.org/PR35641
Alignment.AlignConsecutiveDeclarations = true;
verifyFormat("int func() { //\n"
- " int b;\n"
- " int c;\n"
+ " int b;\n"
+ " unsigned c;\n"
"}",
Alignment);
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43303: [Format] Fix for bug 35641
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Thanks for the fix. Comment at: unittests/Format/FormatTest.cpp:9453 + + // Bug 35641 + Alignment.AlignConsecutiveDeclarations = true; Make this "See llvm.org/PR35641". Comment at: unittests/Format/FormatTest.cpp:9457 + " int b;\n" + " int c;\n" + "}", Might be useful to use different types here to verify that alignment is actually happening, e.g. "int" and "unsigned". Repository: rC Clang https://reviews.llvm.org/D43303 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43303: [Format] Fix for bug 35641
kadircet created this revision.
kadircet added reviewers: rsmith, djasper.
kadircet added a project: clang.
Herald added a subscriber: cfe-commits.
Bug was caused due to comments at the start of scope. For a code like:
int func() { //
int b;
int c;
}
the comment at the first line gets IndentAndNestingLevel (1,1) whereas
the following declarations get only (0,1) which prevents them from insertion
of a new scope. So, I changed the AlignTokenSequence to look at previous
*non-comment* token when deciding whether to introduce a new scope into
stack or not.
Repository:
rC Clang
https://reviews.llvm.org/D43303
Files:
lib/Format/WhitespaceManager.cpp
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -9449,6 +9449,14 @@
"});\n",
Alignment);
Alignment.PointerAlignment = FormatStyle::PAS_Right;
+
+ // Bug 35641
+ Alignment.AlignConsecutiveDeclarations = true;
+ verifyFormat("int func() { //\n"
+ " int b;\n"
+ " int c;\n"
+ "}",
+ Alignment);
}
TEST_F(FormatTest, LinuxBraceBreaking) {
Index: lib/Format/WhitespaceManager.cpp
===
--- lib/Format/WhitespaceManager.cpp
+++ lib/Format/WhitespaceManager.cpp
@@ -255,8 +255,14 @@
Changes[ScopeStack.back()].indentAndNestingLevel())
ScopeStack.pop_back();
+// Compare current token to previous non-comment token to ensure whether
+// it is in a deeper scope or not.
+unsigned PreviousNonComment = i - 1;
+while (PreviousNonComment > Start &&
+ Changes[PreviousNonComment].Tok->is(tok::comment))
+ PreviousNonComment--;
if (i != Start && Changes[i].indentAndNestingLevel() >
- Changes[i - 1].indentAndNestingLevel())
+ Changes[PreviousNonComment].indentAndNestingLevel())
ScopeStack.push_back(i);
bool InsideNestedScope = ScopeStack.size() != 0;
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -9449,6 +9449,14 @@
"});\n",
Alignment);
Alignment.PointerAlignment = FormatStyle::PAS_Right;
+
+ // Bug 35641
+ Alignment.AlignConsecutiveDeclarations = true;
+ verifyFormat("int func() { //\n"
+ " int b;\n"
+ " int c;\n"
+ "}",
+ Alignment);
}
TEST_F(FormatTest, LinuxBraceBreaking) {
Index: lib/Format/WhitespaceManager.cpp
===
--- lib/Format/WhitespaceManager.cpp
+++ lib/Format/WhitespaceManager.cpp
@@ -255,8 +255,14 @@
Changes[ScopeStack.back()].indentAndNestingLevel())
ScopeStack.pop_back();
+// Compare current token to previous non-comment token to ensure whether
+// it is in a deeper scope or not.
+unsigned PreviousNonComment = i - 1;
+while (PreviousNonComment > Start &&
+ Changes[PreviousNonComment].Tok->is(tok::comment))
+ PreviousNonComment--;
if (i != Start && Changes[i].indentAndNestingLevel() >
- Changes[i - 1].indentAndNestingLevel())
+ Changes[PreviousNonComment].indentAndNestingLevel())
ScopeStack.push_back(i);
bool InsideNestedScope = ScopeStack.size() != 0;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
