Inline trivial constructors
I was investigating a Chromium merge failure where an unused variable triggered a warning in Chromium but not google3: cl/276485378. It appears that in google3 the compiler will only flag unused variables if they are primitive types, or if the struct/class has an inline constexpr constructor. So I performed a quick audit of our code and made the most obvious examples follow this pattern.
gfe-relnote: inline constructors, no behavior change
PiperOrigin-RevId: 276787382
Change-Id: Ib59b54d390b0e89966d2a8ccac2f4d1eaa478893
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc
index 1a686d9..24faad8 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc
@@ -4277,7 +4277,6 @@
use_tagging_decrypter();
connection_.SetEncrypter(ENCRYPTION_INITIAL,
std::make_unique<TaggingEncrypter>(0x01));
- QuicPacketNumber packet_number;
connection_.SendCryptoStreamData();
// Simulate the retransmission alarm firing and the socket blocking.
diff --git a/quic/core/quic_packet_number.cc b/quic/core/quic_packet_number.cc
index b3009ce..6e6804b 100644
--- a/quic/core/quic_packet_number.cc
+++ b/quic/core/quic_packet_number.cc
@@ -8,15 +8,6 @@
namespace quic {
-QuicPacketNumber::QuicPacketNumber()
- : packet_number_(UninitializedPacketNumber()) {}
-
-QuicPacketNumber::QuicPacketNumber(uint64_t packet_number)
- : packet_number_(packet_number) {
- DCHECK_NE(UninitializedPacketNumber(), packet_number)
- << "Use default constructor for uninitialized packet number";
-}
-
void QuicPacketNumber::Clear() {
packet_number_ = UninitializedPacketNumber();
}
@@ -111,9 +102,4 @@
return os;
}
-// static
-uint64_t QuicPacketNumber::UninitializedPacketNumber() {
- return std::numeric_limits<uint64_t>::max();
-}
-
} // namespace quic
diff --git a/quic/core/quic_packet_number.h b/quic/core/quic_packet_number.h
index 066d49c..8a75149 100644
--- a/quic/core/quic_packet_number.h
+++ b/quic/core/quic_packet_number.h
@@ -21,10 +21,15 @@
class QUIC_EXPORT_PRIVATE QuicPacketNumber {
public:
// Construct an uninitialized packet number.
- QuicPacketNumber();
+ constexpr QuicPacketNumber() : packet_number_(UninitializedPacketNumber()) {}
+
// Construct a packet number from uint64_t. |packet_number| cannot equal the
// sentinel value.
- explicit QuicPacketNumber(uint64_t packet_number);
+ explicit constexpr QuicPacketNumber(uint64_t packet_number)
+ : packet_number_(packet_number) {
+ DCHECK_NE(UninitializedPacketNumber(), packet_number)
+ << "Use default constructor for uninitialized packet number";
+ }
// Packet number becomes uninitialized after calling this function.
void Clear();
@@ -80,7 +85,9 @@
friend inline uint64_t operator-(QuicPacketNumber lhs, QuicPacketNumber rhs);
// The sentinel value representing an uninitialized packet number.
- static uint64_t UninitializedPacketNumber();
+ static constexpr uint64_t UninitializedPacketNumber() {
+ return std::numeric_limits<uint64_t>::max();
+ }
uint64_t packet_number_;
};
diff --git a/quic/core/quic_stream_send_buffer.h b/quic/core/quic_stream_send_buffer.h
index 1bccd0f..a1f7ac7 100644
--- a/quic/core/quic_stream_send_buffer.h
+++ b/quic/core/quic_stream_send_buffer.h
@@ -42,7 +42,8 @@
};
struct QUIC_EXPORT_PRIVATE StreamPendingRetransmission {
- StreamPendingRetransmission(QuicStreamOffset offset, QuicByteCount length)
+ constexpr StreamPendingRetransmission(QuicStreamOffset offset,
+ QuicByteCount length)
: offset(offset), length(length) {}
// Starting offset of this pending retransmission.
diff --git a/quic/core/quic_types.cc b/quic/core/quic_types.cc
index c584714..6ebc4eb 100644
--- a/quic/core/quic_types.cc
+++ b/quic/core/quic_types.cc
@@ -11,9 +11,6 @@
namespace quic {
-QuicConsumedData::QuicConsumedData(size_t bytes_consumed, bool fin_consumed)
- : bytes_consumed(bytes_consumed), fin_consumed(fin_consumed) {}
-
std::ostream& operator<<(std::ostream& os, const QuicConsumedData& s) {
os << "bytes_consumed: " << s.bytes_consumed
<< " fin_consumed: " << s.fin_consumed;
@@ -61,11 +58,6 @@
return "<invalid>";
}
-WriteResult::WriteResult() : status(WRITE_STATUS_ERROR), bytes_written(0) {}
-
-WriteResult::WriteResult(WriteStatus status, int bytes_written_or_error_code)
- : status(status), bytes_written(bytes_written_or_error_code) {}
-
std::ostream& operator<<(std::ostream& os, const WriteResult& s) {
os << "{ status: " << s.status;
if (s.status == WRITE_STATUS_OK) {
diff --git a/quic/core/quic_types.h b/quic/core/quic_types.h
index 0146851..f3299d0 100644
--- a/quic/core/quic_types.h
+++ b/quic/core/quic_types.h
@@ -55,7 +55,8 @@
// A struct for functions which consume data payloads and fins.
struct QUIC_EXPORT_PRIVATE QuicConsumedData {
- QuicConsumedData(size_t bytes_consumed, bool fin_consumed);
+ constexpr QuicConsumedData(size_t bytes_consumed, bool fin_consumed)
+ : bytes_consumed(bytes_consumed), fin_consumed(fin_consumed) {}
// By default, gtest prints the raw bytes of an object. The bool data
// member causes this object to have padding bytes, which causes the
@@ -115,8 +116,10 @@
// A struct used to return the result of write calls including either the number
// of bytes written or the error code, depending upon the status.
struct QUIC_EXPORT_PRIVATE WriteResult {
- WriteResult(WriteStatus status, int bytes_written_or_error_code);
- WriteResult();
+ constexpr WriteResult(WriteStatus status, int bytes_written_or_error_code)
+ : status(status), bytes_written(bytes_written_or_error_code) {}
+
+ constexpr WriteResult() : WriteResult(WRITE_STATUS_ERROR, 0) {}
bool operator==(const WriteResult& other) const {
if (status != other.status) {
@@ -478,9 +481,9 @@
// Information about a newly acknowledged packet.
struct QUIC_EXPORT_PRIVATE AckedPacket {
- AckedPacket(QuicPacketNumber packet_number,
- QuicPacketLength bytes_acked,
- QuicTime receive_timestamp)
+ constexpr AckedPacket(QuicPacketNumber packet_number,
+ QuicPacketLength bytes_acked,
+ QuicTime receive_timestamp)
: packet_number(packet_number),
bytes_acked(bytes_acked),
receive_timestamp(receive_timestamp) {}
diff --git a/quic/core/quic_versions.cc b/quic/core/quic_versions.cc
index 4ac7560..e565a99 100644
--- a/quic/core/quic_versions.cc
+++ b/quic/core/quic_versions.cc
@@ -44,11 +44,6 @@
} // namespace
-ParsedQuicVersion::ParsedQuicVersion(HandshakeProtocol handshake_protocol,
- QuicTransportVersion transport_version)
- : handshake_protocol(handshake_protocol),
- transport_version(transport_version) {}
-
bool ParsedQuicVersion::KnowsWhichDecrypterToUse() const {
return transport_version > QUIC_VERSION_46 ||
handshake_protocol == PROTOCOL_TLS1_3;
diff --git a/quic/core/quic_versions.h b/quic/core/quic_versions.h
index 3e34cce..8522c67 100644
--- a/quic/core/quic_versions.h
+++ b/quic/core/quic_versions.h
@@ -134,10 +134,12 @@
HandshakeProtocol handshake_protocol;
QuicTransportVersion transport_version;
- ParsedQuicVersion(HandshakeProtocol handshake_protocol,
- QuicTransportVersion transport_version);
+ constexpr ParsedQuicVersion(HandshakeProtocol handshake_protocol,
+ QuicTransportVersion transport_version)
+ : handshake_protocol(handshake_protocol),
+ transport_version(transport_version) {}
- ParsedQuicVersion(const ParsedQuicVersion& other)
+ constexpr ParsedQuicVersion(const ParsedQuicVersion& other)
: handshake_protocol(other.handshake_protocol),
transport_version(other.transport_version) {}
diff --git a/quic/test_tools/simple_session_notifier.cc b/quic/test_tools/simple_session_notifier.cc
index 72a23d7..5ba12c6 100644
--- a/quic/test_tools/simple_session_notifier.cc
+++ b/quic/test_tools/simple_session_notifier.cc
@@ -46,7 +46,6 @@
StreamState& stream_state = stream_map_.find(id)->second;
const bool had_buffered_data =
HasBufferedStreamData() || HasBufferedControlFrames();
- QuicConsumedData total_consumed(0, false);
QuicStreamOffset offset = stream_state.bytes_sent;
QUIC_DVLOG(1) << "WriteOrBuffer stream_id: " << id << " [" << offset << ", "
<< offset + data_length << "), fin: " << (state != NO_FIN);
