Merge pull request #2355 from hzeller/feature-20250216-minmax
A few clang-tidy findings addressed
diff --git a/.clang-tidy b/.clang-tidy
index e210319..be00837 100644
--- a/.clang-tidy
+++ b/.clang-tidy
@@ -49,6 +49,7 @@
-readability-isolate-declaration,
-readability-magic-numbers,
-readability-make-member-function-const,
+ -readability-math-missing-parentheses,
-readability-named-parameter,
-readability-qualified-auto,
-readability-redundant-access-specifiers,
diff --git a/verible/common/formatting/align.cc b/verible/common/formatting/align.cc
index 7f9ceb0..e72b5f6 100644
--- a/verible/common/formatting/align.cc
+++ b/verible/common/formatting/align.cc
@@ -1084,7 +1084,7 @@
for (const auto &row : values) {
for (const int delta : row) {
// Only accumulate positive values.
- if (delta > result) result = delta;
+ result = std::max(delta, result);
}
}
return result;
diff --git a/verible/common/formatting/layout-optimizer-internal.h b/verible/common/formatting/layout-optimizer-internal.h
index cd489f7..ed59fe7 100644
--- a/verible/common/formatting/layout-optimizer-internal.h
+++ b/verible/common/formatting/layout-optimizer-internal.h
@@ -617,7 +617,7 @@
//
// Iterator: iterator type that dereferences to LayoutFunction.
template <class Iterator>
- LayoutFunction Choice(const Iterator begin, const Iterator end) const {
+ LayoutFunction Choice(const Iterator &begin, const Iterator &end) const {
static_assert(IsIteratorDereferencingTo<Iterator, LayoutFunction>,
"Iterator's value type must be LayoutFunction.");
const auto lfs = make_container_range(begin, end);
diff --git a/verible/common/formatting/layout-optimizer.cc b/verible/common/formatting/layout-optimizer.cc
index 9428077..1459687 100644
--- a/verible/common/formatting/layout-optimizer.cc
+++ b/verible/common/formatting/layout-optimizer.cc
@@ -402,7 +402,7 @@
if ((segment_it + 1).IsEnd()) continue;
const int column = (segment_it + 1)->column;
CHECK_GE(column, current_column);
- if (column < next_column) next_column = column;
+ next_column = std::min(column, next_column);
}
current_column = next_column;
}
@@ -434,7 +434,7 @@
const int column =
(segment_it + 1).IsEnd() ? kInfinity : segment_it[1].column;
- if (column < next_knot) next_knot = column;
+ next_knot = std::min(column, next_knot);
}
do {
diff --git a/verible/common/lsp/lsp-text-buffer.cc b/verible/common/lsp/lsp-text-buffer.cc
index 9cf7b80..27e4cf4 100644
--- a/verible/common/lsp/lsp-text-buffer.cc
+++ b/verible/common/lsp/lsp-text-buffer.cc
@@ -14,6 +14,7 @@
#include "verible/common/lsp/lsp-text-buffer.h"
+#include <algorithm>
#include <numeric>
#include <string>
#include <string_view>
@@ -86,7 +87,7 @@
if (!str->empty() && str->back() == '\n') --str_end;
if (c.range.start.character > str_end) return false;
- if (end_char > str_end) end_char = str_end;
+ end_char = std::min(end_char, str_end);
if (end_char < c.range.start.character) return false;
document_length_ -= str->length();
diff --git a/verible/common/lsp/message-stream-splitter.cc b/verible/common/lsp/message-stream-splitter.cc
index 3daf864..a3c07d9 100644
--- a/verible/common/lsp/message-stream-splitter.cc
+++ b/verible/common/lsp/message-stream-splitter.cc
@@ -48,12 +48,12 @@
static constexpr std::string_view kEndHeaderMarker = "\r\n\r\n";
static constexpr std::string_view kContentLengthHeader = "Content-Length: ";
- int header_marker_len = kEndHeaderMarker.length();
- auto end_of_header = data.find(kEndHeaderMarker);
+ const size_t header_marker_len = kEndHeaderMarker.length();
+ const size_t end_of_header = data.find(kEndHeaderMarker);
if (end_of_header == std::string_view::npos) return kIncompleteHeader;
// Very dirty search for header - we don't check if starts with line.
- const std::string_view header_content(data.data(), end_of_header);
+ const std::string_view header_content = data.substr(0, end_of_header);
auto found_ContentLength_header = header_content.find(kContentLengthHeader);
if (found_ContentLength_header == std::string_view::npos) {
return kGarbledHeader;
@@ -80,8 +80,8 @@
int body_size = 0;
const int body_offset = ParseHeaderGetBodyOffset(*data, &body_size);
if (body_offset == kGarbledHeader) {
- std::string_view limited_view(
- data->data(), std::min(data->size(), static_cast<size_t>(256)));
+ const std::string_view limited_view =
+ data->substr(0, std::min(data->size(), static_cast<size_t>(256)));
return absl::InvalidArgumentError(
absl::StrCat("No `Content-Length:` header. '",
absl::CEscape(limited_view), "...'"));
@@ -93,8 +93,8 @@
return absl::OkStatus(); // Only insufficient partial buffer available.
}
- std::string_view header(data->data(), body_offset);
- std::string_view body(data->data() + body_offset, body_size);
+ const std::string_view header = data->substr(0, body_offset);
+ const std::string_view body = data->substr(body_offset, body_size);
message_processor_(header, body);
stats_largest_body_ = std::max(stats_largest_body_, body.size());
diff --git a/verible/common/text/macro-definition.cc b/verible/common/text/macro-definition.cc
index d1a32aa..7f795f0 100644
--- a/verible/common/text/macro-definition.cc
+++ b/verible/common/text/macro-definition.cc
@@ -104,7 +104,7 @@
}
}
// Didn't match enum type nor find map entry, so don't substitute.
- return token_info;
+ return token_info; // NOLINT(bugprone-return-const-ref-from-parameter)
}
} // namespace verible
diff --git a/verible/common/text/tree-utils.h b/verible/common/text/tree-utils.h
index a69cfaa..8a18924 100644
--- a/verible/common/text/tree-utils.h
+++ b/verible/common/text/tree-utils.h
@@ -54,11 +54,9 @@
// The following no-op overloads allow SymbolCastToNode() to work with zero
// overhead when the argument type is statically known to be the same.
inline const SyntaxTreeNode &SymbolCastToNode(const SyntaxTreeNode &node) {
- return node;
+ return node; // NOLINT(bugprone-return-const-ref-from-parameter)
}
-inline SyntaxTreeNode &SymbolCastToNode(SyntaxTreeNode &node) { // NOLINT
- return node;
-}
+inline SyntaxTreeNode &SymbolCastToNode(SyntaxTreeNode &node) { return node; }
// Returns a SyntaxTreeLeaf down_casted from a Symbol.
const SyntaxTreeLeaf &SymbolCastToLeaf(const Symbol &);
diff --git a/verible/common/util/container-proxy_test.cc b/verible/common/util/container-proxy_test.cc
index b5766fd..5e8220f 100644
--- a/verible/common/util/container-proxy_test.cc
+++ b/verible/common/util/container-proxy_test.cc
@@ -113,7 +113,7 @@
std::equal(values.begin(), values.end(), other.values.begin());
}
- friend std::ostream &operator<<(std::ostream &s, TestTrace obj) {
+ friend std::ostream &operator<<(std::ostream &s, const TestTrace &obj) {
switch (obj.triggered_method) {
case ContainerProxyEvent::kUnknown:
s << "UNKNOWN";
diff --git a/verible/common/util/container-util.h b/verible/common/util/container-util.h
index b27f12c..85bfa91 100644
--- a/verible/common/util/container-util.h
+++ b/verible/common/util/container-util.h
@@ -43,7 +43,8 @@
template <class KeyType>
struct ValueTypeFromKeyType<KeyType, KeyType> {
const KeyType &operator()(const KeyType &k) const {
- return k; // just forward the key
+ // Just forward the key
+ return k; // NOLINT(bugprone-return-const-ref-from-parameter)
}
};
} // namespace internal
diff --git a/verible/common/util/file-util.cc b/verible/common/util/file-util.cc
index bd94cf6..27a4c6f 100644
--- a/verible/common/util/file-util.cc
+++ b/verible/common/util/file-util.cc
@@ -179,10 +179,10 @@
}
char buffer[4096];
int bytes_read;
- do {
- bytes_read = fread(buffer, 1, sizeof(buffer), stream);
+ while (!ferror(stream) && !feof(stream) &&
+ (bytes_read = fread(buffer, 1, sizeof(buffer), stream)) >= 0) {
content.append(buffer, bytes_read);
- } while (bytes_read > 0);
+ }
fclose(stream);
return content;
diff --git a/verible/common/util/forward.h b/verible/common/util/forward.h
index 9778108..56af125 100644
--- a/verible/common/util/forward.h
+++ b/verible/common/util/forward.h
@@ -31,7 +31,9 @@
// Intentionally restricted to const reference to avoid the surprise
// of modifying a temporary reference.
// See also the two-parameter overload.
- const T &operator()(const T &t) const { return t; }
+ const T &operator()(const T &t) const {
+ return t; // NOLINT(bugprone-return-const-ref-from-parameter)
+ }
// (overload) Constructs a temporary rvalue, when types are different.
// This works with explicit-only constructors and implicit constructors.
diff --git a/verible/verilog/analysis/checkers/constraint-name-style-rule.cc b/verible/verilog/analysis/checkers/constraint-name-style-rule.cc
index c6be316..0c66ab9 100644
--- a/verible/verilog/analysis/checkers/constraint-name-style-rule.cc
+++ b/verible/verilog/analysis/checkers/constraint-name-style-rule.cc
@@ -51,7 +51,7 @@
.desc =
"Check that constraint names follow the required name style "
"specified by a regular expression.",
- .param = {{"pattern", kSuffix.data()}},
+ .param = {{"pattern", std::string(kSuffix)}},
};
return d;
}
diff --git a/verible/verilog/analysis/verilog-linter-configuration.cc b/verible/verilog/analysis/verilog-linter-configuration.cc
index 140e449..4703127 100644
--- a/verible/verilog/analysis/verilog-linter-configuration.cc
+++ b/verible/verilog/analysis/verilog-linter-configuration.cc
@@ -281,7 +281,8 @@
template <typename T>
static absl::StatusOr<std::vector<std::unique_ptr<T>>> CreateRules(
const std::map<analysis::LintRuleId, RuleSetting> &config,
- std::function<std::unique_ptr<T>(const analysis::LintRuleId &)> factory) {
+ const std::function<std::unique_ptr<T>(const analysis::LintRuleId &)>
+ &factory) {
std::vector<std::unique_ptr<T>> rule_instances;
for (const auto &rule_pair : config) {
const RuleSetting &setting = rule_pair.second;
diff --git a/verible/verilog/formatting/token-annotator.cc b/verible/verilog/formatting/token-annotator.cc
index f0f97e7..d4af24a 100644
--- a/verible/verilog/formatting/token-annotator.cc
+++ b/verible/verilog/formatting/token-annotator.cc
@@ -240,9 +240,7 @@
}
int spaces = right.OriginalLeadingSpaces().length();
- if (spaces > 1) {
- spaces = 1;
- }
+ spaces = std::min(spaces, 1);
return {spaces, "Limit <= 1 space before binary operator inside []."};
}
if (left.format_token_enum == FormatTokenType::binary_operator &&
diff --git a/verible/verilog/tools/ls/autoexpand.cc b/verible/verilog/tools/ls/autoexpand.cc
index c1ab6b3..c2683e2 100644
--- a/verible/verilog/tools/ls/autoexpand.cc
+++ b/verible/verilog/tools/ls/autoexpand.cc
@@ -548,10 +548,8 @@
template <>
Dimension MaxDimension(const DimensionRange first,
const DimensionRange second) {
- int64_t max = std::max(std::max(first.msb, first.lsb),
- std::max(second.msb, second.lsb));
- int64_t min = std::min(std::min(first.msb, first.lsb),
- std::min(second.msb, second.lsb));
+ int64_t max = std::max({first.msb, first.lsb, second.msb, second.lsb});
+ int64_t min = std::min({first.msb, first.lsb, second.msb, second.lsb});
if (first.msb >= first.lsb) {
return DimensionRange{.msb = max, .lsb = min};
}
