- Revision
- 254509
- Author
- ab...@igalia.com
- Date
- 2020-01-14 07:22:20 -0800 (Tue, 14 Jan 2020)
Log Message
[WTF] Make MediaTime constructor constexpr
https://bugs.webkit.org/show_bug.cgi?id=206036
Reviewed by Adrian Perez de Castro.
Source/WTF:
https://bugs.webkit.org/show_bug.cgi?id=205723 allowed to declare
MediaTime variables as static inside functions without needing a
global destructor.
It did not eliminate the call to the MediaTime constructor on runtime
though. This wasn't a problem for static variables inside functions,
as the compiler adds a guard variable to call the constructor the
first time the function is called.
On the other hand, for variables defined outside of the scope of the
function, for them to be initialized the MediaTime constructor would
have to be called at runtime from a global constructor, something
we're trying to avoid and which generates an error in clang.
But again, MediaTime is a simple class with only integral values, we
shouldn't need a runtime function call to initialize it!
This patch makes the MediaTime constructor constexpr so that we don't
need runtime initialization for static MediaTime variables. This
allows us to declare them outside functions and enables the compiler
to generate code without guard variables when static MediaTime
variables are declared inside functions.
A test has been added accessing a global const static MediaTime. The
build should not produce any error stating the need for a global
constructor.
* wtf/MediaTime.cpp:
* wtf/MediaTime.h:
(WTF::MediaTime::MediaTime):
Tools:
Added test for global static MediaTime constants.
* TestWebKitAPI/Tests/WTF/MediaTime.cpp:
(TestWebKitAPI::TEST):
Modified Paths
Diff
Modified: trunk/Source/WTF/ChangeLog (254508 => 254509)
--- trunk/Source/WTF/ChangeLog 2020-01-14 14:51:56 UTC (rev 254508)
+++ trunk/Source/WTF/ChangeLog 2020-01-14 15:22:20 UTC (rev 254509)
@@ -1,3 +1,41 @@
+2020-01-14 Alicia Boya García <ab...@igalia.com>
+
+ [WTF] Make MediaTime constructor constexpr
+ https://bugs.webkit.org/show_bug.cgi?id=206036
+
+ Reviewed by Adrian Perez de Castro.
+
+ https://bugs.webkit.org/show_bug.cgi?id=205723 allowed to declare
+ MediaTime variables as static inside functions without needing a
+ global destructor.
+
+ It did not eliminate the call to the MediaTime constructor on runtime
+ though. This wasn't a problem for static variables inside functions,
+ as the compiler adds a guard variable to call the constructor the
+ first time the function is called.
+
+ On the other hand, for variables defined outside of the scope of the
+ function, for them to be initialized the MediaTime constructor would
+ have to be called at runtime from a global constructor, something
+ we're trying to avoid and which generates an error in clang.
+
+ But again, MediaTime is a simple class with only integral values, we
+ shouldn't need a runtime function call to initialize it!
+
+ This patch makes the MediaTime constructor constexpr so that we don't
+ need runtime initialization for static MediaTime variables. This
+ allows us to declare them outside functions and enables the compiler
+ to generate code without guard variables when static MediaTime
+ variables are declared inside functions.
+
+ A test has been added accessing a global const static MediaTime. The
+ build should not produce any error stating the need for a global
+ constructor.
+
+ * wtf/MediaTime.cpp:
+ * wtf/MediaTime.h:
+ (WTF::MediaTime::MediaTime):
+
2020-01-14 David Kilzer <ddkil...@apple.com>
Enable -Wconditional-uninitialized in bmalloc, WTF, _javascript_Core
Modified: trunk/Source/WTF/wtf/MediaTime.cpp (254508 => 254509)
--- trunk/Source/WTF/wtf/MediaTime.cpp 2020-01-14 14:51:56 UTC (rev 254508)
+++ trunk/Source/WTF/wtf/MediaTime.cpp 2020-01-14 15:22:20 UTC (rev 254509)
@@ -72,24 +72,6 @@
const uint32_t MediaTime::MaximumTimeScale = 1000000000;
-MediaTime::MediaTime()
- : m_timeValue(0)
- , m_timeScale(DefaultTimeScale)
- , m_timeFlags(Valid)
-{
-}
-
-MediaTime::MediaTime(int64_t value, uint32_t scale, uint8_t flags)
- : m_timeValue(value)
- , m_timeScale(scale)
- , m_timeFlags(flags)
-{
- if (scale || isInvalid())
- return;
-
- *this = value < 0 ? negativeInfiniteTime() : positiveInfiniteTime();
-}
-
MediaTime::MediaTime(const MediaTime& rhs)
{
*this = rhs;
Modified: trunk/Source/WTF/wtf/MediaTime.h (254508 => 254509)
--- trunk/Source/WTF/wtf/MediaTime.h 2020-01-14 14:51:56 UTC (rev 254508)
+++ trunk/Source/WTF/wtf/MediaTime.h 2020-01-14 15:22:20 UTC (rev 254509)
@@ -53,8 +53,8 @@
DoubleValue = 1 << 5,
};
- MediaTime();
- MediaTime(int64_t value, uint32_t scale, uint8_t flags = Valid);
+ constexpr MediaTime();
+ constexpr MediaTime(int64_t value, uint32_t scale, uint8_t flags = Valid);
MediaTime(const MediaTime& rhs);
static MediaTime createWithFloat(float floatTime);
@@ -148,6 +148,24 @@
uint8_t m_timeFlags;
};
+constexpr MediaTime::MediaTime()
+ : m_timeValue(0)
+ , m_timeScale(DefaultTimeScale)
+ , m_timeFlags(Valid)
+{
+}
+
+constexpr MediaTime::MediaTime(int64_t value, uint32_t scale, uint8_t flags)
+ : m_timeValue(value)
+ , m_timeScale(scale)
+ , m_timeFlags(flags)
+{
+ if (scale || !(flags & Valid))
+ return;
+
+ *this = value < 0 ? negativeInfiniteTime() : positiveInfiniteTime();
+}
+
inline MediaTime operator*(int32_t lhs, const MediaTime& rhs) { return rhs.operator*(lhs); }
WTF_EXPORT_PRIVATE extern MediaTime abs(const MediaTime& rhs);
Modified: trunk/Tools/ChangeLog (254508 => 254509)
--- trunk/Tools/ChangeLog 2020-01-14 14:51:56 UTC (rev 254508)
+++ trunk/Tools/ChangeLog 2020-01-14 15:22:20 UTC (rev 254509)
@@ -1,3 +1,15 @@
+2020-01-14 Alicia Boya García <ab...@igalia.com>
+
+ [WTF] Make MediaTime constructor constexpr
+ https://bugs.webkit.org/show_bug.cgi?id=206036
+
+ Reviewed by Adrian Perez de Castro.
+
+ Added test for global static MediaTime constants.
+
+ * TestWebKitAPI/Tests/WTF/MediaTime.cpp:
+ (TestWebKitAPI::TEST):
+
2020-01-13 Fujii Hironori <hironori.fu...@sony.com>
Unreviewed sort-Xcode-project-file
Modified: trunk/Tools/TestWebKitAPI/Tests/WTF/MediaTime.cpp (254508 => 254509)
--- trunk/Tools/TestWebKitAPI/Tests/WTF/MediaTime.cpp 2020-01-14 14:51:56 UTC (rev 254508)
+++ trunk/Tools/TestWebKitAPI/Tests/WTF/MediaTime.cpp 2020-01-14 15:22:20 UTC (rev 254509)
@@ -56,6 +56,13 @@
namespace TestWebKitAPI {
+// MediaTime values should be able to be declared static anywhere, just like you can do so with integers.
+// This should not require global constructors or destructors.
+#pragma clang diagnostic push
+#pragma clang diagnostic error "-Wglobal-constructors"
+static const MediaTime oneSecond(1, 1);
+#pragma clang diagnostic pop
+
TEST(WTF, MediaTime)
{
// Comparison Operators
@@ -97,6 +104,7 @@
EXPECT_TRUE(!MediaTime::createWithDouble(0.0, 1));
EXPECT_FALSE(!MediaTime(1, 1));
EXPECT_FALSE((bool)MediaTime::invalidTime());
+ EXPECT_EQ(MediaTime(10, 10), oneSecond);
// Addition Operators
EXPECT_EQ(MediaTime::positiveInfiniteTime() + MediaTime::positiveInfiniteTime(), MediaTime::positiveInfiniteTime());
