Rename base::IgnoreResult to base::IgnoreReturn?

47 views
Skip to first unread message

Xiaohan Wang (王消寒)

unread,
May 6, 2022, 7:11:38 PM5/6/22
to chromium-dev
I have a case where a function expects a reply callback with a result (e.g. bool for success), but the caller doesn't really care about the result at all, e.g.
```
void Foo(base::OnceCallback<void(bool)> reply_callback);

Bar(base::OnceClosure closure) {
  Foo(???) // Needs an adapter from `closure` to `reply_callback` that ignores the result.
}
```

For example:
```
void IgnoreResult(base::OnceClosure callback, bool /*success*/) {
  std::move(callback).Run();
}

Bar(base::OnceClosure closure) {
  Foo(base::BindOnce(&IgnoreResult, std::move(closure)));
}
```

That all worked pretty well until I realized there's already a base::IgnoreResult, that ignores the "return value".

I would argue that "return" and "result" have different meanings in the callback world. "return" is the sync return value, and "result" is often the async returned result. All the documentation on base::IgnoreResult mentions that it's the return value that's being ignored.

With that, does it make sense to rename base::IgnoreResult to base::IgnoreReturn? If possible, I'd like to add the above IgnoreResult() (and make it templated) to bind.h, but that can be a different discussion.

Best,
Xiaohan

Daniel Cheng

unread,
May 6, 2022, 7:28:02 PM5/6/22
to xhw...@chromium.org, chromium-dev
I think you'll find people on either side of the fence for result vs return here. To me, result doesn't really imply async.

In spirit, IgnoreResult here seems pretty similar to base::DoNothing(). One way to write this might be something like:
  base::BindOnce(base::OnceCallback<void(bool)>(base::DoNothing()).Then(std::move(closure)))

With a templated IgnoreResult, you'd need to explicitly specify template params anyway. This is still a little more wordy than an all-in-one-helper, but I previously moved base::DoNothing() away from that model (base::DoNothing() supported implicit conversions to callback types) because there were a number of places where it didn't work well and ended up being verbose: https://bugs.chromium.org/p/chromium/issues/detail?id=1252980

It also revealed a number of places where we were nesting callbacks multiple times, which is a bit inefficient, e.g. see https://chromium-review.googlesource.com/c/chromium/src/+/3183366.

Daniel

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAF1j9YPkM5AhxoK24Z4SEi-mNw2fWV3Li8C-H0wXu1zPsDwCRA%40mail.gmail.com.

Joe Mason

unread,
May 9, 2022, 1:52:30 PM5/9/22
to Daniel Cheng, Xiaohan Wang (王消寒), chromium-dev
I would call your usage "ignore parameter": the technical issue is that the callback type has N parameters, and you want to bind a function that only cares about N-1 of them. The fact that the parameter is the result of an operation is coincidence.

I could have sworn we already had a helper for this, but I can't find it now. I guess I'm making it up.

Daniel Cheng

unread,
May 9, 2022, 2:07:51 PM5/9/22
to Joe Mason, Xiaohan Wang (王消寒), chromium-dev
That helper is base::DoNothing() :)

Though I realized in the above example, it'd be better to write:
Bar(base::OnceClosure closure) {
  Foo(base::OnceCallback<void(bool)>(base::DoNothing()).Then(std::move(closure)));
}

And skip the extra base::BindOnce() that I added by mistake.

Daniel

Xiaohan Wang (王消寒)

unread,
May 9, 2022, 2:36:44 PM5/9/22
to Daniel Cheng, Joe Mason, chromium-dev
I agree "parameter" is less ambiguous. But for the same reason, I feel "return" is less ambiguous than "result". But that could just be me.

dcheng: That's a clever use of `base::DoNothing()` and `.Then()`. I'll use it for now before we have anything better.

Actually, OOC, won't the following just work? I am sure I am missing something here.

Bar(base::OnceClosure closure) {
  Foo(base::DoNothing().Then(std::move(closure)));
}

Joe Mason

unread,
May 9, 2022, 3:18:59 PM5/9/22
to Xiaohan Wang (王消寒), Daniel Cheng, chromium-dev
I think Daniel said base::DoNothing() won't automatically deduce the callback signature anymore.

But if I'm reading this right, I think a slightly less verbose version would be:

```
Bar(base::OnceClosure closure) {
  Foo( base::DoNothing::Once<void(bool)>().Then(std::move(closure)));
}
```

This doesn't address a case where there are multiple parameters and you just want to ignore one, though:

```
void Foo(base::OnceCallback<void(bool, int)> reply_callback);

Bar(base::OnceCallback<void(int)> int_callback) {
  // The bool gets ignored, but how does the int get passed through to int_callback?
  Foo(base::DoNothing::Once<void(bool)>().Then(std::move(int_callback)));
}
```

Best way I know to do this is with a lambda, which is also pretty verbose.
Reply all
Reply to author
Forward
0 new messages