[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

Please mark review comments as done if you have addressed them. Can you also clean up the test cases, removing overlapping/redundant ones, making sure a test case doesn't end with a newline (e.g., line 5380), etc?



================
Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:69-75
+      if (Line.InMacroBody) {
+        Indent = (Line.PPLevel + 1) * PPIndentWidth;
+        Indent += (Line.Level - Line.PPLevel - 1) * Style.IndentWidth;
+        Indent += Style.IndentWidth - PPIndentWidth;
+      } else {
+        Indent = Line.Level * PPIndentWidth;
+      }
----------------
To make it simpler and clearer.


================
Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:63
     skipLine(Line, /*UnknownIndent=*/true);
-    if (Line.InPPDirective ||
-        (Style.IndentPPDirectives == FormatStyle::PPDIS_BeforeHash &&
-         Line.Type == LT_CommentAbovePPDirective)) {
-      unsigned IndentWidth =
+    if (Style.IndentPPDirectives != FormatStyle::PPDIS_None &&
+        (Line.InPPDirective ||
----------------
owenpan wrote:
> You don't need to add this condition.
Please ignore my comment above.


================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1280
+
+  if (Style.IndentPPDirectives != FormatStyle::PPDIS_None)
+    Line->PPLevel = PPBranchLevel + (IncludeGuard == IG_Defined ? 0 : 1);
----------------
We don't need the `if` statement anymore. Actually, it's better to always set `PPLevel` for `InMacroBody` lines.


================
Comment at: clang/lib/Format/UnwrappedLineParser.h:47-65
+  /// The nested preprocessor #if* level of the \c UnwrappedLine. This does not
+  /// include header-guards.
+  /// For example:
+  /// #ifndef foobar_h      PPLevel still at : 0 (because header guard)
+  /// #define foobar_h      PPLevel still at : 0
+  /// #if A                 PPLevel from     : 0 -> 1
+  /// if (abc) {            PPLevel still at : 1 (not preprocessor indent)
----------------
We probably don't need the (outdated) examples as we can print `PPLevel` in `TokenAnnotator::printDebugInfo` now.


================
Comment at: clang/unittests/Format/FormatTest.cpp:5382-5384
+
+  style.IndentWidth = 4;
+  style.PPIndentWidth = 1;
----------------
Please delete.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137181