[PATCH] D158363: [clang-format] Fix segmentation fault when formatting nested namespaces

[Owen Pan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf465a482caa9: [clang-format] Fix segmentation fault when 
formatting nested namespaces (authored by d0nc1h0t, committed by owenpan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158363

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4180,6 +4180,16 @@
"void foo() {}\n"
"} // namespace ns",
Style);
+
+  FormatStyle LLVMWithCompactInnerNamespace = getLLVMStyle();
+  LLVMWithCompactInnerNamespace.CompactNamespaces = true;
+  LLVMWithCompactInnerNamespace.NamespaceIndentation = FormatStyle::NI_Inner;
+  verifyFormat("namespace ns1 { namespace ns2 { namespace ns3 {\n"
+   "// block for debug mode\n"
+   "#ifndef NDEBUG\n"
+   "#endif\n"
+   "}}} // namespace ns1::ns2::ns3",
+   LLVMWithCompactInnerNamespace);
 }
 
 TEST_F(FormatTest, NamespaceMacros) {
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -386,11 +386,14 @@
   // Reduce indent level for bodies of namespaces which were compacted,
   // but only if their content was indented in the first place.
   auto *ClosingLine = AnnotatedLines.begin() + ClosingLineIndex + 1;
-  auto OutdentBy = I[J]->Level - TheLine->Level;
+  const int OutdentBy = I[J]->Level - TheLine->Level;
+  assert(OutdentBy >= 0);
   for (auto *CompactedLine = I + J; CompactedLine <= ClosingLine;
++CompactedLine) {
-if (!(*CompactedLine)->InPPDirective)
-  (*CompactedLine)->Level -= OutdentBy;
+if (!(*CompactedLine)->InPPDirective) {
+  const int Level = (*CompactedLine)->Level;
+  (*CompactedLine)->Level = std::max(Level - OutdentBy, 0);
+}
   }
 }
 return J - 1;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4180,6 +4180,16 @@
"void foo() {}\n"
"} // namespace ns",
Style);
+
+  FormatStyle LLVMWithCompactInnerNamespace = getLLVMStyle();
+  LLVMWithCompactInnerNamespace.CompactNamespaces = true;
+  LLVMWithCompactInnerNamespace.NamespaceIndentation = FormatStyle::NI_Inner;
+  verifyFormat("namespace ns1 { namespace ns2 { namespace ns3 {\n"
+   "// block for debug mode\n"
+   "#ifndef NDEBUG\n"
+   "#endif\n"
+   "}}} // namespace ns1::ns2::ns3",
+   LLVMWithCompactInnerNamespace);
 }
 
 TEST_F(FormatTest, NamespaceMacros) {
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -386,11 +386,14 @@
   // Reduce indent level for bodies of namespaces which were compacted,
   // but only if their content was indented in the first place.
   auto *ClosingLine = AnnotatedLines.begin() + ClosingLineIndex + 1;
-  auto OutdentBy = I[J]->Level - TheLine->Level;
+  const int OutdentBy = I[J]->Level - TheLine->Level;
+  assert(OutdentBy >= 0);
   for (auto *CompactedLine = I + J; CompactedLine <= ClosingLine;
++CompactedLine) {
-if (!(*CompactedLine)->InPPDirective)
-  (*CompactedLine)->Level -= OutdentBy;
+if (!(*CompactedLine)->InPPDirective) {
+  const int Level = (*CompactedLine)->Level;
+  (*CompactedLine)->Level = std::max(Level - OutdentBy, 0);
+}
   }
 }
 return J - 1;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158363: [clang-format] Fix segmentation fault when formatting nested namespaces

[Arkadiy Yudintsev via Phabricator via cfe-commits]

d0nc1h0t added a comment.

In D158363#463 , @owenpan wrote:

> We need the name and email you want to use in order to commit the patch for 
> you.

Arkadiy Yudintsev 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158363

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

[PATCH] D158363: [clang-format] Fix segmentation fault when formatting nested namespaces

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

We need the name and email you want to use in order to commit the patch for you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158363

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

[PATCH] D158363: [clang-format] Fix segmentation fault when formatting nested namespaces

[Arkadiy Yudintsev via Phabricator via cfe-commits]

d0nc1h0t added a comment.

In D158363#4632451 , @owenpan wrote:

> Do you need us to commit it for you? See 
> https://llvm.org/docs/Phabricator.html#committing-someone-s-change-from-phabricator.

