[PATCH] D152804: [clang-format] Propose a new solution to - Fix overlapping replacements before PPDirectives

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

I got something that looks promising:

  diff --git a/clang/lib/Format/FormatToken.h b/clang/lib/Format/FormatToken.h
  index dd23bd35411d..bba030238338 100644
  --- a/clang/lib/Format/FormatToken.h
  +++ b/clang/lib/Format/FormatToken.h
  @@ -418,6 +418,12 @@ public:
     /// and thereby e.g. leave an empty line between two function definitions.
     unsigned NewlinesBefore = 0;
  
  +  /// The number of newlines immediately before the \c Token after formatting.
  +  ///
  +  /// This is used to avoid overlapping whitespace replacements when \c Newlines
  +  /// is recomputed for a finalized preprocessor branching directive.
  +  int Newlines = -1;
  +
     /// The offset just past the last '\n' in this token's leading
     /// whitespace (relative to \c WhiteSpaceStart). 0 if there is no '\n'.
     unsigned LastNewlineOffset = 0;
  diff --git a/clang/lib/Format/UnwrappedLineFormatter.cpp b/clang/lib/Format/UnwrappedLineFormatter.cpp
  index 5172deb494b2..72293945122b 100644
  --- a/clang/lib/Format/UnwrappedLineFormatter.cpp
  +++ b/clang/lib/Format/UnwrappedLineFormatter.cpp
  @@ -1418,22 +1418,13 @@ unsigned UnwrappedLineFormatter::format(
     return Penalty;
   }
  
  -void UnwrappedLineFormatter::formatFirstToken(
  -    const AnnotatedLine &Line, const AnnotatedLine *PreviousLine,
  -    const AnnotatedLine *PrevPrevLine,
  -    const SmallVectorImpl<AnnotatedLine *> &Lines, unsigned Indent,
  -    unsigned NewlineIndent) {
  -  FormatToken &RootToken = *Line.First;
  -  if (RootToken.is(tok::eof)) {
  -    unsigned Newlines =
  -        std::min(RootToken.NewlinesBefore,
  -                 Style.KeepEmptyLinesAtEOF ? Style.MaxEmptyLinesToKeep + 1 : 1);
  -    unsigned TokenIndent = Newlines ? NewlineIndent : 0;
  -    Whitespaces->replaceWhitespace(RootToken, Newlines, TokenIndent,
  -                                   TokenIndent);
  -    return;
  -  }
  -  unsigned Newlines =
  +static auto computeNewlines(const AnnotatedLine &Line,
  +                            const AnnotatedLine *PreviousLine,
  +                            const AnnotatedLine *PrevPrevLine,
  +                            const SmallVectorImpl<AnnotatedLine *> &Lines,
  +                            const FormatStyle &Style) {
  +  const auto &RootToken = *Line.First;
  +  auto Newlines =
         std::min(RootToken.NewlinesBefore, Style.MaxEmptyLinesToKeep + 1);
     // Remove empty lines before "}" where applicable.
     if (RootToken.is(tok::r_brace) &&
  @@ -1512,7 +1503,32 @@ void UnwrappedLineFormatter::formatFirstToken(
       }
     }
  
  -  if (Newlines)
  +  return Newlines;
  +}
  +
  +void UnwrappedLineFormatter::formatFirstToken(
  +    const AnnotatedLine &Line, const AnnotatedLine *PreviousLine,
  +    const AnnotatedLine *PrevPrevLine,
  +    const SmallVectorImpl<AnnotatedLine *> &Lines, unsigned Indent,
  +    unsigned NewlineIndent) {
  +  FormatToken &RootToken = *Line.First;
  +  if (RootToken.is(tok::eof)) {
  +    unsigned Newlines =
  +        std::min(RootToken.NewlinesBefore,
  +                 Style.KeepEmptyLinesAtEOF ? Style.MaxEmptyLinesToKeep + 1 : 1);
  +    unsigned TokenIndent = Newlines ? NewlineIndent : 0;
  +    Whitespaces->replaceWhitespace(RootToken, Newlines, TokenIndent,
  +                                   TokenIndent);
  +    return;
  +  }
  +
  +  if (RootToken.Newlines < 0) {
  +    RootToken.Newlines = computeNewlines(Line, PreviousLine, PrevPrevLine,
  +                                         Lines, Style);
  +  }
  +
  +  assert(RootToken.Newlines >= 0);
  +  if (RootToken.Newlines > 0)
       Indent = NewlineIndent;
  
     // Preprocessor directives get indented before the hash only if specified. In
  @@ -1524,7 +1540,7 @@ void UnwrappedLineFormatter::formatFirstToken(
       Indent = 0;
     }
  
  -  Whitespaces->replaceWhitespace(RootToken, Newlines, Indent, Indent,
  +  Whitespaces->replaceWhitespace(RootToken, RootToken.Newlines, Indent, Indent,
                                    /*IsAligned=*/false,
                                    Line.InPPDirective &&
                                        !RootToken.HasUnescapedNewline);

I'll submit a patch based on the above in a day or two.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152804