[sync] Add constexpr to EnumSet

[sync] Add constexpr to EnumSet

constexpr allows the programmer to assert their intention that a
particular function or variable should be evaluatable at compile time
given the correct circumstances. If one adheres to the restrictions
imposed by constexpr, the compiler can often aggressively optimize
away code, leading to smaller binary size and improved performance.

This cl makes ModelTypeSet's varargs constructor as well as All() and
FromRange()
constexpr. It leverages this to make several of model_type.h's Types()
functions contexpr as well.

In my testing, this saves about 34 kb from the release binary. A
synthetic performance test that calls UserTypes(), CoreTypes() etc. in a
loop ran 3-4x faster on both debug and release builds, although the
magnitude of the difference was obviously smaller in the release builds.

Review-Url: https://codereview.chromium.org/2859033002
Cr-Commit-Position: refs/heads/master@{#478847}
Committed: https://chromium.googlesource.com/chromium/src/+/4d8a7c16ddb43b19a0768c68ccb2c72400879485

[Patrick Noland, 2017-05-03 21:49:05]

The CQ bit was checked by pnoland@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-03 21:49:47]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-03 22:01:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-03 22:01:04]

Dry run: Try jobs failed on following builders:
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Patrick Noland, 2017-05-04 18:01:53]

The CQ bit was checked by pnoland@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-04 18:02:54]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-04 18:19:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-04 18:20:00]

Dry run: Try jobs failed on following builders:
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)

[Patrick Noland, 2017-05-04 19:53:44]

The CQ bit was checked by pnoland@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-04 19:54:15]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-04 20:20:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-04 20:20:36]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Patrick Noland, 2017-05-04 21:05:56]

The CQ bit was checked by pnoland@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-04 21:06:31]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-04 21:35:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-04 21:35:57]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Patrick Noland, 2017-05-05 00:38:16]

The CQ bit was checked by pnoland@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-05 00:38:43]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-05 02:03:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-05 02:03:13]

Dry run: This issue passed the CQ dry run.

[Patrick Noland, 2017-05-05 20:45:01]

The CQ bit was checked by pnoland@chromium.org to run a CQ dry run

[Patrick Noland, 2017-05-05 20:45:13]

Patchset #1 (id:1) has been deleted

[Patrick Noland, 2017-05-05 20:45:21]

Patchset #1 (id:20001) has been deleted

[Patrick Noland, 2017-05-05 20:45:28]

Patchset #1 (id:40001) has been deleted

[Patrick Noland, 2017-05-05 20:45:34]

Patchset #1 (id:60001) has been deleted

[Patrick Noland, 2017-05-05 20:45:40]

Patchset #1 (id:80001) has been deleted

[Patrick Noland, 2017-05-05 20:45:47]

Patchset #1 (id:100001) has been deleted

[Patrick Noland, 2017-05-05 20:45:54]

Patchset #1 (id:120001) has been deleted

[commit-bot: I haz the power, 2017-05-05 20:46:27]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Patrick Noland, 2017-05-05 20:47:50]

Description was changed from

==========
[sync] Add constexpr to EnumSet

BUG=
==========

to

==========
[sync] Add constexpr to EnumSet

constexpr allows the programmer to assert their intention that a
particular function or variable should be evaluatable at compile time
given the correct circumstances. If one adheres to the restrictions
imposed by constexpr, the compiler can often aggressively optimize
away code, leading to smaller binary size and improved performance.

This cl makes ModelTypeSet's varargs constructor as well as All() and
FromRange()
constexpr. It leverages this to make several of model_type.h's Types()
functions contexpr as well.

In my testing, this saves about 34 kb from the release binary. A
synthetic performance test that calls UserTypes(), CoreTypes() etc. in a
loop ran 3-4x faster on both debug and release builds, although the
magnitude of the difference was obviously smaller in the release builds.
==========

[Patrick Noland, 2017-05-05 20:47:51]

pnoland@chromium.org changed reviewers:
+ skym@chromium.org

[commit-bot: I haz the power, 2017-05-05 21:50:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-05 21:50:19]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)

[Patrick Noland, 2017-05-05 22:49:12]

Sky, PTAL

I think the trybot failures are infra flakiness, but I will retry them to double
check

[skym, 2017-05-05 23:15:42]

lgtm

https://codereview.chromium.org/2859033002/diff/140001/components/sync/base/e...
File components/sync/base/enum_set.h (right):

