-
Notifications
You must be signed in to change notification settings - Fork 456
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix cppcoreguidelines-narrowing-conversions warning reported by clang-tidy #1159
Changes from 1 commit
245e299
0084655
01539d3
9daa79f
a2fe597
60cc348
0e3275e
1f46891
10461c6
187d5a1
fec58b1
1e4d846
af2f01a
6f63f41
d7410bf
501be08
a05a728
54b7842
82cb120
291e577
32aea24
27ef7a2
9818f10
dbbc259
d752b38
91f3286
3127679
3e7333c
e364ee4
86f1a27
931e179
8fe94d5
4b3293d
2858cb1
38f7529
1c89644
c034178
3370ba3
dedf6c1
dcabf0d
a45b195
f01cf83
810ef4c
8cdb044
77758b6
a5ec5f2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -22,9 +22,11 @@ | |||||
|
||||||
#include <fmt/format.h> | ||||||
|
||||||
#include <limits> | ||||||
#include <string> | ||||||
#include <utility> | ||||||
|
||||||
#include "parse_util.h" | ||||||
#include "status.h" | ||||||
#include "string_util.h" | ||||||
|
||||||
|
@@ -101,27 +103,30 @@ class MultiStringField : public ConfigField { | |||||
std::vector<std::string> *receiver_; | ||||||
}; | ||||||
|
||||||
class IntField : public ConfigField { | ||||||
template <typename IntegerType> | ||||||
class IntegerField : public ConfigField { | ||||||
public: | ||||||
IntField(int *receiver, int n, int min, int max) : receiver_(receiver), min_(min), max_(max) { *receiver_ = n; } | ||||||
~IntField() override = default; | ||||||
IntegerField(IntegerType *receiver, IntegerType n, IntegerType min, IntegerType max) | ||||||
: receiver_(receiver), min_(min), max_(max) { | ||||||
*receiver_ = n; | ||||||
} | ||||||
~IntegerField() override = default; | ||||||
std::string ToString() override { return std::to_string(*receiver_); } | ||||||
Status ToNumber(int64_t *n) override { | ||||||
*n = *receiver_; | ||||||
return Status::OK(); | ||||||
} | ||||||
Status Set(const std::string &v) override { | ||||||
int64_t n; | ||||||
auto s = Util::DecimalStringToNum(v, &n, min_, max_); | ||||||
auto s = ParseInt(v, {min_, max_}); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It can be deduced. Is it better to explicitly pass type? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it is better to pass type explicitly here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||||||
if (!s.IsOK()) return s; | ||||||
*receiver_ = static_cast<int>(n); | ||||||
*receiver_ = s.GetValue(); | ||||||
return Status::OK(); | ||||||
} | ||||||
|
||||||
private: | ||||||
int *receiver_; | ||||||
int min_ = INT_MIN; | ||||||
int max_ = INT_MAX; | ||||||
IntegerType *receiver_; | ||||||
IntegerType min_ = std::numeric_limits<IntegerType>::min(); | ||||||
IntegerType max_ = std::numeric_limits<IntegerType>::max(); | ||||||
}; | ||||||
|
||||||
class OctalField : public ConfigField { | ||||||
|
@@ -147,31 +152,6 @@ class OctalField : public ConfigField { | |||||
int max_ = INT_MAX; | ||||||
}; | ||||||
|
||||||
class Int64Field : public ConfigField { | ||||||
public: | ||||||
Int64Field(int64_t *receiver, int64_t n, int64_t min, int64_t max) : receiver_(receiver), min_(min), max_(max) { | ||||||
*receiver_ = n; | ||||||
} | ||||||
~Int64Field() override = default; | ||||||
std::string ToString() override { return std::to_string(*receiver_); } | ||||||
Status ToNumber(int64_t *n) override { | ||||||
*n = *receiver_; | ||||||
return Status::OK(); | ||||||
} | ||||||
Status Set(const std::string &v) override { | ||||||
int64_t n; | ||||||
auto s = Util::DecimalStringToNum(v, &n, min_, max_); | ||||||
if (!s.IsOK()) return s; | ||||||
*receiver_ = n; | ||||||
return Status::OK(); | ||||||
} | ||||||
|
||||||
private: | ||||||
int64_t *receiver_; | ||||||
int64_t min_ = INT64_MIN; | ||||||
int64_t max_ = INT64_MAX; | ||||||
}; | ||||||
|
||||||
class YesNoField : public ConfigField { | ||||||
public: | ||||||
YesNoField(bool *receiver, bool b) : receiver_(receiver) { *receiver_ = b; } | ||||||
|
@@ -218,28 +198,3 @@ class EnumField : public ConfigField { | |||||
int *receiver_; | ||||||
configEnum *enums_ = nullptr; | ||||||
}; | ||||||
|
||||||
class Uint32Field : public ConfigField { | ||||||
public: | ||||||
Uint32Field(uint32_t *receiver, uint32_t n, uint32_t min, uint32_t max) : receiver_(receiver), min_(min), max_(max) { | ||||||
*receiver_ = n; | ||||||
} | ||||||
~Uint32Field() override = default; | ||||||
std::string ToString() override { return std::to_string(*receiver_); } | ||||||
Status ToNumber(int64_t *n) override { | ||||||
*n = *receiver_; | ||||||
return Status::OK(); | ||||||
} | ||||||
Status Set(const std::string &v) override { | ||||||
int64_t n; | ||||||
auto s = Util::DecimalStringToNum(v, &n, min_, max_); | ||||||
if (!s.IsOK()) return s; | ||||||
*receiver_ = static_cast<uint32_t>(n); | ||||||
return Status::OK(); | ||||||
} | ||||||
|
||||||
private: | ||||||
uint32_t *receiver_; | ||||||
uint32_t min_ = INT_MIN; | ||||||
uint32_t max_ = INT_MAX; | ||||||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer we specify template types explicitly instead of deduced from arguments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In all places (and with
int
type) or only in few places with typesuint32_t
anduint64_t
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can define some aliases, e.g.
using IntField = IntergerField<int>
,using UInt32Field = IntergerField<uint32_t>
.Then you can use these alias in the config map, so most config kv will remain the original form (
IntField
).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done