[clang-format] Don't finalize #if, #else, #endif, etc.

[owenpan]

Summary

Don't finalize a preprocessor branch directive if it's the first token of an annotated line. See the rationale at D150057#inline-1449546.

Fixes https://github.com/llvm/llvm-project/issues/63379.

[Sedeniono]

clang/lib/Format/UnwrappedLineFormatter.cpp

1416–1421

After some time, I found your comment here which explains the intention of the "do not finalize '#'" thing. Would it be possible to provide the reasoning here as some comment, or at least some hint that it has to do with multiple passes because of PP branches? To give future readers some chance to understand what is going on. Maybe something along the lines of

// Never mark the '#' of a PP branch as finalized, since clang-format performs 
// multiple passes, where each pass represents a version where the code of 
// one of the PP branches is filled in. Later passes must also  be allowed to 
// format the PP tokens.

Alternatively or additionally, I would be happy with a link to your explanation in the code or the commit message, if the llvm rules/conventions allow it.

Apart from this, I have dug way too little into how clang-format works to reason about the change. I tested it with the full hpp file from https://github.com/llvm/llvm-project/issues/57117, and can confirm that the issue is fixed. So I believe you. ;-)
But: Considering your comment here, I thought that the whole "do not finalize '#'" thing is relevant only if there are some "else"-PP-branches? But in the minimal example/the unit test, there is no "else" branch? How do the multiple passes because of PP branches relate to the clang-format off comment?

[owenpan]

clang/lib/Format/UnwrappedLineFormatter.cpp

1416–1421

Alternatively or additionally, I would be happy with a link to your explanation in the code or the commit message, if the llvm rules/conventions allow it.

I've updated the summary and will include the link in the commit message.

But: Considering your comment here, I thought that the whole "do not finalize '#'" thing is relevant only if there are some "else"-PP-branches? But in the minimal example/the unit test, there is no "else" branch?

This patch fixes the issue that #if/#else etc. may get formatted even if clang-format off is specified. It's not specific to #else, and the added unit test is sufficient.

How do the multiple passes because of PP branches relate to the clang-format off comment?

They are not (or should not be) related. We probably should add an attribute to FormatToken exclusively for clang-format off as Finalized doesn't necessarily mean clang-format off is in effect.

clang/lib/Format/UnwrappedLineFormatter.cpp

if (ShouldFormat && TheLine.Type != LT_Invalid) {
 
         // Notify the WhitespaceManager about the unchanged whitespace.
         for (FormatToken *Tok = TheLine.First->Next; Tok; Tok = Tok->Next)
           Whitespaces->addUntouchableToken(*Tok, TheLine.InPPDirective);
       }
       NextLine = Joiner.getNextMergedLine(DryRun, IndentTracker);
       RangeMinLevel = UINT_MAX;
     }
-    if (!DryRun)
-      markFinalized(TheLine.First);
+    if (!DryRun) {
+      auto *Tok = TheLine.First;
+      if (Tok->is(tok::hash) && !Tok->Previous && Tok->Next &&
+          Tok->Next->isOneOf(tok::pp_if, tok::pp_ifdef, tok::pp_ifndef,
+                             tok::pp_elif, tok::pp_elifdef, tok::pp_elifndef,
+                             tok::pp_else, tok::pp_endif)) {
+        Tok = Tok->Next;
+      }
+      markFinalized(Tok);
+    }
   }
   PenaltyCache[CacheKey] = Penalty;
   return Penalty;
 }
 
 static auto computeNewlines(const AnnotatedLine &Line,
                             const AnnotatedLine *PreviousLine,
                             const AnnotatedLine *PrevPrevLine,

clang/lib/Format/WhitespaceManager.cpp

: Tok(&Tok), CreateReplacement(CreateReplacement),
       PreviousEndOfTokenColumn(0), EscapedNewlineColumn(0),
       StartOfBlockComment(nullptr), IndentationOffset(0), ConditionalsLevel(0) {
 }
 
 void WhitespaceManager::replaceWhitespace(FormatToken &Tok, unsigned Newlines,
                                           unsigned Spaces,
                                           unsigned StartOfTokenColumn,
                                           bool IsAligned, bool InPPDirective) {
-  auto PPBranchDirectiveStartsLine = [&Tok] {
-    return Tok.is(tok::hash) && !Tok.Previous && Tok.Next &&
-           Tok.Next->isOneOf(tok::pp_if, tok::pp_ifdef, tok::pp_ifndef,
-                             tok::pp_elif, tok::pp_elifdef, tok::pp_elifndef,
-                             tok::pp_else, tok::pp_endif);
-  };
-  if ((Tok.Finalized && !PPBranchDirectiveStartsLine()) ||
-      (Tok.MacroCtx && Tok.MacroCtx->Role == MR_ExpandedArg)) {
+  if (Tok.Finalized || (Tok.MacroCtx && Tok.MacroCtx->Role == MR_ExpandedArg))
     return;
-  }
   Tok.setDecision((Newlines > 0) ? FD_Break : FD_Continue);
   Changes.push_back(Change(Tok, /*CreateReplacement=*/true, Tok.WhitespaceRange,
                            Spaces, StartOfTokenColumn, Newlines, "", "",
                            IsAligned, InPPDirective && !Tok.IsFirst,
                            /*IsInsideToken=*/false));
 }
 
 void WhitespaceManager::addUntouchableToken(const FormatToken &Tok,

clang/unittests/Format/FormatTest.cpp

verifyFormat("int *i;\n"
                "int* j;\n"
                "// clang-format only\n"
                "int* k;",
                "int* i;\n"
                "// clang-format off:0\n"
                "int* j;\n"
                "// clang-format only\n"
                "int* k;");
+
+  verifyNoChange("// clang-format off\n"
+                 "#if 0\n"
+                 "        #if SHOULD_STAY_INDENTED\n"
+                 " #endif\n"
+                 "#endif\n"
+                 "// clang-format on");
 }
 
 TEST_F(FormatTest, DoNotCrashOnInvalidInput) {
   format("? ) =");
   verifyNoCrash("#define a\\\n /**/}");
 }
 
 TEST_F(FormatTest, FormatsTableGenCode) {