djasper added inline comments.
================
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;
----------------
euhlmann wrote:
> djasper wrote:
> > 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).
> I apologize, the style option documentation was a typo. The patch summary has
> the intended behavior, and I mentioned there that I count the hash as a
> column. Part of the reasoning for this is because it would look the same
> visually with spaces or tabs.
Do you know of a coding style that writes something about this? I think the
similarity of spaces vs. tabs is not a strong reason because a source file will
either use one or the other. To me:
#if a == 1
# define X
# if b == 2
# define Y
...
Would look weird. I'd prefer if we kept this simpler and more consistent.
================
Comment at: lib/Format/UnwrappedLineParser.cpp:701
+ PPMaybeIncludeGuard->TokenText == FormatTok->TokenText &&
+ PPIndentLevel > 0) {
+ --PPIndentLevel;
----------------
euhlmann wrote:
> djasper wrote:
> > 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
> It sounds like these would require two passes, since we wouldn't know until
> the end whether an include guard spans the whole file or has a #else. Do we
> really want two passes? Or do you have any advice on how these tests can be
> implemented?
You could store an additional bool that gets set to true if the possible
include guard is matched up with an appropriate #endif in the last line of a
file. If that bool is true, you can remove 1 from the level of all preprocessor
lines.
https://reviews.llvm.org/D35955
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