'git push' or 'arc land' give an error 'remote: Permission to 
llvm/llvm-project.git denied to d0nc1h0t'. 
On an empty folder I tried

  git clone https://github.com/llvm/llvm-project.git/ .
  arc patch D158363
  arc land

with the same error. Maybe there are not enough rights to the project on GitHub?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158363

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

[PATCH] D158363: [clang-format] Fix segmentation fault when formatting nested namespaces

[Arkadiy Yudintsev via Phabricator via cfe-commits]

d0nc1h0t updated this revision to Diff 555317.
d0nc1h0t added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158363

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4180,6 +4180,16 @@
"void foo() {}\n"
"} // namespace ns",
Style);
+
+  FormatStyle LLVMWithCompactInnerNamespace = getLLVMStyle();
+  LLVMWithCompactInnerNamespace.CompactNamespaces = true;
+  LLVMWithCompactInnerNamespace.NamespaceIndentation = FormatStyle::NI_Inner;
+  verifyFormat("namespace ns1 { namespace ns2 { namespace ns3 {\n"
+   "// block for debug mode\n"
+   "#ifndef NDEBUG\n"
+   "#endif\n"
+   "}}} // namespace ns1::ns2::ns3",
+   LLVMWithCompactInnerNamespace);
 }
 
 TEST_F(FormatTest, NamespaceMacros) {
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -386,11 +386,14 @@
   // Reduce indent level for bodies of namespaces which were compacted,
   // but only if their content was indented in the first place.
   auto *ClosingLine = AnnotatedLines.begin() + ClosingLineIndex + 1;
-  auto OutdentBy = I[J]->Level - TheLine->Level;
+  const int OutdentBy = I[J]->Level - TheLine->Level;
+  assert(OutdentBy >= 0);
   for (auto *CompactedLine = I + J; CompactedLine <= ClosingLine;
++CompactedLine) {
-if (!(*CompactedLine)->InPPDirective)
-  (*CompactedLine)->Level -= OutdentBy;
+if (!(*CompactedLine)->InPPDirective) {
+  const int Level = (*CompactedLine)->Level;
+  (*CompactedLine)->Level = std::max(Level - OutdentBy, 0);
+}
   }
 }
 return J - 1;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4180,6 +4180,16 @@
"void foo() {}\n"
"} // namespace ns",
Style);
+
+  FormatStyle LLVMWithCompactInnerNamespace = getLLVMStyle();
+  LLVMWithCompactInnerNamespace.CompactNamespaces = true;
+  LLVMWithCompactInnerNamespace.NamespaceIndentation = FormatStyle::NI_Inner;
+  verifyFormat("namespace ns1 { namespace ns2 { namespace ns3 {\n"
+   "// block for debug mode\n"
+   "#ifndef NDEBUG\n"
+   "#endif\n"
+   "}}} // namespace ns1::ns2::ns3",
+   LLVMWithCompactInnerNamespace);
 }
 
 TEST_F(FormatTest, NamespaceMacros) {
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -386,11 +386,14 @@
   // Reduce indent level for bodies of namespaces which were compacted,
   // but only if their content was indented in the first place.
   auto *ClosingLine = AnnotatedLines.begin() + ClosingLineIndex + 1;
-  auto OutdentBy = I[J]->Level - TheLine->Level;
+  const int OutdentBy = I[J]->Level - TheLine->Level;
+  assert(OutdentBy >= 0);
   for (auto *CompactedLine = I + J; CompactedLine <= ClosingLine;
++CompactedLine) {
-if (!(*CompactedLine)->InPPDirective)
-  (*CompactedLine)->Level -= OutdentBy;
+if (!(*CompactedLine)->InPPDirective) {
+  const int Level = (*CompactedLine)->Level;
+  (*CompactedLine)->Level = std::max(Level - OutdentBy, 0);
+}
   }
 }
 return J - 1;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158363: [clang-format] Fix segmentation fault when formatting nested namespaces

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

Do you need us to commit it for you? See 
https://llvm.org/docs/Phabricator.html#committing-someone-s-change-from-phabricator.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158363

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

[PATCH] D158363: [clang-format] Fix segmentation fault when formatting nested namespaces

[Arkadiy Yudintsev via Phabricator via cfe-commits]

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

Fixed. (Relied on auto-formatting in the IDE)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158363

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

[PATCH] D158363: [clang-format] Fix segmentation fault when formatting nested namespaces

