Use `inline static constexpr` now that C++17 is supported [chromium/src : main]
Will Cassella (Gerrit)
Attention is currently required from: Kentaro Hara.
Will Cassella would like Kentaro Hara to review this change.
Use `inline static constexpr` now that C++17 is supported
Previously, C++ required you to declare "storage" for static constexpr
variables in a compilation unit, similar to other static variables, as
otherwise you would get linker errors. This doesn't really make any
sense for constexpr variables as they're not meant to be modified in any
capacity, and should be inlined wherever they're used. C++17 fixes that
by letting you declare "static inline constexpr" variables, and now that
we've officially adopted C++17 this CL does just that.
Change-Id: Ib84687657d5914d05b7d5417dde767731e138800
---
M third_party/blink/renderer/core/html/media/html_media_element.h
M third_party/blink/renderer/core/html/media/html_media_element.cc
2 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/third_party/blink/renderer/core/html/media/html_media_element.cc b/third_party/blink/renderer/core/html/media/html_media_element.cc
index 50e9fb1..ec63220 100644
--- a/third_party/blink/renderer/core/html/media/html_media_element.cc
+++ b/third_party/blink/renderer/core/html/media/html_media_element.cc
@@ -444,11 +444,6 @@
} // anonymous namespace
-// TODO(https://crbug.com/752720): Remove this once C++17 is adopted (and hence,
-// `inline constexpr` is supported).
-constexpr double HTMLMediaElement::kMinPlaybackRate;
-constexpr double HTMLMediaElement::kMaxPlaybackRate;
-
// static
MIMETypeRegistry::SupportsType HTMLMediaElement::GetSupportsType(
const ContentType& content_type) {
diff --git a/third_party/blink/renderer/core/html/media/html_media_element.h b/third_party/blink/renderer/core/html/media/html_media_element.h
index 9dc34064..3c48056 100644
--- a/third_party/blink/renderer/core/html/media/html_media_element.h
+++ b/third_party/blink/renderer/core/html/media/html_media_element.h
@@ -111,8 +111,8 @@
public:
// Limits the range of media playback rate.
- static constexpr double kMinPlaybackRate = 0.0625;
- static constexpr double kMaxPlaybackRate = 16.0;
+ static inline constexpr double kMinPlaybackRate = 0.0625;
+ static inline constexpr double kMaxPlaybackRate = 16.0;
bool IsMediaElement() const override { return true; }
To view, visit change 3360064. To unsubscribe, or for help writing mail filters, visit settings.
Will Cassella (Gerrit)
Attention is currently required from: Kentaro Hara.
1 comment:
Patchset:
Hey Kentaro! I though we had more //media owners on this directory, but turns out it's just Dale who's OOO until next year. Would you mind taking a look at this if you're available? Thanks!
To view, visit change 3360064. To unsubscribe, or for help writing mail filters, visit settings.
Avi Drissman (Gerrit)
Attention is currently required from: Kentaro Hara, Will Cassella.
Patch set 1:Code-Review -1
1 comment:
Patchset:
Just because we now support C++17 doesn’t mean you can yet use its new features.
https://chromium.googlesource.com/chromium/src/+/main/styleguide/c++/c++11.md lists all the features of the new versions of C++, and this feature is still on the TBD list (https://chromium.googlesource.com/chromium/src/+/main/styleguide/c++/c++11.md#Inline-variables-tbd).
That document has a procedure for approving features, and the thread for this feature is at https://groups.google.com/a/chromium.org/g/cxx/c/hmyGFD80ocE/m/IF7bmCMPDAAJ . Since you want to use it, please chime in on the thread with your reasoning and join the discussion. However, until this feature is officially approved for use, you may not use it.
To view, visit change 3360064. To unsubscribe, or for help writing mail filters, visit settings.
Will Cassella (Gerrit)
Attention is currently required from: Kentaro Hara, Avi Drissman.
Patch set 1:Commit-Queue +1
1 comment:
Patchset:
Just because we now support C++17 doesn’t mean you can yet use its new features. […]
That was fast! 😊
Sorry, I jumped the gun on this. I've commented on the thread you linked.
To view, visit change 3360064. To unsubscribe, or for help writing mail filters, visit settings.