https://codereview.chromium.org/2859033002/diff/140001/components/sync/base/e...
components/sync/base/enum_set.h:129: // constexpr construct our bitset. This
packing is done by left shifting 1 by
Can you rework/remove the "This packing is done by ...." sentence, you're
explaining what, not why.

https://codereview.chromium.org/2859033002/diff/140001/components/sync/base/e...
components/sync/base/enum_set.h:134: return 1ULL << (ToIndex(head)) |
bitstring(tail...);
Not really understanding the precedence of operators, this line makes me uneasy.
I'd feel better with some parens, but if you like this more, feel free to
ignore.

https://codereview.chromium.org/2859033002/diff/140001/components/sync/base/e...
components/sync/base/enum_set.h:140: static_assert(kValueCount < 64,
Is this really the right place for the static assert?

https://codereview.chromium.org/2859033002/diff/140001/components/sync/base/e...
components/sync/base/enum_set.h:144: "qualifiers from this file.");
What impact will this have? Why is this bad?

https://codereview.chromium.org/2859033002/diff/140001/components/sync/base/e...
components/sync/base/enum_set.h:150: "Max number of enum values is 64 for
constexpr "
Lets not duplicate quite so much. Maybe have a big comment explaining things,
and the static assert text can be empty/not present?

https://codereview.chromium.org/2859033002/diff/140001/components/sync/base/e...
components/sync/base/enum_set.h:160: "Max number of enum values is 64 for
constexpr "
same

https://codereview.chromium.org/2859033002/diff/140001/components/sync/base/e...
components/sync/base/enum_set.h:165: EnumBitSet(((1ULL << ToIndex(end)) - (1ULL
<< ToIndex(start))) |
Omg this is so clever! Love it! What do you think of having another function
that does the 1ULL << ToIndex(e) operation, it seems to be showing up
everywhere.

https://codereview.chromium.org/2859033002/diff/140001/components/sync/base/m...
File components/sync/base/model_type.h (right):

https://codereview.chromium.org/2859033002/diff/140001/components/sync/base/m...
components/sync/base/model_type.h:200: ModelTypeSet ProtocolTypes();
Why isn't this one constexpr?

https://codereview.chromium.org/2859033002/diff/140001/components/sync/base/m...
components/sync/base/model_type.h:219: constexpr bool
IsUserSelectableType(ModelType model_type) {
Can you re-order these functions so all the constexpr are next to eachother?

https://codereview.chromium.org/2859033002/diff/140001/components/sync/base/m...
components/sync/base/model_type.h:267: ModelTypeSet CoreTypes();
Why isn't this one constexpr?

https://codereview.chromium.org/2859033002/diff/140001/components/sync/base/m...
components/sync/base/model_type.h:269: ModelTypeSet PriorityCoreTypes();
Why isn't this one constexpr?

https://codereview.chromium.org/2859033002/diff/140001/components/sync/base/m...
components/sync/base/model_type.h:297: FullModelTypeSet
ToFullModelTypeSet(ModelTypeSet in);
Why isn't this one constexpr?

[Patrick Noland, 2017-05-08 17:59:58]

The CQ bit was checked by pnoland@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-08 18:00:42]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-08 19:41:59]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-08 19:42:00]

Dry run: This issue passed the CQ dry run.

[Patrick Noland, 2017-05-08 20:27:53]

pnoland@chromium.org changed reviewers:
+ noyau@chromium.org

[Patrick Noland, 2017-05-08 20:29:20]

+noyau for added reading_list dependency; the new use of the build flag is the
same as the old one, but in a different subdirectory of sync

https://codereview.chromium.org/2859033002/diff/140001/components/sync/base/e...
File components/sync/base/enum_set.h (right):

https://codereview.chromium.org/2859033002/diff/140001/components/sync/base/e...
components/sync/base/enum_set.h:129: // constexpr construct our bitset. This
packing is done by left shifting 1 by
On 2017/05/05 23:15:41, skym wrote:
> Can you rework/remove the "This packing is done by ...." sentence, you're
> explaining what, not why.

Done.

https://codereview.chromium.org/2859033002/diff/140001/components/sync/base/e...
components/sync/base/enum_set.h:134: return 1ULL << (ToIndex(head)) |
bitstring(tail...);
On 2017/05/05 23:15:41, skym wrote:
> Not really understanding the precedence of operators, this line makes me
uneasy.
> I'd feel better with some parens, but if you like this more, feel free to
> ignore.

Done.

