Before move semantics, the IWYU (Include What You Use) rules were simple:1) if you declare a method that takes Foo by value you must include foo.h (so that users who merely forward a provided |foo| to the param don't have to include foo.h)2) if you declare a method that returns Foo by value you must include foo.h (so that users who don't use the return value don't have to include foo.h)3) if you take Foo by pointer/ref only you can fwd-decl class Foo;4) if you use Foo you obviously need foo.hWith move-semantics however I just ran into an interesting scenario:void Bar(base::OnceClosure closure);should a header providing this method include callback_forward.h or callback.h or neither?
Per rule #1 above it should include callback.h... but!...because of move-only semantics, any caller must either:a) std::move(closure) into the param.b) Have |closure| already be an r-value.a) Forces the caller to include callback.h by IWYU rule #4 (for std::move)b) Forces the API that provides the r-value to include callback.h by IWYU rule #2 and therefore the caller transitively already has foo.hSo including callback.h to define Bar() is redundant..?PS: Does that make callback_forward.h essentially be deprecated in the OnceClosure world?
--
--
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 view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAJTZ7LLT8TxFoxTMmGRgVrGXXNmo6HTjvKfAFCSgG5watahLPw%40mail.gmail.com.
On Fri, Jul 21, 2017 at 3:15 PM Gabriel Charette <g...@chromium.org> wrote:Before move semantics, the IWYU (Include What You Use) rules were simple:1) if you declare a method that takes Foo by value you must include foo.h (so that users who merely forward a provided |foo| to the param don't have to include foo.h)2) if you declare a method that returns Foo by value you must include foo.h (so that users who don't use the return value don't have to include foo.h)3) if you take Foo by pointer/ref only you can fwd-decl class Foo;4) if you use Foo you obviously need foo.hWith move-semantics however I just ran into an interesting scenario:void Bar(base::OnceClosure closure);should a header providing this method include callback_forward.h or callback.h or neither?The idea behind callback_forward.h is to provide a lightweight header to #include in headers to avoid pulling in heavyweight headers via transitive dependencies. So the header should still be #include'ing callback_forward.h.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAJTZ7LLBzqO%2BdZyf8sMSuH7m-Qnn6LMCE3XTk-7_gu9_VYo%3D9g%40mail.gmail.com.
On Fri, Jul 21, 2017 at 8:03 PM Daniel Cheng <dch...@chromium.org> wrote:On Fri, Jul 21, 2017 at 3:15 PM Gabriel Charette <g...@chromium.org> wrote:Before move semantics, the IWYU (Include What You Use) rules were simple:1) if you declare a method that takes Foo by value you must include foo.h (so that users who merely forward a provided |foo| to the param don't have to include foo.h)2) if you declare a method that returns Foo by value you must include foo.h (so that users who don't use the return value don't have to include foo.h)3) if you take Foo by pointer/ref only you can fwd-decl class Foo;4) if you use Foo you obviously need foo.hWith move-semantics however I just ran into an interesting scenario:void Bar(base::OnceClosure closure);should a header providing this method include callback_forward.h or callback.h or neither?The idea behind callback_forward.h is to provide a lightweight header to #include in headers to avoid pulling in heavyweight headers via transitive dependencies. So the header should still be #include'ing callback_forward.h.Read on below though, my point is callback_forward.h doesn't do anything here because the users of this API are already guaranteed to have callback.h included. So either we include callback.h (redundant but closer to IWYU rules) or we include neither. callback_forward.h is incorrect.
Related question: Do we have any IWYU tools, say, for scanning existing code? Or, even, auto-fixing header files? I wouldn't mind running such a tool every few months to "keep my house in order."
On Fri, Jul 21, 2017 at 6:24 PM Gabriel Charette <g...@chromium.org> wrote:On Fri, Jul 21, 2017 at 8:03 PM Daniel Cheng <dch...@chromium.org> wrote:On Fri, Jul 21, 2017 at 3:15 PM Gabriel Charette <g...@chromium.org> wrote:Before move semantics, the IWYU (Include What You Use) rules were simple:1) if you declare a method that takes Foo by value you must include foo.h (so that users who merely forward a provided |foo| to the param don't have to include foo.h)2) if you declare a method that returns Foo by value you must include foo.h (so that users who don't use the return value don't have to include foo.h)3) if you take Foo by pointer/ref only you can fwd-decl class Foo;4) if you use Foo you obviously need foo.hWith move-semantics however I just ran into an interesting scenario:void Bar(base::OnceClosure closure);should a header providing this method include callback_forward.h or callback.h or neither?The idea behind callback_forward.h is to provide a lightweight header to #include in headers to avoid pulling in heavyweight headers via transitive dependencies. So the header should still be #include'ing callback_forward.h.Read on below though, my point is callback_forward.h doesn't do anything here because the users of this API are already guaranteed to have callback.h included. So either we include callback.h (redundant but closer to IWYU rules) or we include neither. callback_forward.h is incorrect.There can be consumers of the header that don't use Bar(): in that case, callback_forward.h is sufficient.
Le ven. 21 juil. 2017 22:14, Daniel Cheng <dch...@chromium.org> a écrit :On Fri, Jul 21, 2017 at 6:24 PM Gabriel Charette <g...@chromium.org> wrote:On Fri, Jul 21, 2017 at 8:03 PM Daniel Cheng <dch...@chromium.org> wrote:On Fri, Jul 21, 2017 at 3:15 PM Gabriel Charette <g...@chromium.org> wrote:Before move semantics, the IWYU (Include What You Use) rules were simple:1) if you declare a method that takes Foo by value you must include foo.h (so that users who merely forward a provided |foo| to the param don't have to include foo.h)2) if you declare a method that returns Foo by value you must include foo.h (so that users who don't use the return value don't have to include foo.h)3) if you take Foo by pointer/ref only you can fwd-decl class Foo;4) if you use Foo you obviously need foo.hWith move-semantics however I just ran into an interesting scenario:void Bar(base::OnceClosure closure);should a header providing this method include callback_forward.h or callback.h or neither?The idea behind callback_forward.h is to provide a lightweight header to #include in headers to avoid pulling in heavyweight headers via transitive dependencies. So the header should still be #include'ing callback_forward.h.Read on below though, my point is callback_forward.h doesn't do anything here because the users of this API are already guaranteed to have callback.h included. So either we include callback.h (redundant but closer to IWYU rules) or we include neither. callback_forward.h is incorrect.There can be consumers of the header that don't use Bar(): in that case, callback_forward.h is sufficient.But isn't this a valid argument for every parameter being passed by value and goes against IWYU rule #1 above? Or maybe it's fine for move-only types passed by value because if the caller does use Bar() he's responsible for callback.h as highlighted below?
// NOTE: Header files that do not require the full definition of Callback or // Closure should #include "base/callback_forward.h" instead of this file.
PK
--
--
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 view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAAHOzFCdKmcxoGeRrFq9kFgg7d3mYP8wKV-8HAXm7QmRt297QA%40mail.gmail.com.
PK