owenpan added inline comments.
================
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;
+ }
----------------
Sedeniono wrote:
> 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?
> 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.
I've updated the summary and will include the link in the commit message.
> 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?
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.
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
