[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:92-94
+  if (Line.Level >= IndentForLevel.size())
+    IndentForLevel.resize(Line.Level + 1, UnknownIndent ? -1 : Indent);
+}
----------------
owenpan wrote:
> Please run git-clang-format.
Will do for next submissions (thought `arc` did that automatically on precommit).


================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:112
       : Line(Line), TokenSource(TokenSource), ResetToken(ResetToken),
-        PreviousLineLevel(Line.Level), PreviousTokenSource(TokenSource),
-        Token(nullptr), PreviousToken(nullptr) {
+        PreviousLineLevel(Line.Level), PreviousLinePPLevel(Line.PPLevel),
+        PreviousTokenSource(TokenSource), Token(nullptr),
----------------
owenpan wrote:
> Do you need to add `PreviousLinePPLevel` here? If yes, can you add test cases for it? Otherwise, you don't need to make any changes to `ScopedMacroState`.
It's needed.

Without:
```
Expected equality of these values:
  Expected.str()
    Which is: "#ifndef foo\n#define foo\nif (emacs) {\n#ifdef is\n #define lit           \\\n     if (af) {         \\\n         return duh(); \\\n     }\n#endif\n}\n#endif"
  format(test::messUp(Code), ObjCStyle)
    Which is: "#ifndef foo\n#define foo\nif (emacs) {\n#ifdef is\n #define lit        \\\n  if (af) {         \\\n      return duh(); \\\n  }\n#endif\n}\n#endif"
With diff:
@@ -3,8 +3,8 @@
 if (emacs) {
 #ifdef is
- #define lit           \\
-     if (af) {         \\
-         return duh(); \\
-     }
+ #define lit        \\
+  if (af) {         \\
+      return duh(); \\
+  }
 #endif
 }
```


================
Comment at: clang/lib/Format/UnwrappedLineParser.h:66
+  /// #endif                PPLevel still at : 0
+  int PPLevel;
+
----------------
owenpan wrote:
> You need to initialize `PPLevel` in the constructor. FWIW I'd use `unsigned` to be consistent with `Level` above.
Will initialize. It's `int` because we do a less than zero check in the Formatter.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137181