Skip to content

Commit

Permalink
Replace std::isnan/isinf/isfinite UB overloads for time classes with …
Browse files Browse the repository at this point in the history
…member functions

https://bugs.webkit.org/show_bug.cgi?id=264241
rdar://problem/117986797

Reviewed by Dan Glastonbury.

Time types under WTF (MonotonicTime, etc.) add overloads of std::isnan, isinf, and isfinite,
but it is generally UB to add declarations&definitions to `namespace std`:
https://en.cppreference.com/w/cpp/language/extending_std

Instead, these functions should be class member functions, or free functions outside of `std`.
I don't believe these functions are used in generic code, so I think member functions are more
appropriate here.

* Source/JavaScriptCore/inspector/agents/InspectorHeapAgent.cpp:
(Inspector::InspectorHeapAgent::didGarbageCollect):
* Source/WTF/wtf/ApproximateTime.cpp:
(WTF::ApproximateTime::approximateWallTime const):
(WTF::ApproximateTime::approximateMonotonicTime const):
* Source/WTF/wtf/ApproximateTime.h:
(WTF::ApproximateTime::MarkableTraits::isEmptyValue):
(std::isnan): Deleted.
(std::isinf): Deleted.
(std::isfinite): Deleted.
* Source/WTF/wtf/GenericTimeMixin.h:
(WTF::GenericTimeMixin::isNaN const):
(WTF::GenericTimeMixin::isInfinity const):
(WTF::GenericTimeMixin::isFinite const):
(WTF::GenericTimeMixin::timePointFromNow):
* Source/WTF/wtf/MonotonicTime.cpp:
(WTF::MonotonicTime::approximateWallTime const):
* Source/WTF/wtf/MonotonicTime.h:
(std::isnan): Deleted.
(std::isinf): Deleted.
(std::isfinite): Deleted.
* Source/WTF/wtf/Seconds.h:
(WTF::Seconds::MarkableTraits::isEmptyValue):
(std::isnan): Deleted.
(std::isinf): Deleted.
(std::isfinite): Deleted.
* Source/WTF/wtf/Stopwatch.h:
(WTF::Stopwatch::start):
(WTF::Stopwatch::stop):
(WTF::Stopwatch::fromMonotonicTime const):
* Source/WTF/wtf/TimeWithDynamicClockType.cpp:
(WTF::hasElapsed):
* Source/WTF/wtf/TimeWithDynamicClockType.h:
(std::isnan): Deleted.
(std::isinf): Deleted.
(std::isfinite): Deleted.
* Source/WTF/wtf/WallTime.cpp:
(WTF::WallTime::approximateMonotonicTime const):
* Source/WTF/wtf/WallTime.h:
(WTF::WallTime::MarkableTraits::isEmptyValue):
(std::isnan): Deleted.
(std::isinf): Deleted.
(std::isfinite): Deleted.
* Source/WTF/wtf/WorkerPool.cpp:
(WTF::WorkerPool::shouldSleep):
* Source/WTF/wtf/posix/ThreadingPOSIX.cpp:
(WTF::ThreadCondition::timedWait):
* Source/WTF/wtf/win/ThreadingWin.cpp:
(WTF::absoluteTimeToWaitTimeoutInterval):
* Source/WebCore/Modules/applepay/ApplePaySession.cpp:
(WebCore::ApplePayDeferredPaymentRequest::validate const):
* Source/WebCore/Modules/applepay/cocoa/PaymentSummaryItemsCocoa.mm:
(WebCore::platformRecurringSummaryItem):
(WebCore::platformDeferredSummaryItem):
* Source/WebCore/bindings/IDLTypes.h:
(WebCore::IDLDate::isNullValue):
* Source/WebCore/bindings/js/IDBBindingUtilities.cpp:
(WebCore::createIDBKeyFromValue):
* Source/WebCore/fileapi/FileReader.cpp:
(WebCore::FileReader::didReceiveData):
* Source/WebCore/inspector/InspectorCanvas.cpp:
(WebCore::InspectorCanvas::finalizeFrame):
* Source/WebCore/page/LocalDOMWindow.cpp:
(WebCore::LocalDOMWindow::consumeLastActivationIfNecessary):
* Source/WebCore/page/cocoa/ResourceUsageOverlayCocoa.mm:
(WebCore::gcTimerString):
* Source/WebCore/page/linux/ResourceUsageOverlayLinux.cpp:
(WebCore::gcTimerString):
* Source/WebCore/platform/Timer.cpp:
(WebCore::TimerBase::setNextFireTime):
(WebCore::TimerBase::nextUnalignedFireInterval const):
* Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:
(CachedResourceStreamingClient::dataReceived):
* Source/WebCore/platform/mediastream/cocoa/DisplayCaptureSourceCocoa.cpp:
(WebCore::DisplayCaptureSourceCocoa::elapsedTime):
* Source/WebCore/platform/mock/MockRealtimeAudioSource.cpp:
(WebCore::MockRealtimeAudioSource::tick):
* Source/WebCore/platform/mock/MockRealtimeVideoSource.cpp:
(WebCore::MockRealtimeVideoSource::elapsedTime):
* Source/WebKit/Platform/IPC/Timeout.h:
(IPC::Timeout::Timeout):
(IPC::Timeout::isInfinity const):
* Source/WebKit/Platform/cocoa/CocoaHelpers.mm:
(WebKit::toAPI):
* Source/WebKit/Shared/ApplePay/cocoa/DeferredPaymentRequestCocoa.mm:
(WebKit::platformDeferredPaymentRequest):
* Tools/TestWebKitAPI/Tests/WTF/Time.cpp:
(TestWebKitAPI::TEST):