[Arkadiy Yudintsev via Phabricator via cfe-commits]

d0nc1h0t updated this revision to Diff 553085.
d0nc1h0t added a comment.

Added missing const + clang-format.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158363

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4223,6 +4223,16 @@
"void foo() {}\n"
"} // namespace ns\n",
Style);
+
+  FormatStyle LLVMWithCompactInnerNamespace = getLLVMStyle();
+  LLVMWithCompactInnerNamespace.CompactNamespaces = true;
+  LLVMWithCompactInnerNamespace.NamespaceIndentation = FormatStyle::NI_Inner;
+  verifyFormat("namespace ns1 { namespace ns2 { namespace ns3 {\n"
+   "// block for debug mode\n"
+   "#ifndef NDEBUG\n"
+   "#endif\n"
+   "}}} // namespace ns1::ns2::ns3",
+   LLVMWithCompactInnerNamespace);
 }
 
 TEST_F(FormatTest, NamespaceMacros) {
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -386,11 +386,14 @@
   // Reduce indent level for bodies of namespaces which were compacted,
   // but only if their content was indented in the first place.
   auto *ClosingLine = AnnotatedLines.begin() + ClosingLineIndex + 1;
-  auto OutdentBy = I[J]->Level - TheLine->Level;
+  const int OutdentBy = I[J]->Level - TheLine->Level;
+  assert(OutdentBy >= 0);
   for (auto *CompactedLine = I + J; CompactedLine <= ClosingLine;
++CompactedLine) {
-if (!(*CompactedLine)->InPPDirective)
-  (*CompactedLine)->Level -= OutdentBy;
+if (!(*CompactedLine)->InPPDirective) {
+  const int Level = (*CompactedLine)->Level;
+  (*CompactedLine)->Level = std::max(Level - OutdentBy, 0);
+}
   }
 }
 return J - 1;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4223,6 +4223,16 @@
"void foo() {}\n"
"} // namespace ns\n",
Style);
+
+  FormatStyle LLVMWithCompactInnerNamespace = getLLVMStyle();
+  LLVMWithCompactInnerNamespace.CompactNamespaces = true;
+  LLVMWithCompactInnerNamespace.NamespaceIndentation = FormatStyle::NI_Inner;
+  verifyFormat("namespace ns1 { namespace ns2 { namespace ns3 {\n"
+   "// block for debug mode\n"
+   "#ifndef NDEBUG\n"
+   "#endif\n"
+   "}}} // namespace ns1::ns2::ns3",
+   LLVMWithCompactInnerNamespace);
 }
 
 TEST_F(FormatTest, NamespaceMacros) {
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -386,11 +386,14 @@
   // Reduce indent level for bodies of namespaces which were compacted,
   // but only if their content was indented in the first place.
   auto *ClosingLine = AnnotatedLines.begin() + ClosingLineIndex + 1;
-  auto OutdentBy = I[J]->Level - TheLine->Level;
+  const int OutdentBy = I[J]->Level - TheLine->Level;
+  assert(OutdentBy >= 0);
   for (auto *CompactedLine = I + J; CompactedLine <= ClosingLine;
++CompactedLine) {
-if (!(*CompactedLine)->InPPDirective)
-  (*CompactedLine)->Level -= OutdentBy;
+if (!(*CompactedLine)->InPPDirective) {
+  const int Level = (*CompactedLine)->Level;
+  (*CompactedLine)->Level = std::max(Level - OutdentBy, 0);
+}
   }
 }
 return J - 1;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158363: [clang-format] Fix segmentation fault when formatting nested namespaces

[Owen Pan via Phabricator via cfe-commits]

owenpan accepted this revision.
owenpan added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:389
   auto *ClosingLine = AnnotatedLines.begin() + ClosingLineIndex + 1;
-  auto OutdentBy = I[J]->Level - TheLine->Level;
+  int OutdentBy = I[J]->Level - TheLine->Level;
+  assert(OutdentBy >= 0);

I forgot to suggest `const`.



Comment at: clang/unittests/Format/FormatTest.cpp:4231-4234
+"// block for debug mode\n"
+"#ifndef NDEBUG\n"
+"#endif\n"
+"}}} // namespace ns1::ns2::ns3",

Did you run `git-clang-format`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158363

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

[PATCH] D158363: [clang-format] Fix segmentation fault when formatting nested namespaces

[Arkadiy Yudintsev via Phabricator via cfe-commits]

