MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: krasimir, JakeMerdichAMD, curdeius, sammccall.
MyDeveloperDay added projects: clang, clang-format.
https://bugs.llvm.org/show_bug.cgi?id=46130 from Twitter
https://twitter.com/ikautak/status/1265998988232159232
I have seen this myself many times.. if you have format on save and you work in
an editor where you are constantly saving (:w muscle memory)
If you are in the middle of editing and somehow you've missed a { or } in your
code, somewhere, often way below where you are at the bottom of your file the
namespace comment fixer will have put the namespace on the previous closing
brace.
This leads to you having to fix up the bottom of the file.
This revision prevents that happening by performing an initial pass of the
tokens and simply counting the number of `{` and `}` and ensuring they balance.
If they don't balance we don't do any namespace fixing as it will likely be
unstable and incorrect.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D80830
Files:
clang/lib/Format/NamespaceEndCommentsFixer.cpp
clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
Index: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
===================================================================
--- clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
+++ clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
@@ -1089,6 +1089,34 @@
"void d() {\n"
"}\n"));
}
+
+TEST_F(NamespaceEndCommentsFixerTest, IgnoreUnbalanced) {
+ EXPECT_EQ("namespace A {\n"
+ "class Foo {\n"
+ "}\n"
+ "}// namespace A\n",
+ fixNamespaceEndComments("namespace A {\n"
+ "class Foo {\n"
+ "}\n"
+ "}\n"));
+ EXPECT_EQ("namespace A {\n"
+ "class Foo {\n"
+ "}\n",
+ fixNamespaceEndComments("namespace A {\n"
+ "class Foo {\n"
+ "}\n"));
+
+ EXPECT_EQ("namespace A {\n"
+ "class Foo {\n"
+ "}\n"
+ "}\n"
+ "}\n",
+ fixNamespaceEndComments("namespace A {\n"
+ "class Foo {\n"
+ "}\n"
+ "}\n"
+ "}\n"));
+}
} // end namespace
} // end namespace format
} // end namespace clang
Index: clang/lib/Format/NamespaceEndCommentsFixer.cpp
===================================================================
--- clang/lib/Format/NamespaceEndCommentsFixer.cpp
+++ clang/lib/Format/NamespaceEndCommentsFixer.cpp
@@ -205,6 +205,23 @@
const SourceManager &SourceMgr = Env.getSourceManager();
AffectedRangeMgr.computeAffectedLines(AnnotatedLines);
tooling::Replacements Fixes;
+
+ // Spin through the lines and ensure we have balanced braces.
+ int Braces = 0;
+ for (size_t I = 0, E = AnnotatedLines.size(); I != E; ++I) {
+ FormatToken *Tok = AnnotatedLines[I]->First;
+ while (Tok) {
+ Braces += Tok->is(tok::l_brace) ? 1 : Tok->is(tok::r_brace) ? -1 : 0;
+ Tok = Tok->Next;
+ }
+ }
+ // Don't attempt to comment unbalanced braces or this can
+ // lead to comments being placed on the closing brace which isn't
+ // the matching brace of the namespace. (occurs during incomplete editing).
+ if (Braces != 0) {
+ return {Fixes, 0};
+ }
+
std::string AllNamespaceNames = "";
size_t StartLineIndex = SIZE_MAX;
StringRef NamespaceTokenText;
Index: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
===================================================================
--- clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
+++ clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
@@ -1089,6 +1089,34 @@
"void d() {\n"
"}\n"));
}
+
+TEST_F(NamespaceEndCommentsFixerTest, IgnoreUnbalanced) {
+ EXPECT_EQ("namespace A {\n"
+ "class Foo {\n"
+ "}\n"
+ "}// namespace A\n",
+ fixNamespaceEndComments("namespace A {\n"
+ "class Foo {\n"
+ "}\n"
+ "}\n"));
+ EXPECT_EQ("namespace A {\n"
+ "class Foo {\n"
+ "}\n",
+ fixNamespaceEndComments("namespace A {\n"
+ "class Foo {\n"
+ "}\n"));
+
+ EXPECT_EQ("namespace A {\n"
+ "class Foo {\n"
+ "}\n"
+ "}\n"
+ "}\n",
+ fixNamespaceEndComments("namespace A {\n"
+ "class Foo {\n"
+ "}\n"
+ "}\n"
+ "}\n"));
+}
} // end namespace
} // end namespace format
} // end namespace clang
Index: clang/lib/Format/NamespaceEndCommentsFixer.cpp
===================================================================
--- clang/lib/Format/NamespaceEndCommentsFixer.cpp
+++ clang/lib/Format/NamespaceEndCommentsFixer.cpp
@@ -205,6 +205,23 @@
const SourceManager &SourceMgr = Env.getSourceManager();
AffectedRangeMgr.computeAffectedLines(AnnotatedLines);
tooling::Replacements Fixes;
+
+ // Spin through the lines and ensure we have balanced braces.
+ int Braces = 0;
+ for (size_t I = 0, E = AnnotatedLines.size(); I != E; ++I) {
+ FormatToken *Tok = AnnotatedLines[I]->First;
+ while (Tok) {
+ Braces += Tok->is(tok::l_brace) ? 1 : Tok->is(tok::r_brace) ? -1 : 0;
+ Tok = Tok->Next;
+ }
+ }
+ // Don't attempt to comment unbalanced braces or this can
+ // lead to comments being placed on the closing brace which isn't
+ // the matching brace of the namespace. (occurs during incomplete editing).
+ if (Braces != 0) {
+ return {Fixes, 0};
+ }
+
std::string AllNamespaceNames = "";
size_t StartLineIndex = SIZE_MAX;
StringRef NamespaceTokenText;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
