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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