d0nc1h0t updated this revision to Diff 553017.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158363

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4223,6 +4223,16 @@
"void foo() {}\n"
"} // namespace ns\n",
Style);
+
+  FormatStyle LLVMWithCompactInnerNamespace = getLLVMStyle();
+  LLVMWithCompactInnerNamespace.CompactNamespaces = true;
+  LLVMWithCompactInnerNamespace.NamespaceIndentation = FormatStyle::NI_Inner;
+  verifyFormat("namespace ns1 { namespace ns2 { namespace ns3 {\n"
+"// block for debug mode\n"
+"#ifndef NDEBUG\n"
+"#endif\n"
+"}}} // namespace ns1::ns2::ns3",
+   LLVMWithCompactInnerNamespace);
 }
 
 TEST_F(FormatTest, NamespaceMacros) {
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -386,11 +386,14 @@
   // Reduce indent level for bodies of namespaces which were compacted,
   // but only if their content was indented in the first place.
   auto *ClosingLine = AnnotatedLines.begin() + ClosingLineIndex + 1;
-  auto OutdentBy = I[J]->Level - TheLine->Level;
+  int OutdentBy = I[J]->Level - TheLine->Level;
+  assert(OutdentBy >= 0);
   for (auto *CompactedLine = I + J; CompactedLine <= ClosingLine;
++CompactedLine) {
-if (!(*CompactedLine)->InPPDirective)
-  (*CompactedLine)->Level -= OutdentBy;
+if (!(*CompactedLine)->InPPDirective) {
+  const int Level = (*CompactedLine)->Level;
+  (*CompactedLine)->Level = std::max(Level - OutdentBy, 0);
+}
   }
 }
 return J - 1;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4223,6 +4223,16 @@
"void foo() {}\n"
"} // namespace ns\n",
Style);
+
+  FormatStyle LLVMWithCompactInnerNamespace = getLLVMStyle();
+  LLVMWithCompactInnerNamespace.CompactNamespaces = true;
+  LLVMWithCompactInnerNamespace.NamespaceIndentation = FormatStyle::NI_Inner;
+  verifyFormat("namespace ns1 { namespace ns2 { namespace ns3 {\n"
+"// block for debug mode\n"
+"#ifndef NDEBUG\n"
+"#endif\n"
+"}}} // namespace ns1::ns2::ns3",
+   LLVMWithCompactInnerNamespace);
 }
 
 TEST_F(FormatTest, NamespaceMacros) {
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -386,11 +386,14 @@
   // Reduce indent level for bodies of namespaces which were compacted,
   // but only if their content was indented in the first place.
   auto *ClosingLine = AnnotatedLines.begin() + ClosingLineIndex + 1;
-  auto OutdentBy = I[J]->Level - TheLine->Level;
+  int OutdentBy = I[J]->Level - TheLine->Level;
+  assert(OutdentBy >= 0);
   for (auto *CompactedLine = I + J; CompactedLine <= ClosingLine;
++CompactedLine) {
-if (!(*CompactedLine)->InPPDirective)
-  (*CompactedLine)->Level -= OutdentBy;
+if (!(*CompactedLine)->InPPDirective) {
+  const int Level = (*CompactedLine)->Level;
+  (*CompactedLine)->Level = std::max(Level - OutdentBy, 0);
+}
   }
 }
 return J - 1;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158363: [clang-format] Fix segmentation fault when formatting nested namespaces

[Owen Pan via Phabricator via cfe-commits]

owenpan requested changes to this revision.
owenpan added a comment.
This revision now requires changes to proceed.

It seems that you forgot to update the diff?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158363

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

[PATCH] D158363: [clang-format] Fix segmentation fault when formatting nested namespaces

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:389
   auto *ClosingLine = AnnotatedLines.begin() + ClosingLineIndex + 1;
   auto OutdentBy = I[J]->Level - TheLine->Level;
   for (auto *CompactedLine = I + J; CompactedLine <= ClosingLine;

See below.



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:392-393
++CompactedLine) {
 if (!(*CompactedLine)->InPPDirective)
-  (*CompactedLine)->Level -= OutdentBy;
+  (*CompactedLine)->Level -= std::min(OutdentBy, 
(*CompactedLine)->Level);
   }

Or similar.



