[PATCH] Add ForEachChunk() to sanitizer allocators.

[David Blaikie]

On Sun, Mar 17, 2013 at 7:46 PM, Sergey Matveev <earthdok at google.com> wrote:
>
>
> ================
> Comment at: lib/sanitizer_common/tests/sanitizer_allocator_test.cc:571
> @@ +570,3 @@
> + public:
> +  explicit IterationTestCallback(std::set<void *> *chunks)
> +    : chunks_(chunks) {}
> ----------------
> David Blaikie wrote:
>> (not important, but just a thought)
>>
>> The LLVM project codebase doesn't really have the convention to pass mutable data by pointer (we're usually more than happy to pass by non-const ref). Not sure if the sanitizer code has adopted this convention consistently/separately from the rest of LLVM.
> Sanitizer code actually follows the Google style guide, which strongly prohibits non-const reference parameters.
>
> TBH I should've made the parameter to ForEachChunk a pointer. The way it is now is pretty awkward.

I'd say in this case (maybe) you could justify the current design as
"compatibility with existing conventions" in the C++ standard library.

(the other way to do a stateful callback API is the same way
std::for_each works - taking a parameter by const ref (or
rvalue/perfect forwarding/etc), copy it, call all the callbacks on it,
then return it)

>
>
> ================
> Comment at: lib/sanitizer_common/tests/sanitizer_allocator_test.cc:610
> @@ +609,3 @@
> +  a->ForceLock();
> +  a->template ForEachChunk<IterationTestCallback>(callback);
> +  a->ForceUnlock();
> ----------------
> David Blaikie wrote:
>> Any reason you can't just call this with "a->ForEachChunk(callback)", relying on template argument deduction?
> Thanks for pointing that out.
>
>
> http://llvm-reviews.chandlerc.com/D539