0

Add warning to base::BarrierCallback about reference elision

BarrierCallback's doc comment states that when T is a reference, it
drops the reference and moves/copies the underlying value type. Under
the hood, this is accomplished with the magical base::remove_cvref.

It's easy not to realize that the references contained within types,
like std::pair<const Foo&, const Bar&>, will not be converted to their
value types because base::remove_cvref only operates on the top type.

To be clear, this is just an easy mistake that callers might make. It's
not quite a footgun, nor is it something that BarrierCallback could
guard against with incantations of template magic. I ran into this sharp
edge in another CL, and figured it deserves a short warning:
https://chromium-review.googlesource.com/c/chromium/src/+/3226048/14

Change-Id: I20f58e8ae77b525dfe522b89f170ae04287eaa4b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3252025
Reviewed-by: Chris Fredrickson <cfredric@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Dan McArdle <dmcardle@chromium.org>
Cr-Commit-Position: refs/heads/main@{#941833}
This commit is contained in:
Dan McArdle
2021-11-15 21:04:02 +00:00
committed by Chromium LUCI CQ
parent 3960bf988a
commit a4afa60818

@ -68,6 +68,10 @@ void ShouldNeverRun(T t) {
// is copied. (BarrierCallback does not support `T`s that are neither movable
// nor copyable.) If T is a reference, the reference is removed, and the
// callback moves or copies the underlying value per the previously stated rule.
// Beware of creating dangling references. Types that contain references but are
// not references themselves are not modified for callback storage, e.g.
// `std::pair<int, const Foo&>`. Dangling references will be passed to
// `done_callback` if the referenced `Foo` objects have already been deleted.
//
// If `num_callbacks` is 0, `done_callback` is executed immediately.
//