Comment at: clang/unittests/Format/FormatTest.cpp:4230-4237
+  verifyNoCrash("namespace ns1 { namespace ns2 { namespace ns3 {\n"
+"void func_in_ns() {\n"
+"  int res{0};\n"
+"// block for debug mode\n"
+"#ifndef NDEBUG\n"
+"#endif\n"
+"}\n"

It seems that we can reduce the test case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158363

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

[PATCH] D158363: [clang-format] Fix segmentation fault when formatting nested namespaces

[Arkadiy Yudintsev via Phabricator via cfe-commits]

d0nc1h0t updated this revision to Diff 552604.
d0nc1h0t added a comment.

Updated the patch with full context.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158363

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4223,6 +4223,19 @@
"void foo() {}\n"
"} // namespace ns\n",
Style);
+
+  FormatStyle LLVMWithCompactInnerNamespace = getLLVMStyle();
+  LLVMWithCompactInnerNamespace.CompactNamespaces = true;
+  LLVMWithCompactInnerNamespace.NamespaceIndentation = FormatStyle::NI_Inner;
+  verifyNoCrash("namespace ns1 { namespace ns2 { namespace ns3 {\n"
+"void func_in_ns() {\n"
+"  int res{0};\n"
+"// block for debug mode\n"
+"#ifndef NDEBUG\n"
+"#endif\n"
+"}\n"
+"}}}\n",
+LLVMWithCompactInnerNamespace);
 }
 
 TEST_F(FormatTest, NamespaceMacros) {
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -390,7 +390,7 @@
   for (auto *CompactedLine = I + J; CompactedLine <= ClosingLine;
++CompactedLine) {
 if (!(*CompactedLine)->InPPDirective)
-  (*CompactedLine)->Level -= OutdentBy;
+  (*CompactedLine)->Level -= std::min(OutdentBy, 
(*CompactedLine)->Level);
   }
 }
 return J - 1;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4223,6 +4223,19 @@
"void foo() {}\n"
"} // namespace ns\n",
Style);
+
+  FormatStyle LLVMWithCompactInnerNamespace = getLLVMStyle();
+  LLVMWithCompactInnerNamespace.CompactNamespaces = true;
+  LLVMWithCompactInnerNamespace.NamespaceIndentation = FormatStyle::NI_Inner;
+  verifyNoCrash("namespace ns1 { namespace ns2 { namespace ns3 {\n"
+"void func_in_ns() {\n"
+"  int res{0};\n"
+"// block for debug mode\n"
+"#ifndef NDEBUG\n"
+"#endif\n"
+"}\n"
+"}}}\n",
+LLVMWithCompactInnerNamespace);
 }
 
 TEST_F(FormatTest, NamespaceMacros) {
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -390,7 +390,7 @@
   for (auto *CompactedLine = I + J; CompactedLine <= ClosingLine;
++CompactedLine) {
 if (!(*CompactedLine)->InPPDirective)
-  (*CompactedLine)->Level -= OutdentBy;
+  (*CompactedLine)->Level -= std::min(OutdentBy, (*CompactedLine)->Level);
   }
 }
 return J - 1;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158363: [clang-format] Fix segmentation fault when formatting nested namespaces

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

In D158363#4606159 , @d0nc1h0t wrote:

> In D158363#4604468 , 
> @HazardyKnusperkeks wrote:
>
>> Please upload the patch with the full context.
>
> I'm creating a patch via 'git diff' from the root of the project. What does 
> full context mean?

If you look at your diff here there is `Context not available.`. Please use 
`git diff -U9`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158363

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

[PATCH] D158363: [clang-format] Fix segmentation fault when formatting nested namespaces

[Arkadiy Yudintsev via Phabricator via cfe-commits]

d0nc1h0t added a comment.

In D158363#4604468 , 
@HazardyKnusperkeks wrote:

> Please upload the patch with the full context.

I'm creating a patch via 'git diff' from the root of the project. What does 
full context mean?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158363

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

[PATCH] D158363: [clang-format] Fix segmentation fault when formatting nested namespaces

[Arkadiy Yudintsev via Phabricator via cfe-commits]

d0nc1h0t updated this revision to Diff 552281.
d0nc1h0t added a comment.

