[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

Sedenion via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jun 12 12:00:50 PDT 2023


Sedeniono marked an inline comment as done.
Sedeniono added inline comments.


================
Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:80
+      // directive, since these do not terminate a code block.
+      if (!Line.InPPDirective && Line.Level < IndentForLevel.size())
+        IndentForLevel.resize(Line.Level + 1, -1);
----------------
HazardyKnusperkeks wrote:
> Sedeniono wrote:
> > HazardyKnusperkeks wrote:
> > > Do we need this check? I assume `resize` has a check if it has to grow or shrink, so we would check twice when the size is to be changed and once if not.
> > The `Line.Level < IndentForLevel.size()` is not necessary because of lines 63+64. `resize` itself can both grow and shrink the container. 
> > If `Line.Level >= IndentForLevel.size()` and we omit the `Line.Level < IndentForLevel.size()` in line 80, then the `resize()` in line 81 does nothing because the container already has the size `Line.Level+1`.
> > 
> > I just added the check in line 80 because I thought it made the intention more clear and explicit: Forget indent levels when going to smaller levels.
> > Still, I can of course remove it again. Should I?
> You could put the intention in a comment and add an assert.
Ok, I put the check into an `assert()` instead and uploaded a new diff. Note that asserting for `Line.Level < IndentForLevel.size()` would be wrong, since it can also be equal at this point. The comment already states the intention, so did not change it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151047/new/

https://reviews.llvm.org/D151047



More information about the cfe-commits mailing list