struct with >= 10 bools fails style check |
||||||
Issue description
When adding two more booleans to the following struct:
###
struct Result {
explicit Result(bool initial_values);
bool model_ready_ok;
bool currently_showing_ok;
bool feature_enabled_ok;
bool config_ok;
bool used_ok;
bool trigger_ok;
bool preconditions_ok;
bool session_rate_ok;
bool NoErrors();
};
###
The following error is emitted (multiple times, once for each include of the header):
../../components/feature_engagement_tracker/internal/condition_validator.h:32:10: error: [chromium-style] Complex constructor has an inlined body.
struct Result {
^
1 error generated.
At the time of filing the bug, there seems to be a limit to 9 ints (bools):
https://chromium.googlesource.com/chromium/src/+/f9774e4bbc3820cff53edeb95f5562ae69dc391a/tools/clang/plugins/FindBadConstructsConsumer.cpp#358
Source header:
https://chromium.googlesource.com/chromium/src/+/e1663d2c4db343e0285d6f2cc834654d00383c82/components/feature_engagement_tracker/internal/condition_validator.h#23
Source .cc file:
https://chromium.googlesource.com/chromium/src/+/e1663d2c4db343e0285d6f2cc834654d00383c82/components/feature_engagement_tracker/internal/condition_validator.cc
An example diff that should be possible to apply on top of 02ed4aebda2b2da70a2c32a7b2b92c67c8c01dd4 has been attached.
Note: The diff includes building //components/feature_engagement_tracker:unit_tests for all platforms, since at the time of filing the bug it is only compiled for Android.
,
May 24 2017
So it's not surprising that this fails the style check, since we set a pretty arbitrary limit. At the same time: - The warning is not useful. It complains about the constructor, but of course, the only explicit constructor is not inline. - It's not clear to me that this copy constructor will be expensive in terms of size, maybe it would just be a memcpy. For followups: - We should improve the error to point out which implicitly declared constructor is being warned about. - Check if we really need to emit this warning for structs/classes that only have POD members.
,
Nov 10 2017
|
||||||
►
Sign in to add a comment |
||||||
Comment 1 by nyquist@chromium.org
, May 24 2017