Added unittest.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158363

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4223,6 +4223,19 @@
"void foo() {}\n"
"} // namespace ns\n",
Style);
+
+  FormatStyle LLVMWithCompactInnerNamespace = getLLVMStyle();
+  LLVMWithCompactInnerNamespace.CompactNamespaces = true;
+  LLVMWithCompactInnerNamespace.NamespaceIndentation = FormatStyle::NI_Inner;
+  verifyNoCrash("namespace ns1 { namespace ns2 { namespace ns3 {\n"
+"void func_in_ns() {\n"
+"  int res{0};\n"
+"// block for debug mode\n"
+"#ifndef NDEBUG\n"
+"#endif\n"
+"}\n"
+"}}}\n",
+LLVMWithCompactInnerNamespace);
 }
 
 TEST_F(FormatTest, NamespaceMacros) {
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -390,7 +390,7 @@
   for (auto *CompactedLine = I + J; CompactedLine <= ClosingLine;
++CompactedLine) {
 if (!(*CompactedLine)->InPPDirective)
-  (*CompactedLine)->Level -= OutdentBy;
+  (*CompactedLine)->Level -= std::min(OutdentBy, 
(*CompactedLine)->Level);
   }
 }
 return J - 1;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4223,6 +4223,19 @@
"void foo() {}\n"
"} // namespace ns\n",
Style);
+
+  FormatStyle LLVMWithCompactInnerNamespace = getLLVMStyle();
+  LLVMWithCompactInnerNamespace.CompactNamespaces = true;
+  LLVMWithCompactInnerNamespace.NamespaceIndentation = FormatStyle::NI_Inner;
+  verifyNoCrash("namespace ns1 { namespace ns2 { namespace ns3 {\n"
+"void func_in_ns() {\n"
+"  int res{0};\n"
+"// block for debug mode\n"
+"#ifndef NDEBUG\n"
+"#endif\n"
+"}\n"
+"}}}\n",
+LLVMWithCompactInnerNamespace);
 }
 
 TEST_F(FormatTest, NamespaceMacros) {
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -390,7 +390,7 @@
   for (auto *CompactedLine = I + J; CompactedLine <= ClosingLine;
++CompactedLine) {
 if (!(*CompactedLine)->InPPDirective)
-  (*CompactedLine)->Level -= OutdentBy;
+  (*CompactedLine)->Level -= std::min(OutdentBy, (*CompactedLine)->Level);
   }
 }
 return J - 1;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158363: [clang-format] Fix segmentation fault when formatting nested namespaces

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

Please upload the patch with the full context. And add a test case. (From your 
issue.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158363

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

[PATCH] D158363: [clang-format] Fix segmentation fault when formatting nested namespaces

[Arkadiy Yudintsev via Phabricator via cfe-commits]

d0nc1h0t created this revision.
d0nc1h0t added a project: clang-format.
Herald added projects: All, clang.
Herald added a subscriber: cfe-commits.
Herald added reviewers: rymiel, HazardyKnusperkeks, owenpan, MyDeveloperDay.
Herald added a comment.
d0nc1h0t requested review of this revision.

NOTE: Clang-Format Team Automated Review Comment

It looks like your clang-format review does not contain any unit tests, please 
try to ensure all code changes have a unit test (unless this is an `NFC` or 
refactoring, adding documentation etc..)

Add your unit tests in `clang/unittests/Format` and you can build with `ninja 
FormatTests`.  We recommend using the `verifyFormat(xxx)` format of unit tests 
rather than `EXPECT_EQ` as this will ensure you change is tolerant to random 
whitespace changes (see FormatTest.cpp as an example)

For situations where your change is altering the TokenAnnotator.cpp which can 
happen if you are trying to improve the annotation phase to ensure we are 
correctly identifying the type of a token, please add a token annotator test in 
`TokenAnnotatorTest.cpp`


Fixes #64701 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158363

Files:
  clang/lib/Format/UnwrappedLineFormatter.cpp


Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -390,7 +390,7 @@
   for (auto *CompactedLine = I + J; CompactedLine <= ClosingLine;
++CompactedLine) {
 if (!(*CompactedLine)->InPPDirective)
-  (*CompactedLine)->Level -= OutdentBy;
+  (*CompactedLine)->Level -= std::min(OutdentBy, 
(*CompactedLine)->Level);
   }
 }
 return J - 1;


Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -390,7 +390,7 @@
   for (auto *CompactedLine = I + J; CompactedLine <= ClosingLine;
++CompactedLine) {
 if (!(*CompactedLine)->InPPDirective)
-  (*CompactedLine)->Level -= OutdentBy;
+  (*CompactedLine)->Level -= std::min(OutdentBy, (*CompactedLine)->Level);
   }
 }
 return J - 1;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits