djasper added inline comments.
================
Comment at: docs/ClangFormatStyleOptions.rst:1182
+**IndentPPDirectives** (``bool``)
+ Indent preprocessor directives on conditionals.
----------------
I think we can foresee that a bool is not going to be enough here. Make this an
enum, which for no can contain the values "no" and "afterhash" and in the long
run can get a "beforehash" value.
================
Comment at: lib/Format/ContinuationIndenter.cpp:379
+ // indent preprocessor directives after the hash if required.
+ if ((State.Line->Type == LT_PreprocessorDirective ||
----------------
Start comments upper case.
================
Comment at: lib/Format/ContinuationIndenter.cpp:383
+ Current.Previous->is(tok::hash) && State.FirstIndent > 0) {
+ // subtract 1 so indent lines up with non-preprocessor code
+ Spaces += State.FirstIndent;
----------------
Same here and use full sentences.
Also, this does not seem to be what the example in the style option suggests
and I'd rather not do it (subtracting 1).
================
Comment at: lib/Format/UnwrappedLineFormatter.cpp:1021
+ // Preprocessor directives get indented after the hash.
+ if (Line.Type == LT_PreprocessorDirective ||
----------------
... or not at all. :)
================
Comment at: lib/Format/UnwrappedLineParser.cpp:667
parsePPUnknown();
+ if (IfNDef) {
+ PPMaybeIncludeGuard = IfCondition;
----------------
LLVM style does not use braces for single statement ifs.
================
Comment at: lib/Format/UnwrappedLineParser.cpp:699
}
+ if (PPMaybeIncludeGuard != nullptr &&
+ PPMaybeIncludeGuard->TokenText == FormatTok->TokenText &&
----------------
In LLVM's codebase, we mostly just leave out the "!= nullptr" and rely on the
implicit conversion to bool.
================
Comment at: lib/Format/UnwrappedLineParser.cpp:701
+ PPMaybeIncludeGuard->TokenText == FormatTok->TokenText &&
+ PPIndentLevel > 0) {
+ --PPIndentLevel;
----------------
I think you'll need substantially more tests here:
- An include guard must not have a #else
- An include guard must span the entire file
================
Comment at: lib/Format/UnwrappedLineParser.h:238
+ unsigned PPIndentLevel;
+ FormatToken *PPMaybeIncludeGuard;
----------------
I think this should almost always be PPBranchLevel. Probably the different case
is the #else itself, but I think we can handle that easily.
https://reviews.llvm.org/D35955
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