https://codereview.chromium.org/2859033002/diff/140001/components/sync/base/e...
components/sync/base/enum_set.h:140: static_assert(kValueCount < 64,
On 2017/05/05 23:15:41, skym wrote:
> Is this really the right place for the static assert?

Done.

https://codereview.chromium.org/2859033002/diff/140001/components/sync/base/e...
components/sync/base/enum_set.h:144: "qualifiers from this file.");
On 2017/05/05 23:15:41, skym wrote:
> What impact will this have? Why is this bad?

Done. I explained that it won't affect correctness, but will be a minor
deoptimization.

https://codereview.chromium.org/2859033002/diff/140001/components/sync/base/e...
components/sync/base/enum_set.h:150: "Max number of enum values is 64 for
constexpr "
On 2017/05/05 23:15:41, skym wrote:
> Lets not duplicate quite so much. Maybe have a big comment explaining things,
> and the static assert text can be empty/not present?

Done.

https://codereview.chromium.org/2859033002/diff/140001/components/sync/base/e...
components/sync/base/enum_set.h:160: "Max number of enum values is 64 for
constexpr "
On 2017/05/05 23:15:41, skym wrote:
> same

Done.

https://codereview.chromium.org/2859033002/diff/140001/components/sync/base/e...
components/sync/base/enum_set.h:165: EnumBitSet(((1ULL << ToIndex(end)) - (1ULL
<< ToIndex(start))) |
On 2017/05/05 23:15:41, skym wrote:
> Omg this is so clever! Love it! What do you think of having another function
> that does the 1ULL << ToIndex(e) operation, it seems to be showing up
> everywhere.

Done.

https://codereview.chromium.org/2859033002/diff/140001/components/sync/base/m...
File components/sync/base/model_type.h (right):

https://codereview.chromium.org/2859033002/diff/140001/components/sync/base/m...
components/sync/base/model_type.h:219: constexpr bool
IsUserSelectableType(ModelType model_type) {
On 2017/05/05 23:15:42, skym wrote:
> Can you re-order these functions so all the constexpr are next to eachother?

Done.

https://codereview.chromium.org/2859033002/diff/140001/components/sync/base/m...
components/sync/base/model_type.h:267: ModelTypeSet CoreTypes();
On 2017/05/05 23:15:41, skym wrote:
> Why isn't this one constexpr?

Done.

https://codereview.chromium.org/2859033002/diff/140001/components/sync/base/m...
components/sync/base/model_type.h:269: ModelTypeSet PriorityCoreTypes();
On 2017/05/05 23:15:42, skym wrote:
> Why isn't this one constexpr?

Done.

https://codereview.chromium.org/2859033002/diff/140001/components/sync/base/m...
components/sync/base/model_type.h:297: FullModelTypeSet
ToFullModelTypeSet(ModelTypeSet in);
On 2017/05/05 23:15:41, skym wrote:
> Why isn't this one constexpr?

There's not a clean way to constexpr copy an EnumSet of one type to another
type. You don't have constexpr access to the underlying uint64, and you can't
copy the bitset.

[noyau (Ping after 24h), 2017-05-09 13:10:02]

https://codereview.chromium.org/2859033002/diff/160001/components/sync/base/DEPS
File components/sync/base/DEPS (right):

https://codereview.chromium.org/2859033002/diff/160001/components/sync/base/D...
components/sync/base/DEPS:7: "+components/reading_list/core",
I'm not a big fan of sync/base having a dependency on reading list. This is a
circular dependency, as reading list itself depends on component/sync. None of
the other synced types are listed as dependencies.

Is there a way to avoid this dependency?

If there is no way to do so, can the dependency be removed from
components/sync/syncable?

[Patrick Noland, 2017-05-12 19:44:12]

The CQ bit was checked by pnoland@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-12 19:45:43]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-12 19:48:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-12 19:48:30]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Patrick Noland, 2017-05-12 19:55:07]

The CQ bit was checked by pnoland@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-12 19:56:19]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-12 19:58:11]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-12 19:58:14]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Patrick Noland, 2017-05-15 20:51:27]

The CQ bit was checked by pnoland@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-15 20:52:17]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-15 22:48:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-15 22:48:59]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Patrick Noland, 2017-06-06 20:57:15]

The CQ bit was checked by pnoland@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-06 20:57:30]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-06 21:01:40]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-06 21:01:41]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Patrick Noland, 2017-06-06 21:11:50]

The CQ bit was checked by pnoland@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-06 21:12:10]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-06 22:08:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-06 22:08:33]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Patrick Noland, 2017-06-06 22:27:26]

The CQ bit was checked by pnoland@chromium.org to run a CQ dry run

[Patrick Noland, 2017-06-06 22:27:43]

