From 9ec5176041f28df36d20668b642c5533e8fb1d9b Mon Sep 17 00:00:00 2001 From: John Chadwick Date: Thu, 17 Apr 2025 20:43:14 -0400 Subject: [PATCH] Avoid including time_util in time.h Unfortunately, due to https://github.com/protocolbuffers/protobuf/issues/21301, including time_util inside of a header will cause a lot of headaches on MSVC/Windows. While that can and should be resolved separately of this, in the interim there will be a lot of people using protobuf versions without a fix for this for the foreseeable future. This leaves us with only a couple of obvious options: either inline the constants, or move the function definitions into time.cc. I chose the latter since it doesn't seem like there is likely to be a substantial performance hit for this, as these are only used in a couple of locations that don't appear to be very hot paths. --- internal/time.cc | 33 +++++++++++++++++++++++++++++++++ internal/time.h | 43 +++++++------------------------------------ 2 files changed, 40 insertions(+), 36 deletions(-) diff --git a/internal/time.cc b/internal/time.cc index fb48dd164..d7f7573e6 100644 --- a/internal/time.cc +++ b/internal/time.cc @@ -24,6 +24,7 @@ #include "absl/strings/string_view.h" #include "absl/time/time.h" #include "internal/status_macros.h" +#include "google/protobuf/util/time_util.h" namespace cel::internal { @@ -36,6 +37,38 @@ std::string RawFormatTimestamp(absl::Time timestamp) { } // namespace +absl::Duration MaxDuration() { +// This currently supports a larger range then the current CEL spec. The +// intent is to widen the CEL spec to support the larger range and match +// google.protobuf.Duration from protocol buffer messages, which this +// implementation currently supports. +// TODO(google/cel-spec/issues/214): revisit +return absl::Seconds(google::protobuf::util::TimeUtil::kDurationMaxSeconds) + + absl::Nanoseconds(google::protobuf::util::TimeUtil::kDurationMaxNanoseconds); +} + +absl::Duration MinDuration() { +// This currently supports a larger range then the current CEL spec. The +// intent is to widen the CEL spec to support the larger range and match +// google.protobuf.Duration from protocol buffer messages, which this +// implementation currently supports. +// TODO(google/cel-spec/issues/214): revisit +return absl::Seconds(google::protobuf::util::TimeUtil::kDurationMinSeconds) + + absl::Nanoseconds(google::protobuf::util::TimeUtil::kDurationMinNanoseconds); +} + +absl::Time MaxTimestamp() { + return absl::UnixEpoch() + + absl::Seconds(google::protobuf::util::TimeUtil::kTimestampMaxSeconds) + + absl::Nanoseconds(google::protobuf::util::TimeUtil::kTimestampMaxNanoseconds); +} + +absl::Time MinTimestamp() { + return absl::UnixEpoch() + + absl::Seconds(google::protobuf::util::TimeUtil::kTimestampMinSeconds) + + absl::Nanoseconds(google::protobuf::util::TimeUtil::kTimestampMinNanoseconds); +} + absl::Status ValidateDuration(absl::Duration duration) { if (duration < MinDuration()) { return absl::InvalidArgumentError( diff --git a/internal/time.h b/internal/time.h index 38b5c076e..402cb6c8b 100644 --- a/internal/time.h +++ b/internal/time.h @@ -21,45 +21,16 @@ #include "absl/status/statusor.h" #include "absl/strings/string_view.h" #include "absl/time/time.h" -#include "google/protobuf/util/time_util.h" namespace cel::internal { - inline absl::Duration - MaxDuration() { - // This currently supports a larger range then the current CEL spec. The - // intent is to widen the CEL spec to support the larger range and match - // google.protobuf.Duration from protocol buffer messages, which this - // implementation currently supports. - // TODO(google/cel-spec/issues/214): revisit - return absl::Seconds(google::protobuf::util::TimeUtil::kDurationMaxSeconds) + - absl::Nanoseconds(google::protobuf::util::TimeUtil::kDurationMaxNanoseconds); -} - - inline absl::Duration - MinDuration() { - // This currently supports a larger range then the current CEL spec. The - // intent is to widen the CEL spec to support the larger range and match - // google.protobuf.Duration from protocol buffer messages, which this - // implementation currently supports. - // TODO(google/cel-spec/issues/214): revisit - return absl::Seconds(google::protobuf::util::TimeUtil::kDurationMinSeconds) + - absl::Nanoseconds(google::protobuf::util::TimeUtil::kDurationMinNanoseconds); -} - - inline absl::Time - MaxTimestamp() { - return absl::UnixEpoch() + - absl::Seconds(google::protobuf::util::TimeUtil::kTimestampMaxSeconds) + - absl::Nanoseconds(google::protobuf::util::TimeUtil::kTimestampMaxNanoseconds); -} - - inline absl::Time - MinTimestamp() { - return absl::UnixEpoch() + - absl::Seconds(google::protobuf::util::TimeUtil::kTimestampMinSeconds) + - absl::Nanoseconds(google::protobuf::util::TimeUtil::kTimestampMinNanoseconds); -} +absl::Duration MaxDuration(); + +absl::Duration MinDuration(); + +absl::Time MaxTimestamp(); + +absl::Time MinTimestamp(); absl::Status ValidateDuration(absl::Duration duration);