r195954 - clang-format: Extends formatted ranges to subsequent lines comments.

[Alp Toker]

On 02/12/2013 09:24, Daniel Jasper wrote:
> Fixed in r196080.

Thanks!

This avoids unrelated whitespace edits following the change but there's 
still a problem with preceding unrelated lines getting modified. Not 
sure if this is a regression or pre-existing problem.

With the attached format.cpp input, requesting formatting of line range 
3 causes lines 1 and 2 to get edited:

|clang-format -lines=3:3 format.cpp | diff -U format.cpp -||
||Can't find usable .clang-format, using LLVM style||
||--- format.cpp    2013-12-05 09:51:34.000000000 +0000||
||+++ -    2013-12-05 09:52:43.000000000 +0000||
||@@ -1,5 +1,5 @@||
||-void f() { ||
||- ||
||+void f() {||
||+||
||   // Get the solitary sucessor.||
||   const CFGBlock *Succ = *(Entry->succ_begin());||
|| }|

AFAICT the problem only exists with comment lines. Requesting line 4 
doesn't trigger it. Noticed while preparing the comment typo fix patches.

Alp.


>
>
> On Mon, Dec 2, 2013 at 9:52 AM, Alp Toker <alp at nuanti.com 
> <mailto:alp at nuanti.com>> wrote:
>
>
>     On 02/12/2013 08:24, Daniel Jasper wrote:
>>     I just tried reproducing this and it still works fine here (both
>>     in vim and in emacs). Are you sure that clang-format removes that
>>     trailing whitespace?
>
>     Hi Daniel,
>
>     I think Rafael is onto something.
>
>     If you take a look at r196038 in clang-tools-extra, there's an
>     unrelated whitespace fix that slipped into the commit yesterday:
>
>
>     |@@ -262,7 +262,7 @@ StatementMatcher makePseudoArrayLoopMatcher() {||
>     ||   // Test that the incoming type has a record declaration that
>     has methods||
>     ||   // called 'begin' and 'end'. If the incoming type is const,
>     then make sure||
>     ||   // these methods are also marked const.||
>     ||-  // ||
>     ||+  //||
>     ||   // FIXME: To be completely thorough this matcher should also
>     ensure the||
>     ||   // return type of begin/end is an iterator that dereferences
>     to the same as||
>     ||   // what operator[] or at() returns. Such a test isn't likely
>     to fail except|
>
>     The line that was legitimately changed is a fair bit above so I
>     wouldn't have expected to see this edit to an untouched line.
>
>     I can try re-creating the patch as it was before running
>     clang-format if it'd be any help.
>
>     diff -U0 | clang-format-diff.py -p1 -i
>
>     Alp.
>
>
>>
>>
>>     On Sun, Dec 1, 2013 at 7:01 PM, Rafael Espíndola
>>     <rafael.espindola at gmail.com <mailto:rafael.espindola at gmail.com>>
>>     wrote:
>>
>>         This seems to have caused a regression with at least the emacs
>>         integration. Running clang-format-region now seems to remove
>>         trailing
>>         white space from unrelated areas. For example, run it with
>>         the cursor
>>         in Sema.cpp:1112:
>>
>>           if (!LangOpts.RetainCommentsFromSystemHeaders &&
>>
>>         and the trailing white space in Sema.cpp:1100:
>>
>>         // We have a generic lambda if we parsed auto parameters, or
>>         we have
>>
>>         is removed.
>>
>>         Cheers,
>>         Rafael
>>
>>
>>
>>
>>     _______________________________________________
>>     cfe-commits mailing list
>>     cfe-commits at cs.uiuc.edu  <mailto:cfe-commits at cs.uiuc.edu>
>>     http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>     -- 
>     http://www.nuanti.com
>     the browser experts
>
>

-- 
http://www.nuanti.com
the browser experts

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131205/a0a8c479/attachment.html>
-------------- next part --------------
void f() {  
  
  // Get the solitary sucessor.
  const CFGBlock *Succ = *(Entry->succ_begin());
}