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

[d0nc1h0t]

Summary

Fixing the clang-format crash with the segmentation fault error when formatting code with nested namespaces.

Fixes: #64701

[rymiel]

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

[HazardyKnusperkeks]

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

[d0nc1h0t]

Added unittest.

[d0nc1h0t]

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?

[HazardyKnusperkeks]

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 -U99999.

[d0nc1h0t]

Updated the patch with full context.

[owenpan]

clang/lib/Format/UnwrappedLineFormatter.cpp

389–390

See below.

392–396

Or similar.

clang/unittests/Format/FormatTest.cpp

4187–4194

It seems that we can reduce the test case.

[owenpan]

It seems that you forgot to update the diff?

[owenpan]

clang/lib/Format/UnwrappedLineFormatter.cpp

389–390

I forgot to suggest const.

clang/unittests/Format/FormatTest.cpp

4188–4191

Did you run git-clang-format?

[d0nc1h0t]

Added missing const + clang-format.

[d0nc1h0t]

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

[owenpan]

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

[d0nc1h0t]

Rebase

[d0nc1h0t]

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?

[owenpan]

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

[d0nc1h0t]

In D158363#4634444, @owenpan wrote:

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

Arkadiy Yudintsev <[e-mail]>

clang/lib/Format/UnwrappedLineFormatter.cpp

if (Style.CompactNamespaces) {
              ClosingLineIndex == I[J]->MatchingClosingBlockLineIndex &&
              I[J]->Last->TotalLength < Limit;
              ++J, --ClosingLineIndex) {
           Limit -= I[J]->Last->TotalLength;
 
           // 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;
       }
 
       if (auto nsToken = getMatchingNamespaceToken(TheLine, AnnotatedLines)) {
         int i = 0;
         unsigned openingLine = TheLine->MatchingOpeningBlockLineIndex - 1;

clang/unittests/Format/FormatTest.cpp

TEST_F(FormatTest, FormatsNamespaces) {
   verifyFormat("template <int I>\n"
                "constexpr void foo()\n"
                "  requires(I == 42)\n"
                "{}\n"
                "namespace ns {\n"
                "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) {
   FormatStyle Style = getLLVMStyle();
   Style.NamespaceMacros.push_back("TESTSUITE");
 
   verifyFormat("TESTSUITE(A) {\n"
                "int foo();\n"
                "} // TESTSUITE(A)",
                Style);