Skip to content

Commit

Permalink
Fix "unsafe narrowing" warnings in absl, 5/n.
Browse files Browse the repository at this point in the history
Addresses failures with the following, in some files:
-Wshorten-64-to-32
-Wimplicit-int-conversion
-Wsign-compare
-Wsign-conversion
-Wtautological-unsigned-zero-compare

(This specific CL focuses on .cc files in strings/internal/.)

Bug: chromium:1292951
PiperOrigin-RevId: 468215101
Change-Id: I07fa487bcf2cf62d403489c3be7a5997cdef8987
  • Loading branch information
Abseil Team authored and copybara-github committed Aug 17, 2022
1 parent 934f613 commit fcfc7a6
Show file tree
Hide file tree
Showing 14 changed files with 283 additions and 233 deletions.
8 changes: 4 additions & 4 deletions absl/strings/internal/charconv_bigint.cc
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ int BigUnsigned<max_words>::ReadDigits(const char* begin, const char* end,
// decimal exponent to compensate.
--exponent_adjust;
}
int digit = (*begin - '0');
char digit = (*begin - '0');
--significant_digits;
if (significant_digits == 0 && std::next(begin) != end &&
(digit == 0 || digit == 5)) {
Expand All @@ -255,7 +255,7 @@ int BigUnsigned<max_words>::ReadDigits(const char* begin, const char* end,
// 500000...000000000001 to correctly round up, rather than to nearest.
++digit;
}
queued = 10 * queued + digit;
queued = 10 * queued + static_cast<uint32_t>(digit);
++digits_queued;
if (digits_queued == kMaxSmallPowerOfTen) {
MultiplyBy(kTenToNth[kMaxSmallPowerOfTen]);
Expand Down Expand Up @@ -341,8 +341,8 @@ std::string BigUnsigned<max_words>::ToString() const {
std::string result;
// Build result in reverse order
while (copy.size() > 0) {
int next_digit = copy.DivMod<10>();
result.push_back('0' + next_digit);
uint32_t next_digit = copy.DivMod<10>();
result.push_back('0' + static_cast<char>(next_digit));
}
if (result.empty()) {
result.push_back('0');
Expand Down
4 changes: 2 additions & 2 deletions absl/strings/internal/charconv_parse.cc
Original file line number Diff line number Diff line change
Expand Up @@ -190,11 +190,11 @@ bool IsDigit<16>(char ch) {

template <>
unsigned ToDigit<10>(char ch) {
return ch - '0';
return static_cast<unsigned>(ch - '0');
}
template <>
unsigned ToDigit<16>(char ch) {
return kAsciiToInt[static_cast<unsigned char>(ch)];
return static_cast<unsigned>(kAsciiToInt[static_cast<unsigned char>(ch)]);
}

template <>
Expand Down
5 changes: 3 additions & 2 deletions absl/strings/internal/cord_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,9 @@ class RefcountAndFlags {
}

// Returns the current reference count using acquire semantics.
inline int32_t Get() const {
return count_.load(std::memory_order_acquire) >> kNumFlags;
inline size_t Get() const {
return static_cast<size_t>(count_.load(std::memory_order_acquire) >>
kNumFlags);
}

// Returns whether the atomic integer is 1.
Expand Down
7 changes: 5 additions & 2 deletions absl/strings/internal/cord_rep_btree.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include <cassert>
#include <cstdint>
#include <iostream>
#include <ostream>
#include <string>

#include "absl/base/attributes.h"
Expand Down Expand Up @@ -55,8 +56,10 @@ inline bool exhaustive_validation() {
// Prints the entire tree structure or 'rep'. External callers should
// not specify 'depth' and leave it to its default (0) value.
// Rep may be a CordRepBtree tree, or a SUBSTRING / EXTERNAL / FLAT node.
void DumpAll(const CordRep* rep, bool include_contents, std::ostream& stream,
int depth = 0) {
void DumpAll(const CordRep* rep,
bool include_contents,
std::ostream& stream,
size_t depth = 0) {
// Allow for full height trees + substring -> flat / external nodes.
assert(depth <= CordRepBtree::kMaxDepth + 2);
std::string sharing = const_cast<CordRep*>(rep)->refcount.IsOne()
Expand Down
5 changes: 3 additions & 2 deletions absl/strings/internal/cord_rep_btree.h
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,9 @@ class CordRepBtree : public CordRep {
// local stack variable compared to Cord's current near 400 bytes stack use.
// The maximum `height` value of a node is then `kMaxDepth - 1` as node height
// values start with a value of 0 for leaf nodes.
static constexpr int kMaxDepth = 12;
static constexpr int kMaxHeight = kMaxDepth - 1;
static constexpr size_t kMaxDepth = 12;
// See comments on height() for why this is an int and not a size_t.
static constexpr int kMaxHeight = static_cast<int>(kMaxDepth - 1);

// `Action` defines the action for unwinding changes done at the btree's leaf
// level that need to be propagated up to the parent node(s). Each operation
Expand Down
10 changes: 5 additions & 5 deletions absl/strings/internal/cord_rep_btree_navigator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ CordRepBtreeNavigator::Position CordRepBtreeNavigator::Skip(size_t n) {
// edges that must be skipped.
while (height > 0) {
node = edge->btree();
index_[height] = index;
index_[height] = static_cast<uint8_t>(index);
node_[--height] = node;
index = node->begin();
edge = node->Edge(index);
Expand All @@ -101,7 +101,7 @@ CordRepBtreeNavigator::Position CordRepBtreeNavigator::Skip(size_t n) {
edge = node->Edge(index);
}
}
index_[0] = index;
index_[0] = static_cast<uint8_t>(index);
return {edge, n};
}

Expand All @@ -126,7 +126,7 @@ ReadResult CordRepBtreeNavigator::Read(size_t edge_offset, size_t n) {
do {
length -= edge->length;
while (++index == node->end()) {
index_[height] = index;
index_[height] = static_cast<uint8_t>(index);
if (++height > height_) {
subtree->set_end(subtree_end);
if (length == 0) return {subtree, 0};
Expand Down Expand Up @@ -154,7 +154,7 @@ ReadResult CordRepBtreeNavigator::Read(size_t edge_offset, size_t n) {
// edges that must be read, adding 'down' nodes to `subtree`.
while (height > 0) {
node = edge->btree();
index_[height] = index;
index_[height] = static_cast<uint8_t>(index);
node_[--height] = node;
index = node->begin();
edge = node->Edge(index);
Expand All @@ -178,7 +178,7 @@ ReadResult CordRepBtreeNavigator::Read(size_t edge_offset, size_t n) {
subtree->edges_[subtree_end++] = Substring(edge, 0, length);
}
subtree->set_end(subtree_end);
index_[0] = index;
index_[0] = static_cast<uint8_t>(index);
return {tree, length};
}

Expand Down
13 changes: 8 additions & 5 deletions absl/strings/internal/cordz_info.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ namespace cord_internal {
using ::absl::base_internal::SpinLockHolder;

#ifdef ABSL_INTERNAL_NEED_REDUNDANT_CONSTEXPR_DECL
constexpr int CordzInfo::kMaxStackDepth;
constexpr size_t CordzInfo::kMaxStackDepth;
#endif

ABSL_CONST_INIT CordzInfo::List CordzInfo::global_list_{absl::kConstInit};
Expand Down Expand Up @@ -291,7 +291,7 @@ CordzInfo::MethodIdentifier CordzInfo::GetParentMethod(const CordzInfo* src) {
: src->method_;
}

int CordzInfo::FillParentStack(const CordzInfo* src, void** stack) {
size_t CordzInfo::FillParentStack(const CordzInfo* src, void** stack) {
assert(stack);
if (src == nullptr) return 0;
if (src->parent_stack_depth_) {
Expand All @@ -302,11 +302,14 @@ int CordzInfo::FillParentStack(const CordzInfo* src, void** stack) {
return src->stack_depth_;
}

CordzInfo::CordzInfo(CordRep* rep, const CordzInfo* src,
CordzInfo::CordzInfo(CordRep* rep,
const CordzInfo* src,
MethodIdentifier method)
: rep_(rep),
stack_depth_(absl::GetStackTrace(stack_, /*max_depth=*/kMaxStackDepth,
/*skip_count=*/1)),
stack_depth_(
static_cast<size_t>(absl::GetStackTrace(stack_,
/*max_depth=*/kMaxStackDepth,
/*skip_count=*/1))),
parent_stack_depth_(FillParentStack(src, parent_stack_)),
method_(method),
parent_method_(GetParentMethod(src)),
Expand Down
8 changes: 4 additions & 4 deletions absl/strings/internal/cordz_info.h
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ class ABSL_LOCKABLE CordzInfo : public CordzHandle {
std::atomic<CordzInfo*> head ABSL_GUARDED_BY(mutex){nullptr};
};

static constexpr int kMaxStackDepth = 64;
static constexpr size_t kMaxStackDepth = 64;

explicit CordzInfo(CordRep* rep, const CordzInfo* src,
MethodIdentifier method);
Expand All @@ -216,7 +216,7 @@ class ABSL_LOCKABLE CordzInfo : public CordzHandle {
// `stack_` depending on `parent_stack_` being empty, returning the size of
// the parent stack.
// Returns 0 if `src` is null.
static int FillParentStack(const CordzInfo* src, void** stack);
static size_t FillParentStack(const CordzInfo* src, void** stack);

void ODRCheck() const {
#ifndef NDEBUG
Expand Down Expand Up @@ -244,8 +244,8 @@ class ABSL_LOCKABLE CordzInfo : public CordzHandle {

void* stack_[kMaxStackDepth];
void* parent_stack_[kMaxStackDepth];
const int stack_depth_;
const int parent_stack_depth_;
const size_t stack_depth_;
const size_t parent_stack_depth_;
const MethodIdentifier method_;
const MethodIdentifier parent_method_;
CordzUpdateTracker update_tracker_;
Expand Down
8 changes: 4 additions & 4 deletions absl/strings/internal/cordz_statistics.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,12 @@ struct CordzStatistics {
};

// The size of the cord in bytes. This matches the result of Cord::size().
int64_t size = 0;
size_t size = 0;

// The estimated memory used by the sampled cord. This value matches the
// value as reported by Cord::EstimatedMemoryUsage().
// A value of 0 implies the property has not been recorded.
int64_t estimated_memory_usage = 0;
size_t estimated_memory_usage = 0;

// The effective memory used by the sampled cord, inversely weighted by the
// effective indegree of each allocated node. This is a representation of the
Expand All @@ -59,14 +59,14 @@ struct CordzStatistics {
// by multiple Cord instances, and for cases where a Cord includes the same
// node multiple times (either directly or indirectly).
// A value of 0 implies the property has not been recorded.
int64_t estimated_fair_share_memory_usage = 0;
size_t estimated_fair_share_memory_usage = 0;

// The total number of nodes referenced by this cord.
// For ring buffer Cords, this includes the 'ring buffer' node.
// For btree Cords, this includes all 'CordRepBtree' tree nodes as well as all
// the substring, flat and external nodes referenced by the tree.
// A value of 0 implies the property has not been recorded.
int64_t node_count = 0;
size_t node_count = 0;

// Detailed node counts per type
NodeCounts node_counts;
Expand Down
15 changes: 9 additions & 6 deletions absl/strings/internal/memutil.cc
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,11 @@ size_t memspn(const char* s, size_t slen, const char* accept) {

cont:
c = *p++;
if (slen-- == 0) return p - 1 - s;
if (slen-- == 0)
return static_cast<size_t>(p - 1 - s);
for (spanp = accept; (sc = *spanp++) != '\0';)
if (sc == c) goto cont;
return p - 1 - s;
return static_cast<size_t>(p - 1 - s);
}

size_t memcspn(const char* s, size_t slen, const char* reject) {
Expand All @@ -68,9 +69,10 @@ size_t memcspn(const char* s, size_t slen, const char* reject) {
while (slen-- != 0) {
c = *p++;
for (spanp = reject; (sc = *spanp++) != '\0';)
if (sc == c) return p - 1 - s;
if (sc == c)
return static_cast<size_t>(p - 1 - s);
}
return p - s;
return static_cast<size_t>(p - s);
}

char* mempbrk(const char* s, size_t slen, const char* accept) {
Expand All @@ -97,8 +99,9 @@ const char* memmatch(const char* phaystack, size_t haylen, const char* pneedle,
const char* hayend = phaystack + haylen - neelen + 1;
// A static cast is used here to work around the fact that memchr returns
// a void* on Posix-compliant systems and const void* on Windows.
while ((match = static_cast<const char*>(
memchr(phaystack, pneedle[0], hayend - phaystack)))) {
while (
(match = static_cast<const char*>(memchr(
phaystack, pneedle[0], static_cast<size_t>(hayend - phaystack))))) {
if (memcmp(match, pneedle, neelen) == 0)
return match;
else
Expand Down
35 changes: 19 additions & 16 deletions absl/strings/internal/str_format/arg.cc
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ class IntDigits {
v >>= 3;
} while (v);
start_ = p;
size_ = storage_ + sizeof(storage_) - p;
size_ = static_cast<size_t>(storage_ + sizeof(storage_) - p);
}

// Print the signed or unsigned integer as decimal.
Expand All @@ -86,7 +86,8 @@ class IntDigits {
void PrintAsDec(T v) {
static_assert(std::is_integral<T>::value, "");
start_ = storage_;
size_ = numbers_internal::FastIntToBuffer(v, storage_) - storage_;
size_ = static_cast<size_t>(numbers_internal::FastIntToBuffer(v, storage_) -
storage_);
}

void PrintAsDec(int128 v) {
Expand Down Expand Up @@ -115,7 +116,7 @@ class IntDigits {
if (add_neg) {
*--p = '-';
}
size_ = storage_ + sizeof(storage_) - p;
size_ = static_cast<size_t>(storage_ + sizeof(storage_) - p);
start_ = p;
}

Expand All @@ -138,7 +139,7 @@ class IntDigits {
++p;
}
start_ = p;
size_ = storage_ + sizeof(storage_) - p;
size_ = static_cast<size_t>(storage_ + sizeof(storage_) - p);
}

// Print the unsigned integer as hex using uppercase.
Expand All @@ -154,7 +155,7 @@ class IntDigits {
v >>= 4;
} while (v);
start_ = p;
size_ = storage_ + sizeof(storage_) - p;
size_ = static_cast<size_t>(storage_ + sizeof(storage_) - p);
}

// The printed value including the '-' sign if available.
Expand Down Expand Up @@ -208,10 +209,12 @@ string_view SignColumn(bool neg, const FormatConversionSpecImpl conv) {
return {};
}

bool ConvertCharImpl(unsigned char v, const FormatConversionSpecImpl conv,
FormatSinkImpl *sink) {
bool ConvertCharImpl(char v,
const FormatConversionSpecImpl conv,
FormatSinkImpl* sink) {
size_t fill = 0;
if (conv.width() >= 0) fill = conv.width();
if (conv.width() >= 0)
fill = static_cast<size_t>(conv.width());
ReducePadding(1, &fill);
if (!conv.has_left_flag()) sink->Append(fill, ' ');
sink->Append(1, v);
Expand All @@ -225,7 +228,8 @@ bool ConvertIntImplInnerSlow(const IntDigits &as_digits,
// Print as a sequence of Substrings:
// [left_spaces][sign][base_indicator][zeroes][formatted][right_spaces]
size_t fill = 0;
if (conv.width() >= 0) fill = conv.width();
if (conv.width() >= 0)
fill = static_cast<size_t>(conv.width());

string_view formatted = as_digits.without_neg_or_zero();
ReducePadding(formatted, &fill);
Expand All @@ -236,18 +240,17 @@ bool ConvertIntImplInnerSlow(const IntDigits &as_digits,
string_view base_indicator = BaseIndicator(as_digits, conv);
ReducePadding(base_indicator, &fill);

int precision = conv.precision();
bool precision_specified = precision >= 0;
if (!precision_specified)
precision = 1;
bool precision_specified = conv.precision() >= 0;
size_t precision =
precision_specified ? static_cast<size_t>(conv.precision()) : size_t{1};

if (conv.has_alt_flag() &&
conv.conversion_char() == FormatConversionCharInternal::o) {
// From POSIX description of the '#' (alt) flag:
// "For o conversion, it increases the precision (if necessary) to
// force the first digit of the result to be zero."
if (formatted.empty() || *formatted.begin() != '0') {
int needed = static_cast<int>(formatted.size()) + 1;
size_t needed = formatted.size() + 1;
precision = std::max(precision, needed);
}
}
Expand Down Expand Up @@ -287,7 +290,7 @@ bool ConvertIntArg(T v, const FormatConversionSpecImpl conv,
// FormatConversionChar is declared, but not defined.
switch (static_cast<uint8_t>(conv.conversion_char())) {
case static_cast<uint8_t>(FormatConversionCharInternal::c):
return ConvertCharImpl(static_cast<unsigned char>(v), conv, sink);
return ConvertCharImpl(static_cast<char>(v), conv, sink);

case static_cast<uint8_t>(FormatConversionCharInternal::o):
as_digits.PrintAsOct(static_cast<U>(v));
Expand Down Expand Up @@ -375,7 +378,7 @@ FormatConvertImpl(const char *v, const FormatConversionSpecImpl conv,
len = std::strlen(v);
} else {
// If precision is set, we look for the NUL-terminator on the valid range.
len = std::find(v, v + conv.precision(), '\0') - v;
len = static_cast<size_t>(std::find(v, v + conv.precision(), '\0') - v);
}
return {ConvertStringArg(string_view(v, len), conv, sink)};
}
Expand Down
Loading

0 comments on commit fcfc7a6

Please sign in to comment.