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

[Noah Goldstein via Phabricator via cfe-commits]

goldstein.w.n added inline comments.


================
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;
+      }
----------------
owenpan wrote:
> To make it simpler and clearer.
Done.


================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:838
+  // If this changes PPLevel needs to be used for get correct indentation.
+  assert(!(Line.InMacroBody && InPPDirective));
   return Line.Level * Style.IndentWidth + Length <= ColumnLimit;
----------------
owenpan wrote:
> Shouldn't it be `assert(!Line.InMacroBody && !InPPDirective)` instead? You can also assert `Line.PPLevel == 0`.
I guess the goal was to assert condition where `PPLevel` would be used to calculate if a line might find (if both `InMacroBody` and `InPPDirective`) are true. But your assert should also works fine so fixed.


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


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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137181