This is an archive of the discontinued LLVM Phabricator instance.

[clang-format] Fix cleanup of `AnnotatedLine` to include children nodes.
ClosedPublic

Authored by ymandel on Sep 17 2019, 7:14 AM.

Details

Summary

AnnotatedLine has a tree structure, and things like the body of a lambda will be
a child of the lambda expression. For example,

[&]() { foo(a); };

will have an AnnotatedLine with a child:

[&]() {};
 '- foo(a);

Currently, when the Cleaner class analyzes the affected lines, it does not
cleanup the lines' children nodes, which results in missed cleanup
opportunities, like the lambda body in the example above.

This revision extends the algorithm to visit children, thereby fixing the above problem.

Patch by Eric Li.

Event Timeline

ymandel created this revision.Sep 17 2019, 7:14 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 17 2019, 7:14 AM
krasimir accepted this revision.Sep 17 2019, 7:37 AM

Thank you!

clang/lib/Format/Format.cpp
1420–1421

nit: (I realize this code was like this before, but,)
I find it more useful to have auto *Line instead of auto &Line (and similarly auto *Child instead of auto &Child at line 1028 below) as that makes it immediately clear that inside the body of the for loop we need to use -> to access members of the object.

This revision is now accepted and ready to land.Sep 17 2019, 7:37 AM
ymandel updated this revision to Diff 220504.Sep 17 2019, 7:58 AM

Fixed auto use.

ymandel marked 2 inline comments as done.Sep 17 2019, 7:59 AM
ymandel added inline comments.
clang/lib/Format/Format.cpp
1420–1421

Agreed!

ymandel marked an inline comment as done.Sep 17 2019, 7:59 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptSep 17 2019, 8:10 AM