-
Notifications
You must be signed in to change notification settings - Fork 501
[CodeHealth] Fix clang-tidy warnings part 1 #3493
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
Changes from all commits
a4d3abe
e33170b
604fa56
afabc95
c7ef2d1
3765914
4685f24
64f05d3
fb379f9
8d015b2
4480f10
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 |
|---|---|---|
|
|
@@ -21,7 +21,6 @@ class LongLastValueAggregation : public Aggregation | |
| { | ||
| public: | ||
| LongLastValueAggregation(); | ||
| LongLastValueAggregation(LastValuePointData &&); | ||
| LongLastValueAggregation(const LastValuePointData &); | ||
|
|
||
| void Aggregate(int64_t value, const PointAttributes &attributes = {}) noexcept override; | ||
|
|
@@ -43,16 +42,15 @@ class DoubleLastValueAggregation : public Aggregation | |
| { | ||
| public: | ||
| DoubleLastValueAggregation(); | ||
| DoubleLastValueAggregation(LastValuePointData &&); | ||
|
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.
|
||
| DoubleLastValueAggregation(const LastValuePointData &); | ||
|
|
||
| void Aggregate(int64_t /* value */, const PointAttributes & /* attributes */) noexcept override {} | ||
|
|
||
| void Aggregate(double value, const PointAttributes &attributes = {}) noexcept override; | ||
|
|
||
| virtual std::unique_ptr<Aggregation> Merge(const Aggregation &delta) const noexcept override; | ||
| std::unique_ptr<Aggregation> Merge(const Aggregation &delta) const noexcept override; | ||
|
|
||
| virtual std::unique_ptr<Aggregation> Diff(const Aggregation &next) const noexcept override; | ||
| std::unique_ptr<Aggregation> Diff(const Aggregation &next) const noexcept override; | ||
|
|
||
| PointType ToPoint() const noexcept override; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,7 +22,6 @@ class LongSumAggregation : public Aggregation | |
| { | ||
| public: | ||
| LongSumAggregation(bool is_monotonic); | ||
| LongSumAggregation(SumPointData &&); | ||
|
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.
|
||
| LongSumAggregation(const SumPointData &); | ||
|
|
||
| void Aggregate(int64_t value, const PointAttributes &attributes = {}) noexcept override; | ||
|
|
@@ -44,7 +43,6 @@ class DoubleSumAggregation : public Aggregation | |
| { | ||
| public: | ||
| DoubleSumAggregation(bool is_monotonic); | ||
| DoubleSumAggregation(SumPointData &&); | ||
| DoubleSumAggregation(const SumPointData &); | ||
|
|
||
| void Aggregate(int64_t /* value */, const PointAttributes & /* attributes */) noexcept override {} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,7 +5,6 @@ | |
| #include <chrono> | ||
| #include <memory> | ||
| #include <mutex> | ||
| #include <utility> | ||
|
|
||
| #include "opentelemetry/common/spin_lock_mutex.h" | ||
| #include "opentelemetry/common/timestamp.h" | ||
|
|
@@ -28,8 +27,6 @@ LongLastValueAggregation::LongLastValueAggregation() | |
| point_data_.value_ = static_cast<int64_t>(0); | ||
| } | ||
|
|
||
| LongLastValueAggregation::LongLastValueAggregation(LastValuePointData &&data) : point_data_{data} {} | ||
|
|
||
| LongLastValueAggregation::LongLastValueAggregation(const LastValuePointData &data) | ||
| : point_data_{data} | ||
| {} | ||
|
|
@@ -50,12 +47,12 @@ std::unique_ptr<Aggregation> LongLastValueAggregation::Merge( | |
| nostd::get<LastValuePointData>(delta.ToPoint()).sample_ts_.time_since_epoch()) | ||
| { | ||
| LastValuePointData merge_data = nostd::get<LastValuePointData>(ToPoint()); | ||
| return std::unique_ptr<Aggregation>(new LongLastValueAggregation(std::move(merge_data))); | ||
| return std::unique_ptr<Aggregation>(new LongLastValueAggregation(merge_data)); | ||
|
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. Fixes: warning: std::move of the variable 'merge_data' of the trivially-copyable type 'LastValuePointData' has no effect [performance-move-const-arg]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 bellieve it's a mistake of clang. But it's OK to copy 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. Thanks for the review. The class is trivially copyable, but we could consider disabling this warning with clang-tidy and keep the std::move. I'd prefer to keep the warning enabled and remove the moves that don't have any effect. |
||
| } | ||
| else | ||
| { | ||
| LastValuePointData merge_data = nostd::get<LastValuePointData>(delta.ToPoint()); | ||
| return std::unique_ptr<Aggregation>(new LongLastValueAggregation(std::move(merge_data))); | ||
| return std::unique_ptr<Aggregation>(new LongLastValueAggregation(merge_data)); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -65,12 +62,12 @@ std::unique_ptr<Aggregation> LongLastValueAggregation::Diff(const Aggregation &n | |
| nostd::get<LastValuePointData>(next.ToPoint()).sample_ts_.time_since_epoch()) | ||
| { | ||
| LastValuePointData diff_data = nostd::get<LastValuePointData>(ToPoint()); | ||
| return std::unique_ptr<Aggregation>(new LongLastValueAggregation(std::move(diff_data))); | ||
| return std::unique_ptr<Aggregation>(new LongLastValueAggregation(diff_data)); | ||
| } | ||
| else | ||
| { | ||
| LastValuePointData diff_data = nostd::get<LastValuePointData>(next.ToPoint()); | ||
| return std::unique_ptr<Aggregation>(new LongLastValueAggregation(std::move(diff_data))); | ||
| return std::unique_ptr<Aggregation>(new LongLastValueAggregation(diff_data)); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -86,10 +83,6 @@ DoubleLastValueAggregation::DoubleLastValueAggregation() | |
| point_data_.value_ = 0.0; | ||
| } | ||
|
|
||
| DoubleLastValueAggregation::DoubleLastValueAggregation(LastValuePointData &&data) | ||
| : point_data_{data} | ||
| {} | ||
|
|
||
| DoubleLastValueAggregation::DoubleLastValueAggregation(const LastValuePointData &data) | ||
| : point_data_{data} | ||
| {} | ||
|
|
@@ -110,12 +103,12 @@ std::unique_ptr<Aggregation> DoubleLastValueAggregation::Merge( | |
| nostd::get<LastValuePointData>(delta.ToPoint()).sample_ts_.time_since_epoch()) | ||
| { | ||
| LastValuePointData merge_data = nostd::get<LastValuePointData>(ToPoint()); | ||
| return std::unique_ptr<Aggregation>(new DoubleLastValueAggregation(std::move(merge_data))); | ||
| return std::unique_ptr<Aggregation>(new DoubleLastValueAggregation(merge_data)); | ||
| } | ||
| else | ||
| { | ||
| LastValuePointData merge_data = nostd::get<LastValuePointData>(delta.ToPoint()); | ||
| return std::unique_ptr<Aggregation>(new DoubleLastValueAggregation(std::move(merge_data))); | ||
| return std::unique_ptr<Aggregation>(new DoubleLastValueAggregation(merge_data)); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -126,12 +119,12 @@ std::unique_ptr<Aggregation> DoubleLastValueAggregation::Diff( | |
| nostd::get<LastValuePointData>(next.ToPoint()).sample_ts_.time_since_epoch()) | ||
| { | ||
| LastValuePointData diff_data = nostd::get<LastValuePointData>(ToPoint()); | ||
| return std::unique_ptr<Aggregation>(new DoubleLastValueAggregation(std::move(diff_data))); | ||
| return std::unique_ptr<Aggregation>(new DoubleLastValueAggregation(diff_data)); | ||
| } | ||
| else | ||
| { | ||
| LastValuePointData diff_data = nostd::get<LastValuePointData>(next.ToPoint()); | ||
| return std::unique_ptr<Aggregation>(new DoubleLastValueAggregation(std::move(diff_data))); | ||
| return std::unique_ptr<Aggregation>(new DoubleLastValueAggregation(diff_data)); | ||
| } | ||
| } | ||
|
|
||
|
|
||
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.
Fixes
warning: class 'OtlpFileOstreamBackend' defines a non-default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]