[PATCH] D54861: [SanitizerCommon] Test `CombinedAllocator::ForEachChunk()` in unit tests.

Dan Liew via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 27 09:15:32 PST 2018


delcypher added a comment.

In D54861#1308535 <https://reviews.llvm.org/D54861#1308535>, @delcypher wrote:

> In D54861#1308171 <https://reviews.llvm.org/D54861#1308171>, @kubamracek wrote:
>
> > LGTM, of course (more tests, yay!).
> >
> > The indentation makes this hard to read though. Maybe declare the lambda as a separate variable?
>
>
> The indentation is clang-format's doing. I'm a bit hesitant to put the lambda in a variable because it's only used in one place so putting it inline is the more standard thing to do.
>  However, I don't have a super strong opinion here so if you think it's clearer then I'll make the change.


@kubamracek I tried assigning the lambda to a local variable. You're right, it is more readable. I've updated this patch with that change.


Repository:
  rCRT Compiler Runtime

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

https://reviews.llvm.org/D54861





More information about the llvm-commits mailing list