Canonical link: https://commits.webkit.org/270366@main
  • Loading branch information
squelart committed Nov 8, 2023
1 parent 286cbf7 commit acfb618
Show file tree
Hide file tree
Showing 33 changed files with 55 additions and 138 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ void InspectorHeapAgent::didGarbageCollect(CollectionScope scope)
return;
}

if (std::isnan(m_gcStartTime)) {
if (m_gcStartTime.isNaN()) {
// We were not enabled when the GC began.
return;
}
Expand Down
4 changes: 2 additions & 2 deletions Source/WTF/wtf/ApproximateTime.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,14 @@ namespace WTF {

WallTime ApproximateTime::approximateWallTime() const
{
if (std::isinf(*this))
if (isInfinity())
return WallTime::fromRawSeconds(m_value);
return *this - now() + WallTime::now();
}

MonotonicTime ApproximateTime::approximateMonotonicTime() const
{
if (std::isinf(*this))
if (isInfinity())
return MonotonicTime::fromRawSeconds(m_value);
return *this - now() + MonotonicTime::now();
}
Expand Down
21 changes: 1 addition & 20 deletions Source/WTF/wtf/ApproximateTime.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ static_assert(sizeof(ApproximateTime) == sizeof(double));
struct ApproximateTime::MarkableTraits {
static bool isEmptyValue(ApproximateTime time)
{
return std::isnan(time.m_value);
return time.isNaN();
}

static constexpr ApproximateTime emptyValue()
Expand All @@ -80,23 +80,4 @@ struct ApproximateTime::MarkableTraits {

} // namespace WTF

namespace std {

inline bool isnan(WTF::ApproximateTime time)
{
return std::isnan(time.secondsSinceEpoch().value());
}

inline bool isinf(WTF::ApproximateTime time)
{
return std::isinf(time.secondsSinceEpoch().value());
}

inline bool isfinite(WTF::ApproximateTime time)
{
return std::isfinite(time.secondsSinceEpoch().value());
}

} // namespace std

using WTF::ApproximateTime;
6 changes: 5 additions & 1 deletion Source/WTF/wtf/GenericTimeMixin.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ class GenericTimeMixin {
static constexpr DerivedTime infinity() { return fromRawSeconds(std::numeric_limits<double>::infinity()); }
static constexpr DerivedTime nan() { return fromRawSeconds(std::numeric_limits<double>::quiet_NaN()); }

bool isNaN() const { return std::isnan(m_value); }
bool isInfinity() const { return std::isinf(m_value); }
bool isFinite() const { return std::isfinite(m_value); }

constexpr Seconds secondsSinceEpoch() const { return Seconds(m_value); }

explicit constexpr operator bool() const { return !!m_value; }
Expand Down Expand Up @@ -113,7 +117,7 @@ class GenericTimeMixin {

static constexpr DerivedTime timePointFromNow(Seconds relativeTimeFromNow)
{
if (std::isinf(relativeTimeFromNow))
if (relativeTimeFromNow.isInfinity())
return DerivedTime::fromRawSeconds(relativeTimeFromNow.value());
return DerivedTime::now() + relativeTimeFromNow;
}
Expand Down
2 changes: 1 addition & 1 deletion Source/WTF/wtf/MonotonicTime.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ namespace WTF {

WallTime MonotonicTime::approximateWallTime() const
{
if (std::isinf(*this))
if (isInfinity())
return WallTime::fromRawSeconds(m_value);
return *this - now() + WallTime::now();
}
Expand Down
19 changes: 0 additions & 19 deletions Source/WTF/wtf/MonotonicTime.h
Original file line number Diff line number Diff line change
Expand Up @@ -81,22 +81,3 @@ struct MonotonicTime::MarkableTraits {
};

} // namespace WTF

namespace std {

inline bool isnan(WTF::MonotonicTime time)
{
return std::isnan(time.secondsSinceEpoch().value());
}

inline bool isinf(WTF::MonotonicTime time)
{
return std::isinf(time.secondsSinceEpoch().value());
}

inline bool isfinite(WTF::MonotonicTime time)
{
return std::isfinite(time.secondsSinceEpoch().value());
}

} // namespace std
25 changes: 5 additions & 20 deletions Source/WTF/wtf/Seconds.h
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,10 @@ class Seconds final {
return Seconds(std::numeric_limits<double>::quiet_NaN());
}

bool isNaN() const { return std::isnan(m_value); }
bool isInfinity() const { return std::isinf(m_value); }
bool isFinite() const { return std::isfinite(m_value); }

explicit constexpr operator bool() const { return !!m_value; }

constexpr Seconds operator+(Seconds other) const
Expand Down Expand Up @@ -228,7 +232,7 @@ WTF_EXPORT_PRIVATE void sleep(Seconds);
struct Seconds::MarkableTraits {
static bool isEmptyValue(Seconds seconds)
{
return std::isnan(seconds.value());
return seconds.isNaN();
}

static constexpr Seconds emptyValue()
Expand Down Expand Up @@ -317,24 +321,5 @@ WTF_EXPORT_PRIVATE TextStream& operator<<(TextStream&, Seconds);

using WTF::sleep;

namespace std {

inline bool isnan(WTF::Seconds seconds)
{
return std::isnan(seconds.value());
}

inline bool isinf(WTF::Seconds seconds)
{
return std::isinf(seconds.value());
}

inline bool isfinite(WTF::Seconds seconds)
{
return std::isfinite(seconds.value());
}

} // namespace std

using namespace WTF::seconds_literals;
using WTF::Seconds;
8 changes: 4 additions & 4 deletions Source/WTF/wtf/Stopwatch.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class Stopwatch final : public RefCounted<Stopwatch> {

std::optional<Seconds> fromMonotonicTime(MonotonicTime) const;

bool isActive() const { return !std::isnan(m_lastStartTime); }
bool isActive() const { return !m_lastStartTime.isNaN(); }
private:
Stopwatch() { reset(); }

Expand All @@ -66,14 +66,14 @@ inline void Stopwatch::reset()

inline void Stopwatch::start()
{
ASSERT_WITH_MESSAGE(std::isnan(m_lastStartTime), "Tried to start the stopwatch, but it is already running.");
ASSERT_WITH_MESSAGE(m_lastStartTime.isNaN(), "Tried to start the stopwatch, but it is already running.");

m_lastStartTime = MonotonicTime::now();
}

inline void Stopwatch::stop()
{
ASSERT_WITH_MESSAGE(!std::isnan(m_lastStartTime), "Tried to stop the stopwatch, but it is not running.");
ASSERT_WITH_MESSAGE(!m_lastStartTime.isNaN(), "Tried to stop the stopwatch, but it is not running.");

auto stopTime = MonotonicTime::now();
m_pastInternals.append({ m_lastStartTime, stopTime });
Expand Down Expand Up @@ -105,7 +105,7 @@ inline Seconds Stopwatch::elapsedTimeSince(MonotonicTime timeStamp) const

inline std::optional<Seconds> Stopwatch::fromMonotonicTime(MonotonicTime timeStamp) const
{
if (!std::isnan(m_lastStartTime) && m_lastStartTime < timeStamp)
if (!m_lastStartTime.isNaN() && m_lastStartTime < timeStamp)
return Stopwatch::elapsedTimeSince(timeStamp);

Seconds elapsedTime;
Expand Down
2 changes: 1 addition & 1 deletion Source/WTF/wtf/TimeWithDynamicClockType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ bool hasElapsed(const TimeWithDynamicClockType& time)
// Avoid doing now().
if (!(time > time.withSameClockAndRawSeconds(0)))
return true;
if (std::isinf(time.secondsSinceEpoch().value()))
if (time.secondsSinceEpoch().isInfinity())
return false;

return time <= time.nowWithSameClock();
Expand Down
23 changes: 4 additions & 19 deletions Source/WTF/wtf/TimeWithDynamicClockType.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,10 @@ class TimeWithDynamicClockType final {
WTF_EXPORT_PRIVATE WallTime approximateWallTime() const;
WTF_EXPORT_PRIVATE MonotonicTime approximateMonotonicTime() const;

bool isNaN() const { return std::isnan(m_value); }
bool isInfinity() const { return std::isinf(m_value); }
bool isFinite() const { return std::isfinite(m_value); }

explicit operator bool() const { return !!m_value; }

TimeWithDynamicClockType operator+(Seconds other) const
Expand Down Expand Up @@ -137,25 +141,6 @@ WTF_EXPORT_PRIVATE bool hasElapsed(const TimeWithDynamicClockType&);

} // namespace WTF

namespace std {

inline bool isnan(WTF::TimeWithDynamicClockType time)
{
return std::isnan(time.secondsSinceEpoch().value());
}

inline bool isinf(WTF::TimeWithDynamicClockType time)
{
return std::isinf(time.secondsSinceEpoch().value());
}

inline bool isfinite(WTF::TimeWithDynamicClockType time)
{
return std::isfinite(time.secondsSinceEpoch().value());
}

} // namespace std

using WTF::TimeWithDynamicClockType;
using WTF::hasElapsed;
using WTF::sleep;
2 changes: 1 addition & 1 deletion Source/WTF/wtf/WallTime.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ namespace WTF {

MonotonicTime WallTime::approximateMonotonicTime() const
{
if (std::isinf(*this))
if (isInfinity())
return MonotonicTime::fromRawSeconds(m_value);
return *this - now() + MonotonicTime::now();
}
Expand Down
21 changes: 1 addition & 20 deletions Source/WTF/wtf/WallTime.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ static_assert(sizeof(WallTime) == sizeof(double));
struct WallTime::MarkableTraits {
static bool isEmptyValue(WallTime time)
{
return std::isnan(time.m_value);
return time.isNaN();
}

static constexpr WallTime emptyValue()
Expand All @@ -80,23 +80,4 @@ WTF_EXPORT_PRIVATE Int128 currentTimeInNanoseconds();

} // namespace WTF

namespace std {

inline bool isnan(WTF::WallTime time)
{
return std::isnan(time.secondsSinceEpoch().value());
}

inline bool isinf(WTF::WallTime time)
{
return std::isinf(time.secondsSinceEpoch().value());
}

inline bool isfinite(WTF::WallTime time)
{
return std::isfinite(time.secondsSinceEpoch().value());
}

} // namespace std

using WTF::WallTime;
4 changes: 2 additions & 2 deletions Source/WTF/wtf/WorkerPool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -107,11 +107,11 @@ WorkerPool::~WorkerPool()

bool WorkerPool::shouldSleep(const AbstractLocker&)
{
if (m_timeout > 0_s && std::isinf(m_timeout))
if (m_timeout > 0_s && m_timeout.isInfinity())
return false;

MonotonicTime currentTime = MonotonicTime::now();
if (std::isnan(m_lastTimeoutTime) || (currentTime >= (m_lastTimeoutTime + m_timeout))) {
if (m_lastTimeoutTime.isNaN() || (currentTime >= (m_lastTimeoutTime + m_timeout))) {
m_lastTimeoutTime = currentTime;
return true;
}
Expand Down
2 changes: 1 addition & 1 deletion Source/WTF/wtf/posix/ThreadingPOSIX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -653,7 +653,7 @@ void ThreadCondition::wait(Mutex& mutex)

bool ThreadCondition::timedWait(Mutex& mutex, WallTime absoluteTime)
{
if (std::isinf(absoluteTime)) {
if (absoluteTime.isInfinity()) {
if (absoluteTime == -WallTime::infinity())
return false;
wait(mutex);
Expand Down
2 changes: 1 addition & 1 deletion Source/WTF/wtf/win/ThreadingWin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ void Mutex::unlock()
// Returns an interval in milliseconds suitable for passing to one of the Win32 wait functions (e.g., ::WaitForSingleObject).
static DWORD absoluteTimeToWaitTimeoutInterval(WallTime absoluteTime)
{
if (std::isinf(absoluteTime)) {
if (absoluteTime.isInfinity()) {
if (absoluteTime == -WallTime::infinity())
return 0;
return INFINITE;
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/Modules/applepay/ApplePaySession.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ ExceptionOr<void> ApplePayDeferredPaymentRequest::validate() const
if (!URL { managementURL }.isValid())
return Exception(TypeError, makeString('"', managementURL, "\" is not a valid URL."));

if (std::isnan(freeCancellationDate)) {
if (freeCancellationDate.isNaN()) {
if (!freeCancellationDateTimeZone.isEmpty())
return Exception(TypeError, "Unexpected 'freeCancellationDateTimeZone' when 'freeCancellationDate' is not set."_s);
} else if (freeCancellationDateTimeZone.isEmpty())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,11 @@ static NSCalendarUnit toCalendarUnit(ApplePayRecurringPaymentDateUnit unit)
{
ASSERT(lineItem.paymentTiming == ApplePayPaymentTiming::Recurring);
PKRecurringPaymentSummaryItem *summaryItem = [PAL::getPKRecurringPaymentSummaryItemClass() summaryItemWithLabel:lineItem.label amount:toDecimalNumber(lineItem.amount) type:toPKPaymentSummaryItemType(lineItem.type)];
if (!std::isnan(lineItem.recurringPaymentStartDate))
if (!lineItem.recurringPaymentStartDate.isNaN())
summaryItem.startDate = toDate(lineItem.recurringPaymentStartDate);
summaryItem.intervalUnit = toCalendarUnit(lineItem.recurringPaymentIntervalUnit);
summaryItem.intervalCount = lineItem.recurringPaymentIntervalCount;
if (!std::isnan(lineItem.recurringPaymentEndDate))
if (!lineItem.recurringPaymentEndDate.isNaN())
summaryItem.endDate = toDate(lineItem.recurringPaymentEndDate);
return summaryItem;
}
Expand All @@ -106,7 +106,7 @@ static NSCalendarUnit toCalendarUnit(ApplePayRecurringPaymentDateUnit unit)
{
ASSERT(lineItem.paymentTiming == ApplePayPaymentTiming::Deferred);
PKDeferredPaymentSummaryItem *summaryItem = [PAL::getPKDeferredPaymentSummaryItemClass() summaryItemWithLabel:lineItem.label amount:toDecimalNumber(lineItem.amount) type:toPKPaymentSummaryItemType(lineItem.type)];
if (!std::isnan(lineItem.deferredPaymentDate))
if (!lineItem.deferredPaymentDate.isNaN())
summaryItem.deferredDate = toDate(lineItem.deferredPaymentDate);
return summaryItem;
}
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/bindings/IDLTypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ template<typename T> struct IDLTypedArray : IDLBufferSource<T> { };
struct IDLDate : IDLType<WallTime> {
using NullableType = WallTime;
static WallTime nullValue() { return WallTime::nan(); }
static bool isNullValue(WallTime value) { return std::isnan(value); }
static bool isNullValue(WallTime value) { return value.isNaN(); }
static WallTime extractValueFromNullable(WallTime value) { return value; }
};

Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/bindings/js/IDBBindingUtilities.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ static RefPtr<IDBKey> createIDBKeyFromValue(JSGlobalObject& lexicalGlobalObject,
if (value.inherits<DateInstance>()) {
auto dateValue = valueToDate(lexicalGlobalObject, value);
RETURN_IF_EXCEPTION(scope, { });
if (!std::isnan(dateValue))
if (!dateValue.isNaN())
return IDBKey::createDate(dateValue.secondsSinceEpoch().milliseconds());
}

Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/fileapi/FileReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ void FileReader::didReceiveData()
{
enqueueTask([this] {
auto now = MonotonicTime::now();
if (std::isnan(m_lastProgressNotificationTime)) {
if (m_lastProgressNotificationTime.isNaN()) {
m_lastProgressNotificationTime = now;
return;
}
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/inspector/InspectorCanvas.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -772,7 +772,7 @@ void InspectorCanvas::finalizeFrame()
{
appendActionSnapshotIfNeeded();

if (m_frames && m_frames->length() && !std::isnan(m_currentFrameStartTime)) {
if (m_frames && m_frames->length() && !m_currentFrameStartTime.isNaN()) {
auto currentFrame = static_reference_cast<Protocol::Recording::Frame>(m_frames->get(m_frames->length() - 1));
currentFrame->setDuration((MonotonicTime::now() - m_currentFrameStartTime).milliseconds());

Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/page/LocalDOMWindow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1613,7 +1613,7 @@ bool LocalDOMWindow::consumeTransientActivation()

void LocalDOMWindow::consumeLastActivationIfNecessary()
{
if (!std::isinf(m_lastActivationTimestamp))
if (!m_lastActivationTimestamp.isInfinity())
m_lastActivationTimestamp = -MonotonicTime::infinity();
}

Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/page/cocoa/ResourceUsageOverlayCocoa.mm
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,7 @@ static String formatByteNumber(size_t number)

static String gcTimerString(MonotonicTime timerFireDate, MonotonicTime now)
{
if (std::isnan(timerFireDate))
if (timerFireDate.isNaN())
return "[not scheduled]"_s;
return String::numberToStringFixedPrecision((timerFireDate - now).seconds());
}
Expand Down

0 comments on commit acfb618

Please sign in to comment.