[PATCH] D35955: clang-format: Add preprocessor directive indentation

[Erik Uhlmann via Phabricator via cfe-commits]

euhlmann added inline comments.


================
Comment at: lib/Format/ContinuationIndenter.cpp:387
+    // hash. This causes second-level indents onward to have an extra space
+    // after the tabs. We set the state to column 0 to avoid this misalignment.
+    if (Style.UseTab != FormatStyle::UT_Never)
----------------
krasimir wrote:
> I don't understand this comment. Could you please give an example?
When tabs are enabled, extra spaces would be added because of the column offset caused by the `#` token.
```
#if FOO
#<tab>if BAR
#<tab><space>define BAZ
#<tab>endif
#endif
```
Let me know if there's a better way to fix that.


================
Comment at: lib/Format/UnwrappedLineParser.cpp:732
+  // then we count it as a real include guard and subtract one from every
+  // preprocessor indent.
+  unsigned TokenPosition = Tokens->getPosition();
----------------
krasimir wrote:
> Why do we need to subtract one from every preprocessor indent?
This only knows for sure when something is an include guard at the end of the file. By that time, everything inside the include guard has already been indented one extra level since we didn't know about the guard yet, so it would look like
```
#ifndef FILE_H
#  define FILE_H
// ...
#  other directives
// ...
#endif
```
Subtracting one is the action taken when we decide that there's a valid include guard, so it looks like
```
#ifndef FILE_H
#define FILE_H
// ...
#other directives
// ...
#endif
```


================
Comment at: lib/Format/UnwrappedLineParser.cpp:760
+  }
+  PPMaybeIncludeGuard = nullptr;
   nextToken();
----------------
krasimir wrote:
> Why do we reset `PPMaybeIncludeGuard` here?
The idea is if there's any preprocessor lines between the #ifndef and the #define, it doesn't treat it as an include guard. This is the same reason for the other PPMaybeIncludeGuard reset.


================
Comment at: lib/Format/UnwrappedLineParser.cpp:770
   addUnwrappedLine();
-  Line->Level = 1;
+  ++Line->Level;
 
----------------
krasimir wrote:
> Why do we `++Line->Level` here?
This sets correct indent for multi-line macros, so they are indented one level more than the indent of the #define instead of always at level 1.


https://reviews.llvm.org/D35955