ERROR: Source/WTF/wtf/MediaTime.h:34: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/MediaTime.h:69: Inline functions should not be in classes annotated with WTF_EXPORT_PRIVATE. Remove the macro from the class and apply it to each appropriate method, or move the inline fun ction definition out-of-line. [build/webcore_export] [4] ERROR: Source/WTF/wtf/MediaTime.h:70: Inline functions should not be in classes annotated with WTF_EXPORT_PRIVATE. Remove the macro from the class and apply it to each appropriate method, or move the inline fun ction definition out-of-line. [build/webcore_export] [4] ERROR: Source/WTF/wtf/MediaTime.h:75: Inline functions should not be in classes annotated with WTF_EXPORT_PRIVATE. Remove the macro from the class and apply it to each appropriate method, or move the inline fun ction definition out-of-line. [build/webcore_export] [4] ERROR: Source/WTF/wtf/MediaTime.h:76: Inline functions should not be in classes annotated with WTF_EXPORT_PRIVATE. Remove the macro from the class and apply it to each appropriate method, or move the inline fun ction definition out-of-line. [build/webcore_export] [4] ERROR: Source/WTF/wtf/MediaTime.h:77: Inline functions should not be in classes annotated with WTF_EXPORT_PRIVATE. Remove the macro from the class and apply it to each appropriate method, or move the inline fun ction definition out-of-line. [build/webcore_export] [4] ERROR: Source/WTF/wtf/MediaTime.h:78: Inline functions should not be in classes annotated with WTF_EXPORT_PRIVATE. Remove the macro from the class and apply it to each appropriate method, or move the inline fun ction definition out-of-line. [build/webcore_export] [4] ERROR: Source/WTF/wtf/MediaTime.h:93: Inline functions should not be in classes annotated with WTF_EXPORT_PRIVATE. Remove the macro from the class and apply it to each appropriate method, or move the inline fun ction definition out-of-line. [build/webcore_export] [4] ERROR: Source/WTF/wtf/MediaTime.h:94: Inline functions should not be in classes annotated with WTF_EXPORT_PRIVATE. Remove the macro from the class and apply it to each appropriate method, or move the inline fun ction definition out-of-line. [build/webcore_export] [4] ERROR: Source/WTF/wtf/MediaTime.h:95: Inline functions should not be in classes annotated with WTF_EXPORT_PRIVATE. Remove the macro from the class and apply it to each appropriate method, or move the inline fun ction definition out-of-line. [build/webcore_export] [4] ERROR: Source/WTF/wtf/MediaTime.h:96: Inline functions should not be in classes annotated with WTF_EXPORT_PRIVATE. Remove the macro from the class and apply it to each appropriate method, or move the inline fun ction definition out-of-line. [build/webcore_export] [4] ERROR: Source/WTF/wtf/MediaTime.h:97: Inline functions should not be in classes annotated with WTF_EXPORT_PRIVATE. Remove the macro from the class and apply it to each appropriate method, or move the inline fun ction definition out-of-line. [build/webcore_export] [4] ERROR: Source/WTF/wtf/MediaTime.h:98: Inline functions should not be in classes annotated with WTF_EXPORT_PRIVATE. Remove the macro from the class and apply it to each appropriate method, or move the inline fun ction definition out-of-line. [build/webcore_export] [4] ERROR: Source/WTF/wtf/MediaTime.h:99: Inline functions should not be in classes annotated with WTF_EXPORT_PRIVATE. Remove the macro from the class and apply it to each appropriate method, or move the inline fun ction definition out-of-line. [build/webcore_export] [4] ERROR: Source/WTF/wtf/MediaTime.h:100: Inline functions should not be in classes annotated with WTF_EXPORT_PRIVATE. Remove the macro from the class and apply it to each appropriate method, or move the inline fu nction definition out-of-line. [build/webcore_export] [4] ERROR: Source/WTF/wtf/MediaTime.h:101: Inline functions should not be in classes annotated with WTF_EXPORT_PRIVATE. Remove the macro from the class and apply it to each appropriate method, or move the inline fu nction definition out-of-line. [build/webcore_export] [4] ERROR: Source/WTF/wtf/MediaTime.h:109: Inline functions should not be in classes annotated with WTF_EXPORT_PRIVATE. Remove the macro from the class and apply it to each appropriate method, or move the inline fu nction definition out-of-line. [build/webcore_export] [4] ERROR: Source/WTF/wtf/MediaTime.h:110: Inline functions should not be in classes annotated with WTF_EXPORT_PRIVATE. Remove the macro from the class and apply it to each appropriate method, or move the inline fu nction definition out-of-line. [build/webcore_export] [4]
Created attachment 412731 [details] Patch
Comment on attachment 412731 [details] Patch I think these functions are small and should be inline function. So defining in the header is better. Let's remove WTF_EXPORT_PRIVATE from the class, and annotate some of actually exported functions with WTF_EXPORT_PRIVATE.
<rdar://problem/71114972>
Created attachment 414209 [details] Patch
(In reply to Yusuke Suzuki from comment #2) > Comment on attachment 412731 [details] > Patch > > I think these functions are small and should be inline function. So defining > in the header is better. Let's remove WTF_EXPORT_PRIVATE from the class, and > annotate some of actually exported functions with WTF_EXPORT_PRIVATE. Thanks for the review Yusuke, the patch is updated accordingly, hopefully :)
Created attachment 414216 [details] Patch
Comment on attachment 414216 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=414216&action=review > Source/WTF/wtf/MediaTime.h:68 > + inline MediaTime& operator+=(const MediaTime& rhs) { return *this = *this + rhs; } I don't think the "inline" keyword is needed here
Created attachment 414252 [details] Patch
Comment on attachment 414252 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=414252&action=review > Source/WTF/wtf/MediaTime.h:74 > + inline bool operator<(const MediaTime& rhs) const { return compare(rhs) == LessThan; } here either. > Source/WTF/wtf/MediaTime.h:92 > + inline bool isValid() const { return m_timeFlags & Valid; } ditto > Source/WTF/wtf/MediaTime.h:107 > + inline const int64_t& timeValue() const { return m_timeValue; } ditto
If no inline specifier remains in this patch I'm not sure why Yusuke r-'ed the initial patch.
(In reply to Philippe Normand from comment #10) > If no inline specifier remains in this patch I'm not sure why Yusuke r-'ed > the initial patch. In C++, inline attribute is not for inlining. If you define function in header, compiler will decide whether inlines or not regardless of inline attribute is attached or not. But unless LTO is enabled (WTF / JSC is not LTO enabled for now), function defined in cpp will not be inlined. So the patch should define function in header, and inline attribute is not necessary.