[clang-format] Adding style option for absolute formatting

[acoomans]

Summary

When formatting only certain lines for a file (clang-format -lines=x:x), two types of indentation could be expected:

• absolute: the indentation as if the whole file was formatted
• relative: the indentation is relative to the previous line, even if those are not indented correctly and are not in the range of lines to format

The default behavior for clang-format is relative formatting; e.g: running clang-format -lines=3:3 on (note the extra white space in front of @optional):

@protocol A
 @optional
// comment
- (void)f;
@end
MACRO

would give:

@protocol A
 @optional
 // comment
 - (void)f;
 @end
 MACRO

This diff adds a style option to specify absolute indentation; continuing the example above, with the option set, running clang-format -lines=3:3 would give:

@protocol A
 @optional
// comment
- (void)f;
@end
MACRO

Initial bug report / feature request: https://bugs.llvm.org/show_bug.cgi?id=37975

[acoomans]

An alternative to changing the whole behavior would be to make it a parameter / configuration option.

[djasper]

Could you explain what problem this is fixing?

[acoomans]

@djasper I updated the description of the diff.

This fixes the issue of clang-format -lines=x:x not returning the same results as clang-format, while keeping the current behavior as default.

[acoomans]

Also ping @jolesiak since he initially filed the report

[djasper]

In my opinion, this only addresses one edge case where clang-format -lines output is not identical with a full reformatting. I believe they cannot all usefully be avoided. As such, I am unsure that this option carries its weight of making the implementation more complex.
How often does this happen for you? Why can't you just format the additional incorrect line?

[acoomans]

I don't know; I just picked a random bug from the Bugzilla to get myself familiarized with the LLVM codebase :)

@jolesiak?

[jolesiak]

Sorry for the delay, I'll comment on that tomorrow.

[jolesiak]

First of all, thanks, Arnaud, for looking into this.

Let's address the mentioned bug first.
Let's assume that we use IndentReference = Relative. I think that this particular formatting bug is local (i.e. can be solved while processing a @protocol block). Look at this example:

if (true) {
  int a = 42;
    int b = 42;
  int c = 42;
}
int d = 42;

When we run clang-format -lines=4:4 we get:

if (true) {
  int a = 42;
    int b = 42;
    int c = 42;
}
int d = 42;

Please note that neither } nor int d = 42 is indented.
But this is a different behavior from what we see after running clang-format -lines=3:3 on:

@protocol A
 @optional
// comment
- (void)f;
@end
MACRO

the output is:

@protocol A
 @optional
 // comment
 - (void)f;
 @end
 MACRO

Please note that @end is indented (has different indentation than @protocol; MACRO is indented as well).
To clarify why this is undesired, @end ends @protocol, not @optional. By the way, clang-format doesn't think that @end should match @optional, check the output of an extended example:

@protocol A
 @optional
 // comment
 - (void)f;
 @end
 MACRO
 @end

Second @end still doesn't match the @protocol.

I think it's perfectly fine to allow users to have custom indentations if they so choose. In this particular example, though, clang-format should stop indenting when it reaches @end, giving the following output:

@protocol A
 @optional
 // comment
 - (void)f;
@end
MACRO

Moving to the general case, I must say that introducing IndentReference = Relative/Absolute is a very interesting approach. I can imagine situations when somebody actually want to use IndentReference = Absolute. However, adding an additional option comes at quite big cost and I think that in this case it outweighs the benefits.
I think that a very good outcome could be to comment what is happening next to IndentTracker.adjustToUnmodifiedLine(TheLine); line. E.g. "We adjust an indentation to match the existing code to give users flexibility (instead of ignoring it). It's their responsibility to provide a correct formatting of lines they didn't initially change if these lines break formatting.".

[djasper]

Ok, so IIUC, understanding that @end effective ends a section much like "}" would address the currently observed problems?

[jolesiak]

In D49580#1179924, @djasper wrote:

Ok, so IIUC, understanding that @end effective ends a section much like "}" would address the currently observed problems?

I think so.

[acoomans]

Thanks for the review. I’ll update when I’m back from vacation

Arnaud

[jolesiak]

Thanks!

[acoomans]

Changed approach based on feedback. New changeset: https://reviews.llvm.org/D50535

lib/Format/UnwrappedLineFormatter.cpp

if (ShouldFormat && TheLine.Type != LT_Invalid) {
         for (const FormatToken *Tok = TheLine.First; Tok; Tok = Tok->Next)
           if (!Tok->Children.empty())
             format(Tok->Children, DryRun);
 
       // Adapt following lines on the current indent level to the same level
       // unless the current \c AnnotatedLine is not at the beginning of a line.
       bool StartsNewLine =
           TheLine.First->NewlinesBefore > 0 || TheLine.First->IsFirst;
-      if (StartsNewLine)
+      if (TheLine.Affected && StartsNewLine)
         IndentTracker.adjustToUnmodifiedLine(TheLine);
       if (!DryRun) {
         bool ReformatLeadingWhitespace =
             StartsNewLine && ((PreviousLine && PreviousLine->Affected) ||
                               TheLine.LeadingEmptyLinesAffected);
         // Format the first token.
         if (ReformatLeadingWhitespace)
           formatFirstToken(TheLine, PreviousLine, Lines,

test/Format/line-ranges-indent.cpp

+// RUN: grep -Ev "// *[A-Z-]+:" %s \
+// RUN:   | clang-format -style=LLVM -lines=3:3 \
+// RUN:   | FileCheck -strict-whitespace %s
+// CHECK: {{^\@protocol\ A$}}
+@protocol A
+// CHECK: {{^\ \@optional$}}
+ @optional
+// CHECK: {{^//\ comment$}}
+// comment
+// CHECK: {{^-\ \(void\)f;$}}
+- (void)f;
+// CHECK: {{^\@end$}}
+@end