Re-enable asserts in headers
Several parts of the code had commented-out asserts due to DCHECK not
working in headers or constexpr functions. This updates those to use the
pw_assert light API.
Fixes: 246
Change-Id: I92899045f9dd2188af8d5bbd13472f00a5431b76
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/22820
Commit-Queue: Alexei Frolov <frolv@google.com>
Reviewed-by: Ewout van Bekkum <ewout@google.com>
Reviewed-by: Wyatt Hepler <hepler@google.com>
Reviewed-by: Keir Mierle <keir@google.com>
diff --git a/pw_assert/docs.rst b/pw_assert/docs.rst
index 73f0659..0cd72cf 100644
--- a/pw_assert/docs.rst
+++ b/pw_assert/docs.rst
@@ -461,7 +461,7 @@
Unlike the ``PW_CHECK_*()`` suite of macros, ``PW_ASSERT()`` and
``PW_DASSERT()`` capture no rich information like line numbers, the file,
expression arguments, or the stringified expression. Use these macros **only
- when absolutely necessary**--in headers, constexr contexts, or in rare cases
+ when absolutely necessary**---in headers, constexpr contexts, or in rare cases
where the call site overhead of a full PW_CHECK must be avoided.
Use ``PW_CHECK_*()`` whenever possible.
diff --git a/pw_blob_store/blob_store.cc b/pw_blob_store/blob_store.cc
index bd68285..39d259c 100644
--- a/pw_blob_store/blob_store.cc
+++ b/pw_blob_store/blob_store.cc
@@ -16,6 +16,7 @@
#include <algorithm>
+#include "pw_assert/assert.h"
#include "pw_log/log.h"
#include "pw_status/try.h"
diff --git a/pw_hdlc/public/pw_hdlc/decoder.h b/pw_hdlc/public/pw_hdlc/decoder.h
index a9e3c03..f6f5707 100644
--- a/pw_hdlc/public/pw_hdlc/decoder.h
+++ b/pw_hdlc/public/pw_hdlc/decoder.h
@@ -19,6 +19,7 @@
#include <cstring>
#include <functional> // std::invoke
+#include "pw_assert/light.h"
#include "pw_bytes/span.h"
#include "pw_checksum/crc32.h"
#include "pw_result/result.h"
@@ -55,10 +56,7 @@
// Creates a Frame with the specified data. The data MUST be valid frame data
// with a verified frame check sequence.
constexpr Frame(uint64_t address, std::byte control, ConstByteSpan data)
- : data_(data), address_(address), control_(control) {
- // TODO(pwbug/246): Use PW_DASSERT when available.
- // PW_DASSERT(data.size() >= kMinSizeBytes);
- }
+ : data_(data), address_(address), control_(control) {}
ConstByteSpan data_;
uint64_t address_;
diff --git a/pw_kvs/public/pw_kvs/flash_memory.h b/pw_kvs/public/pw_kvs/flash_memory.h
index 28fd043..8a3d35d0 100644
--- a/pw_kvs/public/pw_kvs/flash_memory.h
+++ b/pw_kvs/public/pw_kvs/flash_memory.h
@@ -166,7 +166,6 @@
FlashPartition::Address address_;
};
- // TODO(pwbug/246): This can be constexpr when tokenized asserts are fixed.
FlashPartition(
FlashMemory* flash,
uint32_t start_sector_index,
@@ -175,7 +174,6 @@
PartitionPermission permission = PartitionPermission::kReadAndWrite);
// Creates a FlashPartition that uses the entire flash with its alignment.
- // TODO(pwbug/246): This can be constexpr when tokenized asserts are fixed.
FlashPartition(FlashMemory* flash)
: FlashPartition(
flash, 0, flash->sector_count(), flash->alignment_bytes()) {}
@@ -301,4 +299,4 @@
};
} // namespace kvs
-} // namespace pw
\ No newline at end of file
+} // namespace pw
diff --git a/pw_log_multisink/log_queue.cc b/pw_log_multisink/log_queue.cc
index 8fbc42c..48f9661 100644
--- a/pw_log_multisink/log_queue.cc
+++ b/pw_log_multisink/log_queue.cc
@@ -14,6 +14,7 @@
#include "pw_log_multisink/log_queue.h"
+#include "pw_assert/assert.h"
#include "pw_log/levels.h"
#include "pw_log_proto/log.pwpb.h"
#include "pw_protobuf/wire_format.h"
diff --git a/pw_result/public/pw_result/result.h b/pw_result/public/pw_result/result.h
index ba78e8a..e529344 100644
--- a/pw_result/public/pw_result/result.h
+++ b/pw_result/public/pw_result/result.h
@@ -15,7 +15,7 @@
#include <algorithm>
-#include "pw_assert/assert.h"
+#include "pw_assert/light.h"
#include "pw_status/status.h"
namespace pw {
@@ -33,10 +33,12 @@
constexpr Result(std::in_place_t, Args&&... args)
: value_(std::forward<Args>(args)...), status_(OkStatus()) {}
- // TODO(pwbug/246): This can be constexpr when tokenized asserts are fixed.
- Result(Status status) : status_(status) { PW_CHECK(status_ != OkStatus()); }
- // TODO(pwbug/246): This can be constexpr when tokenized asserts are fixed.
- Result(Status::Code code) : status_(code) { PW_CHECK(status_ != OkStatus()); }
+ constexpr Result(Status status) : dummy_({}), status_(status) {
+ PW_ASSERT(!status_.ok());
+ }
+ constexpr Result(Status::Code code) : dummy_({}), status_(code) {
+ PW_ASSERT(!status_.ok());
+ }
constexpr Result(const Result&) = default;
constexpr Result& operator=(const Result&) = default;
@@ -47,21 +49,18 @@
constexpr Status status() const { return status_; }
constexpr bool ok() const { return status_.ok(); }
- // TODO(pwbug/246): This can be constexpr when tokenized asserts are fixed.
- T& value() & {
- PW_CHECK_OK(status_);
+ constexpr T& value() & {
+ PW_ASSERT(status_.ok());
return value_;
}
- // TODO(pwbug/246): This can be constexpr when tokenized asserts are fixed.
- const T& value() const& {
- PW_CHECK_OK(status_);
+ constexpr const T& value() const& {
+ PW_ASSERT(status_.ok());
return value_;
}
- // TODO(pwbug/246): This can be constexpr when tokenized asserts are fixed.
- T&& value() && {
- PW_CHECK_OK(status_);
+ constexpr T&& value() && {
+ PW_ASSERT(status_.ok());
return std::move(value_);
}
@@ -86,8 +85,13 @@
}
private:
+ struct Dummy {};
+
union {
T value_;
+
+ // Ensure that there is always a trivial constructor for the union.
+ Dummy dummy_;
};
Status status_;
};
diff --git a/pw_rpc/public/pw_rpc/channel.h b/pw_rpc/public/pw_rpc/channel.h
index 6d87020..a8823ea 100644
--- a/pw_rpc/public/pw_rpc/channel.h
+++ b/pw_rpc/public/pw_rpc/channel.h
@@ -17,7 +17,7 @@
#include <span>
#include <type_traits>
-#include "pw_assert/assert.h"
+#include "pw_assert/light.h"
#include "pw_status/status.h"
namespace pw::rpc {
@@ -96,13 +96,11 @@
protected:
constexpr Channel(uint32_t id, ChannelOutput* output)
: id_(id), output_(output), client_(nullptr) {
- // TODO(pwbug/246): Use PW_ASSERT when that is available.
- // PW_ASSERT(id != kUnassignedChannelId);
+ PW_ASSERT(id != kUnassignedChannelId);
}
ChannelOutput& output() const {
- // TODO(pwbug/246): Use PW_ASSERT when that is available.
- // PW_ASSERT(output_ != nullptr);
+ PW_ASSERT(output_ != nullptr);
return *output_;
}
diff --git a/pw_rpc/public/pw_rpc/internal/base_client_call.h b/pw_rpc/public/pw_rpc/internal/base_client_call.h
index a6b800b..24acafc 100644
--- a/pw_rpc/public/pw_rpc/internal/base_client_call.h
+++ b/pw_rpc/public/pw_rpc/internal/base_client_call.h
@@ -13,7 +13,7 @@
// the License.
#pragma once
-#include "pw_assert/assert.h"
+#include "pw_assert/light.h"
#include "pw_containers/intrusive_list.h"
#include "pw_rpc/internal/channel.h"
#include "pw_rpc/internal/packet.h"
@@ -37,9 +37,7 @@
method_id_(method_id),
handler_(handler),
active_(true) {
- // TODO(pwbug/246): Use PW_ASSERT when that is available.
- // PW_ASSERT(channel_ != nullptr);
-
+ PW_ASSERT(channel_ != nullptr);
Register();
}
diff --git a/pw_rpc/public/pw_rpc/internal/channel.h b/pw_rpc/public/pw_rpc/internal/channel.h
index 3e3350e..077ed6a 100644
--- a/pw_rpc/public/pw_rpc/internal/channel.h
+++ b/pw_rpc/public/pw_rpc/internal/channel.h
@@ -15,6 +15,7 @@
#include <span>
+#include "pw_assert/assert.h"
#include "pw_rpc/channel.h"
#include "pw_status/status.h"