Patchset #4 (id:200001) has been deleted

[Patrick Noland, 2017-06-06 22:27:54]

Patchset #4 (id:220001) has been deleted

[commit-bot: I haz the power, 2017-06-06 22:27:55]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Patrick Noland, 2017-06-06 22:28:03]

Patchset #3 (id:180001) has been deleted

[commit-bot: I haz the power, 2017-06-06 23:44:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-06 23:44:28]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[Patrick Noland, 2017-06-07 17:40:29]

noyau@, PTAL

https://codereview.chromium.org/2859033002/diff/160001/components/sync/base/DEPS
File components/sync/base/DEPS (right):

https://codereview.chromium.org/2859033002/diff/160001/components/sync/base/D...
components/sync/base/DEPS:7: "+components/reading_list/core",
On 2017/05/09 13:10:02, noyau (Ping after 24h) wrote:
> I'm not a big fan of sync/base having a dependency on reading list. This is a
> circular dependency, as reading list itself depends on component/sync. None of
> the other synced types are listed as dependencies.
> 
> Is there a way to avoid this dependency?
> 
> If there is no way to do so, can the dependency be removed from
> components/sync/syncable?

I've resolved this by moving the reading list build flag and runtime switch to a
new /features directory, which seems to be an accepted way of separating the
enabled/disabled state of a component from its implementation.

[noyau (Ping after 24h), 2017-06-07 20:35:14]

Thanks for the changes, way better dependency graph.

lgtm

[Patrick Noland, 2017-06-07 20:37:07]

The CQ bit was checked by pnoland@chromium.org

[Patrick Noland, 2017-06-07 20:37:08]

The patchset sent to the CQ was uploaded after l-g-t-m from skym@chromium.org
Link to the patchset: https://codereview.chromium.org/2859033002/#ps240001
(title: "Move reading list switches and buildflag to /features")

[commit-bot: I haz the power, 2017-06-07 20:39:19]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-07 20:40:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-07 20:40:05]

CQ cannot get SignCLA result. Please try later.

[Patrick Noland, 2017-06-12 21:24:22]

The CQ bit was checked by pnoland@chromium.org

[commit-bot: I haz the power, 2017-06-12 21:24:56]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-13 00:52:13]

CQ is committing da patch.

Bot data: {"patchset_id": 240001, "attempt_start_ts": 1497302662615270,
"parent_rev": "ce9baa0cd40adb9668971b2b4529a3bf7fb462dc", "commit_rev":
"4d8a7c16ddb43b19a0768c68ccb2c72400879485"}

[commit-bot: I haz the power, 2017-06-13 00:52:29]

Description was changed from

==========
[sync] Add constexpr to EnumSet

constexpr allows the programmer to assert their intention that a
particular function or variable should be evaluatable at compile time
given the correct circumstances. If one adheres to the restrictions
imposed by constexpr, the compiler can often aggressively optimize
away code, leading to smaller binary size and improved performance.

This cl makes ModelTypeSet's varargs constructor as well as All() and
FromRange()
constexpr. It leverages this to make several of model_type.h's Types()
functions contexpr as well.

In my testing, this saves about 34 kb from the release binary. A
synthetic performance test that calls UserTypes(), CoreTypes() etc. in a
loop ran 3-4x faster on both debug and release builds, although the
magnitude of the difference was obviously smaller in the release builds.
==========

to

==========
[sync] Add constexpr to EnumSet

constexpr allows the programmer to assert their intention that a
particular function or variable should be evaluatable at compile time
given the correct circumstances. If one adheres to the restrictions
imposed by constexpr, the compiler can often aggressively optimize
away code, leading to smaller binary size and improved performance.

This cl makes ModelTypeSet's varargs constructor as well as All() and
FromRange()
constexpr. It leverages this to make several of model_type.h's Types()
functions contexpr as well.

In my testing, this saves about 34 kb from the release binary. A
synthetic performance test that calls UserTypes(), CoreTypes() etc. in a
loop ran 3-4x faster on both debug and release builds, although the
magnitude of the difference was obviously smaller in the release builds.

Review-Url: https://codereview.chromium.org/2859033002
Cr-Commit-Position: refs/heads/master@{#478847}
Committed:
https://chromium.googlesource.com/chromium/src/+/4d8a7c16ddb43b19a0768c68ccb2...
==========

[commit-bot: I haz the power, 2017-06-13 00:52:31]

Committed patchset #3 (id:240001) as
https://chromium.googlesource.com/chromium/src/+/4d8a7c16ddb43b19a0768c68ccb2...