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

[Sedenion via Phabricator via cfe-commits]

Sedeniono requested changes to this revision.
Sedeniono added inline comments.
This revision now requires changes to proceed.


================
Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:1416-1421
+      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;
+      }
----------------
After some time, I found your comment [here](https://reviews.llvm.org/D150057#inline-1449546) 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](https://reviews.llvm.org/D150057#inline-1449546) 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](https://reviews.llvm.org/D150057#inline-1449546), 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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153243