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);