[PATCH] Calculate and store ColumnWidth instead of CodePointCount	in FormatTokens.
    Alexander Kornienko 
    alexfh at google.com
       
    Mon Sep  9 09:19:54 PDT 2013
    
    
  
  PTAL
================
Comment at: lib/Format/Format.cpp:696
@@ +695,3 @@
+    // FIXME: ColumnWidth actually depends on the start column, we need to take
+    // this to account when the token is moved.
+    FormatTok->ColumnWidth =
----------------
Daniel Jasper wrote:
> nit: .. into account ..
Done.
================
Comment at: lib/Format/Format.cpp:687
@@ +686,3 @@
+    if (FirstNewlinePos != StringRef::npos) {
+      // LastLineColumnWidth is always correct, as it always starts in the
+      // beginning of the line.
----------------
Daniel Jasper wrote:
> This comment is hard to understand. How about:
>  
>   // The last line of this token always starts in column 0.
>   // Thus, the length can be precomputed even in the presence of tabs.
Yep, though not perfect, this explain things better than the original comment.
================
Comment at: lib/Format/Format.cpp:962
@@ +961,3 @@
+    // Never try to merge lines with multiline tokens.
+    for (SmallVectorImpl<AnnotatedLine *>::iterator It = I; It != E; ++It) {
+      for (const FormatToken *Tok = (*It)->First; Tok; Tok = Tok->Next) {
----------------
Daniel Jasper wrote:
> This should not be necessary, as TotalLength will be set to something > TotalLength for multiline tokens. Can you create a test case that fails without this? If so, why?
I've added the check after a few tests started failing.
Without this check, this code:
void f() {
/* qqqqqqq
qqqqqqqq */
}
gets merged to a "single" line:
void f() { /* qqqqqqq
qqqqqqqq */ }
(the second line of the comment probably also gets indented, but I'm not sure).
================
Comment at: lib/Format/FormatToken.h:124
@@ -125,1 +123,3 @@
+  /// no newline in the token.
+  enum { NoNewline = UINT_MAX };
 
----------------
Daniel Jasper wrote:
> How about just using the LastLineColumnWidth 0 for this? I think that would be quite self-explanatory as the last line can never actually have the length 0.
It can, if a line comment ends with an escaped newline (there's now a test for this, btw).
OTOH LastLineColumnWidth is currently only used for string literals, so we could use an arbitrary non-zero value for multi-line line comments. We could also store the column + 1, and 0 would be really a dedicated value for the single line case. But from all these options I prefer the one currently implemented.
http://llvm-reviews.chandlerc.com/D1608
    
    
More information about the cfe-commits
mailing list