Bug 160638

Summary:

Replace NodeOrString with Variant<RefPtr<Node>, String>

Product:

WebKit

Reporter:

Sam Weinig <sam>

Component:

New Bugs

Assignee:

Sam Weinig <sam>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ap, bfulgham, commit-queue, darin, ryanhaddad

Priority:

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none

Sam Weinig

Reported 2016-08-06 15:20:37 PDT

Replace NodeOrString with Variant<Ref<Node>, String>

Attachments
Patch (32.85 KB, patch) 2016-08-06 15:28 PDT, Sam Weinig	no flags	Details Formatted Diff Diff
Patch (33.19 KB, patch) 2016-08-06 18:35 PDT, Sam Weinig	no flags	Details Formatted Diff Diff
Patch (33.35 KB, patch) 2016-08-07 15:33 PDT, Sam Weinig	no flags	Details Formatted Diff Diff
Patch (33.35 KB, patch) 2016-08-07 15:38 PDT, Sam Weinig	no flags	Details Formatted Diff Diff
Patch (34.21 KB, patch) 2016-08-07 15:51 PDT, Sam Weinig	no flags	Details Formatted Diff Diff
Patch (38.80 KB, patch) 2016-08-07 17:52 PDT, Sam Weinig	no flags	Details Formatted Diff Diff
Patch (38.88 KB, patch) 2016-08-07 18:02 PDT, Sam Weinig	no flags	Details Formatted Diff Diff
Patch (37.79 KB, patch) 2016-08-08 13:33 PDT, Sam Weinig	no flags	Details Formatted Diff Diff
Patch (38.55 KB, patch) 2016-08-08 16:04 PDT, Sam Weinig	no flags	Details Formatted Diff Diff
Patch (39.43 KB, patch) 2016-08-08 21:02 PDT, Sam Weinig	no flags	Details Formatted Diff Diff
Patch (39.97 KB, patch) 2016-08-09 07:17 PDT, Sam Weinig	no flags	Details Formatted Diff Diff
Patch (44.81 KB, patch) 2016-08-09 10:44 PDT, Sam Weinig	no flags	Details Formatted Diff Diff
Patch (44.79 KB, patch) 2016-08-09 10:56 PDT, Sam Weinig	no flags	Details Formatted Diff Diff
Patch (44.80 KB, patch) 2016-08-09 11:08 PDT, Sam Weinig	no flags	Details Formatted Diff Diff
Patch (44.81 KB, patch) 2016-08-09 11:32 PDT, Sam Weinig	no flags	Details Formatted Diff Diff
Show Obsolete (14) View All Add attachment proposed patch, testcase, etc.

Sam Weinig

Comment 1 2016-08-06 15:28:40 PDT

Created attachment 285502 [details] Patch

WebKit Commit Bot

Comment 2 2016-08-06 15:30:32 PDT

Attachment 285502 [details] did not pass style-queue: ERROR: Source/WTF/wtf/Variant.h:67: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:67: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1173: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:1173: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1173: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1173: __move_construct_func is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:1176: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:1176: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1176: __destroy_func is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:1942: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:1942: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1942: Missing space before { [whitespace/braces] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:41: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:41: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:42: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:42: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:45: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:50: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:50: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:51: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:59: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:65: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:66: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Source/WTF/wtf/StdLibExtras.h:322: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:323: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:325: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/StdLibExtras.h:335: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 27 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.

Sam Weinig

Comment 3 2016-08-06 18:35:51 PDT

Created attachment 285512 [details] Patch

WebKit Commit Bot

Comment 4 2016-08-06 18:36:41 PDT

Attachment 285512 [details] did not pass style-queue: ERROR: Source/WTF/wtf/Variant.h:75: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:75: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1181: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:1181: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1181: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1181: __move_construct_func is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:1184: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:1184: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1184: __destroy_func is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:1950: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:1950: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1950: Missing space before { [whitespace/braces] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:41: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:41: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:42: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:42: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:45: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:50: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:50: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:51: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:59: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:65: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:66: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Source/WTF/wtf/StdLibExtras.h:322: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:323: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:325: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/StdLibExtras.h:335: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 27 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.

Darin Adler

Comment 5 2016-08-06 18:41:37 PDT

Comment on attachment 285512 [details] Patch Looks good. But seems that Variant.h does not yet compile with the compiler the gtk-wk2 bot is using.

Darin Adler

Comment 6 2016-08-07 09:38:21 PDT

GTK: Variant.h:553:29: error: 'constexpr const _Type&& std::experimental::__variant_data<_Type>::__get_rref(std::experimental::in_place_index_t<0ul>) const' cannot be overloaded Variant.h:547:23: error: with 'constexpr _Type&& std::experimental::__variant_data<_Type>::__get_rref(std::experimental::in_place_index_t<0ul>) const' Variant.h:646:29: error: 'constexpr const _Head&& std::experimental::__variant_data<_Head, _Rest ...>::__get_rref(std::experimental::in_place_index_t<0ul>) const' cannot be overloaded Variant.h:638:23: error: with 'constexpr _Head&& std::experimental::__variant_data<_Head, _Rest ...>::__get_rref(std::experimental::in_place_index_t<0ul>) const' Can't figure out what the Windows error is.

Sam Weinig

Comment 7 2016-08-07 15:33:45 PDT

Created attachment 285536 [details] Patch

WebKit Commit Bot

Comment 8 2016-08-07 15:36:04 PDT

Attachment 285536 [details] did not pass style-queue: ERROR: Source/WTF/wtf/Variant.h:87: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:87: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1193: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:1193: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1193: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1193: __move_construct_func is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:1196: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:1196: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1196: __destroy_func is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:1962: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:1962: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1962: Missing space before { [whitespace/braces] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:41: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:41: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:42: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:42: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:45: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:50: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:50: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:51: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:59: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:65: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:66: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Source/WTF/wtf/StdLibExtras.h:322: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:323: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:325: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/StdLibExtras.h:335: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 27 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.

Sam Weinig

Comment 9 2016-08-07 15:36:30 PDT

(In reply to comment #6) > GTK: > > Variant.h:553:29: error: 'constexpr const _Type&& > std::experimental::__variant_data<_Type>::__get_rref(std::experimental:: > in_place_index_t<0ul>) const' cannot be overloaded > Variant.h:547:23: error: with 'constexpr _Type&& > std::experimental::__variant_data<_Type>::__get_rref(std::experimental:: > in_place_index_t<0ul>) const' > Variant.h:646:29: error: 'constexpr const _Head&& > std::experimental::__variant_data<_Head, _Rest > ...>::__get_rref(std::experimental::in_place_index_t<0ul>) const' cannot be > overloaded > Variant.h:638:23: error: with 'constexpr _Head&& > std::experimental::__variant_data<_Head, _Rest > ...>::__get_rref(std::experimental::in_place_index_t<0ul>) const' > This seems to be due to the GCC version in use making all constexpr functions implicitly const, and therefore failing because it thinks the functions above are essentially the same. I need to figure out what GCC is being used, since I am pretty sure this works in the latest GCC. > Can't figure out what the Windows error is. Yeah, me either. That MSVC output is a mystery to me.

Sam Weinig

Comment 10 2016-08-07 15:38:40 PDT

Created attachment 285537 [details] Patch

WebKit Commit Bot

Comment 11 2016-08-07 15:40:56 PDT

Attachment 285537 [details] did not pass style-queue: ERROR: Source/WTF/wtf/Variant.h:87: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:87: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1193: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:1193: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1193: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1193: __move_construct_func is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:1196: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:1196: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1196: __destroy_func is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:1962: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:1962: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1962: Missing space before { [whitespace/braces] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:41: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:41: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:42: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:42: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:45: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:50: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:50: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:51: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:59: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:65: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:66: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Source/WTF/wtf/StdLibExtras.h:322: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:323: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:325: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/StdLibExtras.h:335: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 27 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.

Sam Weinig

Comment 12 2016-08-07 15:51:52 PDT

Created attachment 285541 [details] Patch

WebKit Commit Bot

Comment 13 2016-08-07 15:53:04 PDT

Attachment 285541 [details] did not pass style-queue: ERROR: Source/WTF/wtf/Variant.h:75: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:75: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:547: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:547: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:547: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/Variant.h:547: __get_rref is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:638: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:638: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:638: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/Variant.h:638: __get_rref is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:657: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:657: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/Variant.h:1181: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:1181: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1181: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1181: __move_construct_func is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:1184: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:1184: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1184: __destroy_func is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:1950: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:1950: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1950: Missing space before { [whitespace/braces] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:41: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:41: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:42: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:42: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:45: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:50: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:50: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:51: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:59: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:65: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:66: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Source/WTF/wtf/StdLibExtras.h:322: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:323: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:325: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/StdLibExtras.h:335: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 37 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.

Sam Weinig

Comment 14 2016-08-07 17:52:09 PDT

Created attachment 285547 [details] Patch

WebKit Commit Bot

Comment 15 2016-08-07 17:53:43 PDT

Attachment 285547 [details] did not pass style-queue: ERROR: Source/WTF/wtf/Variant.h:113: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:113: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:227: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:534: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:534: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:534: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/Variant.h:534: __get_rref is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:625: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:625: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:625: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/Variant.h:625: __get_rref is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:644: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:644: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/Variant.h:1168: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:1168: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1168: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1168: __move_construct_func is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:1171: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:1171: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1171: __destroy_func is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:1783: Missing spaces around != [whitespace/operators] [3] ERROR: Source/WTF/wtf/Variant.h:1784: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1785: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1785: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Variant.h:1792: Missing spaces around != [whitespace/operators] [3] ERROR: Source/WTF/wtf/Variant.h:1793: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1794: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1794: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Variant.h:1800: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1801: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] ERROR: Source/WTF/wtf/Variant.h:1803: Missing spaces around != [whitespace/operators] [3] ERROR: Source/WTF/wtf/Variant.h:1807: Wrong number of spaces before statement. (expected: 28) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Variant.h:1807: Use 'WTFMove()' instead of 'std::move()'. [runtime/wtf_move] [4] ERROR: Source/WTF/wtf/Variant.h:1814: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1815: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] ERROR: Source/WTF/wtf/Variant.h:1817: Missing spaces around != [whitespace/operators] [3] ERROR: Source/WTF/wtf/Variant.h:1821: Wrong number of spaces before statement. (expected: 28) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Variant.h:1821: Use 'WTFMove()' instead of 'std::move()'. [runtime/wtf_move] [4] ERROR: Source/WTF/wtf/Variant.h:1890: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1891: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1960: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:1960: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1960: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1961: Missing space after , [whitespace/comma] [3] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:41: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:41: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:42: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:42: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:45: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:50: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:50: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:51: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:59: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:65: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:66: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Source/WTF/wtf/StdLibExtras.h:322: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:323: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:325: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/StdLibExtras.h:335: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 59 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.

Sam Weinig

Comment 16 2016-08-07 18:02:21 PDT

Created attachment 285548 [details] Patch

WebKit Commit Bot

Comment 17 2016-08-07 18:04:21 PDT

Attachment 285548 [details] did not pass style-queue: ERROR: Source/WTF/wtf/Variant.h:113: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:113: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:227: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:534: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:534: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:534: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/Variant.h:534: __get_rref is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:625: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:625: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:625: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/Variant.h:625: __get_rref is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:644: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:644: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/Variant.h:1168: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:1168: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1168: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1168: __move_construct_func is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:1171: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:1171: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1171: __destroy_func is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:1783: Missing spaces around != [whitespace/operators] [3] ERROR: Source/WTF/wtf/Variant.h:1784: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1785: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1785: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Variant.h:1792: Missing spaces around != [whitespace/operators] [3] ERROR: Source/WTF/wtf/Variant.h:1793: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1794: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1794: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Variant.h:1800: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1801: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] ERROR: Source/WTF/wtf/Variant.h:1803: Missing spaces around != [whitespace/operators] [3] ERROR: Source/WTF/wtf/Variant.h:1807: Wrong number of spaces before statement. (expected: 28) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Variant.h:1807: Use 'WTFMove()' instead of 'std::move()'. [runtime/wtf_move] [4] ERROR: Source/WTF/wtf/Variant.h:1814: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1815: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] ERROR: Source/WTF/wtf/Variant.h:1817: Missing spaces around != [whitespace/operators] [3] ERROR: Source/WTF/wtf/Variant.h:1821: Wrong number of spaces before statement. (expected: 28) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Variant.h:1821: Use 'WTFMove()' instead of 'std::move()'. [runtime/wtf_move] [4] ERROR: Source/WTF/wtf/Variant.h:1890: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1891: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1960: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:1960: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1960: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1961: Missing space after , [whitespace/comma] [3] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:41: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:41: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:42: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:42: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:45: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:50: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:50: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:51: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:59: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:65: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:66: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Source/WTF/wtf/StdLibExtras.h:322: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:323: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:325: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/StdLibExtras.h:335: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 59 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.

Brent Fulgham

Comment 18 2016-08-08 12:03:57 PDT

I see the following errors in a local Windows build. It seems like 24> WebSocketExtensionDispatcher.cpp 24>C:\Projects\WebKit\OpenSource\WebKitBuild\Release\DerivedSources\ForwardingHeaders\wtf/Variant.h(1540): error C2039: 'value': is not a member of 'std::experimental::__noexcept_variant_non_const_copy_construct<WTF::Ref<WebCore::Node>,WTF::String>' (compiling source file C:\Projects\WebKit\OpenSource\Source\WebCore\bindings\js\JSBindingsAllInOne.cpp) 24> C:\Projects\WebKit\OpenSource\WebKitBuild\Release\DerivedSources\ForwardingHeaders\wtf/Variant.h(1541): note: see declaration of 'std::experimental::__noexcept_variant_non_const_copy_construct<WTF::Ref<WebCore::Node>,WTF::String>' (compiling source file C:\Projects\WebKit\OpenSource\Source\WebCore\bindings\js\JSBindingsAllInOne.cpp) 24> c:\projects\webkit\opensource\source\webcore\bindings\js\JSNodeOrString.cpp(46): note: see reference to class template instantiation 'std::experimental::variant<WTF::Ref<WebCore::Node>,WTF::String>' being compiled (compiling source file C:\Projects\WebKit\OpenSource\Source\WebCore\bindings\js\JSBindingsAllInOne.cpp) 24>C:\Projects\WebKit\OpenSource\WebKitBuild\Release\DerivedSources\ForwardingHeaders\wtf/Variant.h(1547): error C2039: 'value': is not a member of 'std::experimental::__noexcept_variant_const_copy_construct<WTF::Ref<WebCore::Node>,WTF::String>' (compiling source file C:\Projects\WebKit\OpenSource\Source\WebCore\bindings\js\JSBindingsAllInOne.cpp) 24> C:\Projects\WebKit\OpenSource\WebKitBuild\Release\DerivedSources\ForwardingHeaders\wtf/Variant.h(1548): note: see declaration of 'std::experimental::__noexcept_variant_const_copy_construct<WTF::Ref<WebCore::Node>,WTF::String>' (compiling source file C:\Projects\WebKit\OpenSource\Source\WebCore\bindings\js\JSBindingsAllInOne.cpp)

Brent Fulgham

Comment 19 2016-08-08 12:04:37 PDT

It seems like this might indicate the VS2015 C++ implementation isn't fully baked for this.

Brent Fulgham

Comment 20 2016-08-08 12:11:05 PDT

(In reply to comment #19) > It seems like this might indicate the VS2015 C++ implementation isn't fully > baked for this. Actually, it seems to be resolving to some weird “std::experimental” namespace, rather than using the implementation in Variant.h.

Sam Weinig

Comment 21 2016-08-08 13:33:59 PDT

Created attachment 285584 [details] Patch

Sam Weinig

Comment 22 2016-08-08 13:36:17 PDT

(In reply to comment #20) > (In reply to comment #19) > > It seems like this might indicate the VS2015 C++ implementation isn't fully > > baked for this. > > Actually, it seems to be resolving to some weird “std::experimental” > namespace, rather than using the implementation in Variant.h. No, I used a type alias to rename std::experimental::variant to WTF::Variant, which is why we are seeing this. It looks like maybe non-copyable types (e.g. Ref) aren't working in it. For now, I am going to see if RefPtr works and figure out later how to make Ref work.

WebKit Commit Bot

Comment 23 2016-08-08 13:36:37 PDT

Attachment 285584 [details] did not pass style-queue: ERROR: Source/WTF/wtf/Variant.h:113: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:113: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:535: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:535: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:535: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/Variant.h:535: __get_rref is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:626: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:626: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:626: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/Variant.h:626: __get_rref is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:645: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:645: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/Variant.h:1169: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:1169: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1169: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1169: __move_construct_func is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:1172: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:1172: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1172: __destroy_func is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:1784: Missing spaces around != [whitespace/operators] [3] ERROR: Source/WTF/wtf/Variant.h:1785: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1786: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1786: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Variant.h:1793: Missing spaces around != [whitespace/operators] [3] ERROR: Source/WTF/wtf/Variant.h:1794: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1795: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1795: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Variant.h:1801: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1802: Use 'WTFMove()' instead of 'std::move()'. [runtime/wtf_move] [4] ERROR: Source/WTF/wtf/Variant.h:1802: Missing spaces around != [whitespace/operators] [3] ERROR: Source/WTF/wtf/Variant.h:1808: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1809: Use 'WTFMove()' instead of 'std::move()'. [runtime/wtf_move] [4] ERROR: Source/WTF/wtf/Variant.h:1809: Missing spaces around != [whitespace/operators] [3] ERROR: Source/WTF/wtf/Variant.h:1877: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1878: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1947: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:1947: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1947: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1948: Missing space after , [whitespace/comma] [3] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:41: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:41: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:42: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:42: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:45: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:50: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:50: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:51: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:59: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:65: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:66: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Source/WTF/wtf/StdLibExtras.h:322: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:323: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:325: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/StdLibExtras.h:335: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 54 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.

Sam Weinig

Comment 24 2016-08-08 16:04:20 PDT

Created attachment 285597 [details] Patch

WebKit Commit Bot

Comment 25 2016-08-08 16:05:30 PDT

Attachment 285597 [details] did not pass style-queue: ERROR: Source/WTF/wtf/Variant.h:113: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:113: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:535: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:535: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:535: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/Variant.h:535: __get_rref is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:626: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:626: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:626: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/Variant.h:626: __get_rref is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:645: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:645: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/Variant.h:1169: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:1169: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1169: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1169: __move_construct_func is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:1172: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:1172: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1172: __destroy_func is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:1784: Missing spaces around != [whitespace/operators] [3] ERROR: Source/WTF/wtf/Variant.h:1785: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1786: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1786: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Variant.h:1793: Missing spaces around != [whitespace/operators] [3] ERROR: Source/WTF/wtf/Variant.h:1794: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1795: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1795: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Variant.h:1801: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1802: Use 'WTFMove()' instead of 'std::move()'. [runtime/wtf_move] [4] ERROR: Source/WTF/wtf/Variant.h:1802: Missing spaces around != [whitespace/operators] [3] ERROR: Source/WTF/wtf/Variant.h:1808: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1809: Use 'WTFMove()' instead of 'std::move()'. [runtime/wtf_move] [4] ERROR: Source/WTF/wtf/Variant.h:1809: Missing spaces around != [whitespace/operators] [3] ERROR: Source/WTF/wtf/Variant.h:1877: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1878: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1947: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:1947: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1947: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1948: Missing space after , [whitespace/comma] [3] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:41: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:41: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:42: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:42: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:45: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:50: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:50: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:51: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:58: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:59: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:65: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:66: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Source/WTF/wtf/StdLibExtras.h:323: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:324: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:326: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/StdLibExtras.h:336: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/StdLibExtras.h:409: Use 'using namespace std;' instead of 'using std::experimental::get;'. [build/using_std] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:410: Use 'using namespace std;' instead of 'using std::experimental::get_if;'. [build/using_std] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:411: Use 'using namespace std;' instead of 'using std::experimental::holds_alternative;'. [build/using_std] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:412: Use 'using namespace std;' instead of 'using std::experimental::visit;'. [build/using_std] [4] Total errors found: 59 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.

Brent Fulgham

Comment 26 2016-08-08 16:13:30 PDT

(In reply to comment #22) > No, I used a type alias to rename std::experimental::variant to > WTF::Variant, which is why we are seeing this. It looks like maybe > non-copyable types (e.g. Ref) aren't working in it. For now, I am going to > see if RefPtr works and figure out later how to make Ref work. For what it's worth, I tried VS 2015 Update 3 and it was not successful building either. :-(

Anders Carlsson

Comment 27 2016-08-08 17:32:38 PDT

Comment on attachment 285597 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=285597&action=review > Source/WebCore/dom/Node.cpp:497 > + return WTFMove(nodeToReturn); No need for move here (it'll actually pessimism the NVRO).

Sam Weinig

Comment 28 2016-08-08 21:02:39 PDT

Created attachment 285628 [details] Patch

WebKit Commit Bot

Comment 29 2016-08-08 21:03:37 PDT

Attachment 285628 [details] did not pass style-queue: ERROR: Source/WTF/wtf/Variant.h:113: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:113: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:535: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:535: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:535: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/Variant.h:535: __get_rref is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:626: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:626: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:626: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/Variant.h:626: __get_rref is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:645: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:645: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/Variant.h:1169: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:1169: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1169: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1169: __move_construct_func is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:1172: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:1172: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1172: __destroy_func is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:1784: Missing spaces around != [whitespace/operators] [3] ERROR: Source/WTF/wtf/Variant.h:1785: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1786: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1786: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Variant.h:1793: Missing spaces around != [whitespace/operators] [3] ERROR: Source/WTF/wtf/Variant.h:1794: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1795: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1795: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Variant.h:1801: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1802: Use 'WTFMove()' instead of 'std::move()'. [runtime/wtf_move] [4] ERROR: Source/WTF/wtf/Variant.h:1802: Missing spaces around != [whitespace/operators] [3] ERROR: Source/WTF/wtf/Variant.h:1808: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1809: Use 'WTFMove()' instead of 'std::move()'. [runtime/wtf_move] [4] ERROR: Source/WTF/wtf/Variant.h:1809: Missing spaces around != [whitespace/operators] [3] ERROR: Source/WTF/wtf/Variant.h:1877: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1878: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1947: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:1947: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1947: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1948: Missing space after , [whitespace/comma] [3] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:41: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:41: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:42: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:42: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:45: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:50: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:50: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:51: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:58: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:59: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:65: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:66: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Source/WTF/wtf/StdLibExtras.h:323: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:324: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:326: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/StdLibExtras.h:336: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/StdLibExtras.h:409: Use 'using namespace std;' instead of 'using std::experimental::get;'. [build/using_std] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:410: Use 'using namespace std;' instead of 'using std::experimental::get_if;'. [build/using_std] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:411: Use 'using namespace std;' instead of 'using std::experimental::holds_alternative;'. [build/using_std] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:412: Use 'using namespace std;' instead of 'using std::experimental::visit;'. [build/using_std] [4] Total errors found: 59 in 13 files If any of these errors are false positives, please file a bug against check-webkit-style.

Sam Weinig

Comment 30 2016-08-09 07:17:25 PDT

Created attachment 285637 [details] Patch

WebKit Commit Bot

Comment 31 2016-08-09 07:18:19 PDT

Attachment 285637 [details] did not pass style-queue: ERROR: Source/WTF/wtf/Variant.h:113: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:113: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:535: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:535: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:535: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/Variant.h:535: __get_rref is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:626: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:626: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:626: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/Variant.h:626: __get_rref is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:645: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:645: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/Variant.h:1169: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:1169: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1169: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1169: __move_construct_func is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:1172: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:1172: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1172: __destroy_func is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:1784: Missing spaces around != [whitespace/operators] [3] ERROR: Source/WTF/wtf/Variant.h:1785: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1786: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1786: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Variant.h:1793: Missing spaces around != [whitespace/operators] [3] ERROR: Source/WTF/wtf/Variant.h:1794: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1795: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1795: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Variant.h:1801: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1802: Use 'WTFMove()' instead of 'std::move()'. [runtime/wtf_move] [4] ERROR: Source/WTF/wtf/Variant.h:1802: Missing spaces around != [whitespace/operators] [3] ERROR: Source/WTF/wtf/Variant.h:1808: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1809: Use 'WTFMove()' instead of 'std::move()'. [runtime/wtf_move] [4] ERROR: Source/WTF/wtf/Variant.h:1809: Missing spaces around != [whitespace/operators] [3] ERROR: Source/WTF/wtf/Variant.h:1877: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1878: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1947: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:1947: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1947: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1948: Missing space after , [whitespace/comma] [3] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:41: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:41: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:42: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:42: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:45: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:50: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:50: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:51: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:58: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:59: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:65: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:66: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:144: Should have a space between // and comment [whitespace/comments] [4] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:145: Should have a space between // and comment [whitespace/comments] [4] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:158: Should have a space between // and comment [whitespace/comments] [4] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:179: Should have a space between // and comment [whitespace/comments] [4] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:180: Should have a space between // and comment [whitespace/comments] [4] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:188: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:323: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:324: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:326: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/StdLibExtras.h:336: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/StdLibExtras.h:409: Use 'using namespace std;' instead of 'using std::experimental::get;'. [build/using_std] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:410: Use 'using namespace std;' instead of 'using std::experimental::get_if;'. [build/using_std] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:411: Use 'using namespace std;' instead of 'using std::experimental::holds_alternative;'. [build/using_std] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:412: Use 'using namespace std;' instead of 'using std::experimental::visit;'. [build/using_std] [4] Total errors found: 65 in 13 files If any of these errors are false positives, please file a bug against check-webkit-style.

Sam Weinig

Comment 32 2016-08-09 10:44:59 PDT

Created attachment 285648 [details] Patch

WebKit Commit Bot

Comment 33 2016-08-09 10:46:46 PDT

Attachment 285648 [details] did not pass style-queue: ERROR: Source/WTF/wtf/Variant.h:117: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:117: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:539: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:539: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:539: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/Variant.h:539: __get_rref is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:630: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:630: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:630: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/Variant.h:630: __get_rref is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:649: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:649: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/Variant.h:1173: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:1173: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1173: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1173: __move_construct_func is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:1176: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:1176: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1176: __destroy_func is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:1533: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1533: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WTF/wtf/Variant.h:1539: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WTF/wtf/Variant.h:1546: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WTF/wtf/Variant.h:1553: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WTF/wtf/Variant.h:1698: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1701: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1728: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1731: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1788: Missing spaces around != [whitespace/operators] [3] ERROR: Source/WTF/wtf/Variant.h:1789: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1790: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1790: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Variant.h:1797: Missing spaces around != [whitespace/operators] [3] ERROR: Source/WTF/wtf/Variant.h:1798: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1799: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1799: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Variant.h:1805: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1806: Use 'WTFMove()' instead of 'std::move()'. [runtime/wtf_move] [4] ERROR: Source/WTF/wtf/Variant.h:1806: Missing spaces around != [whitespace/operators] [3] ERROR: Source/WTF/wtf/Variant.h:1812: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1813: Use 'WTFMove()' instead of 'std::move()'. [runtime/wtf_move] [4] ERROR: Source/WTF/wtf/Variant.h:1813: Missing spaces around != [whitespace/operators] [3] ERROR: Source/WTF/wtf/Variant.h:1841: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1881: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1882: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1951: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:1951: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1951: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1952: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:2053: Missing space before { [whitespace/braces] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:41: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:41: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:42: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:42: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:45: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:50: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:50: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:51: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:58: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:59: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:65: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:66: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Source/WTF/wtf/StdLibExtras.h:323: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:324: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:326: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/StdLibExtras.h:336: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/StdLibExtras.h:409: Use 'using namespace std;' instead of 'using std::experimental::get;'. [build/using_std] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:410: Use 'using namespace std;' instead of 'using std::experimental::get_if;'. [build/using_std] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:411: Use 'using namespace std;' instead of 'using std::experimental::holds_alternative;'. [build/using_std] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:412: Use 'using namespace std;' instead of 'using std::experimental::visit;'. [build/using_std] [4] Total errors found: 70 in 13 files If any of these errors are false positives, please file a bug against check-webkit-style.

Sam Weinig

Comment 34 2016-08-09 10:56:53 PDT

Created attachment 285649 [details] Patch

WebKit Commit Bot

Comment 35 2016-08-09 10:59:31 PDT

Attachment 285649 [details] did not pass style-queue: ERROR: Source/WTF/wtf/Variant.h:117: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:117: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:539: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:539: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:539: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/Variant.h:539: __get_rref is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:630: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:630: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:630: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/Variant.h:630: __get_rref is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:649: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:649: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/Variant.h:1173: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:1173: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1173: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1173: __move_construct_func is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:1176: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:1176: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1176: __destroy_func is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:1533: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1533: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WTF/wtf/Variant.h:1539: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WTF/wtf/Variant.h:1546: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WTF/wtf/Variant.h:1553: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WTF/wtf/Variant.h:1698: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1701: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1728: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1731: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1788: Missing spaces around != [whitespace/operators] [3] ERROR: Source/WTF/wtf/Variant.h:1789: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1790: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1790: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Variant.h:1797: Missing spaces around != [whitespace/operators] [3] ERROR: Source/WTF/wtf/Variant.h:1798: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1799: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1799: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Variant.h:1805: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1806: Use 'WTFMove()' instead of 'std::move()'. [runtime/wtf_move] [4] ERROR: Source/WTF/wtf/Variant.h:1806: Missing spaces around != [whitespace/operators] [3] ERROR: Source/WTF/wtf/Variant.h:1812: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1813: Use 'WTFMove()' instead of 'std::move()'. [runtime/wtf_move] [4] ERROR: Source/WTF/wtf/Variant.h:1813: Missing spaces around != [whitespace/operators] [3] ERROR: Source/WTF/wtf/Variant.h:1841: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1881: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1882: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1951: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:1951: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1951: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1952: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:2053: Missing space before { [whitespace/braces] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:41: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:41: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:42: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:42: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:45: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:50: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:50: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:51: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:58: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:59: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:65: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:66: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Source/WTF/wtf/StdLibExtras.h:323: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:324: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:326: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/StdLibExtras.h:336: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/StdLibExtras.h:409: Use 'using namespace std;' instead of 'using std::experimental::get;'. [build/using_std] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:410: Use 'using namespace std;' instead of 'using std::experimental::get_if;'. [build/using_std] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:411: Use 'using namespace std;' instead of 'using std::experimental::holds_alternative;'. [build/using_std] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:412: Use 'using namespace std;' instead of 'using std::experimental::visit;'. [build/using_std] [4] Total errors found: 70 in 13 files If any of these errors are false positives, please file a bug against check-webkit-style.

Sam Weinig

Comment 36 2016-08-09 11:08:30 PDT

Created attachment 285650 [details] Patch

WebKit Commit Bot

Comment 37 2016-08-09 11:10:35 PDT

Attachment 285650 [details] did not pass style-queue: ERROR: Source/WTF/wtf/Variant.h:117: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:117: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:539: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:539: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:539: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/Variant.h:539: __get_rref is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:630: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:630: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:630: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/Variant.h:630: __get_rref is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:649: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:649: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/Variant.h:1173: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:1173: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1173: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1173: __move_construct_func is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:1176: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:1176: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1176: __destroy_func is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:1533: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1533: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WTF/wtf/Variant.h:1539: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WTF/wtf/Variant.h:1546: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WTF/wtf/Variant.h:1553: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WTF/wtf/Variant.h:1698: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1701: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1728: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1731: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1788: Missing spaces around != [whitespace/operators] [3] ERROR: Source/WTF/wtf/Variant.h:1789: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1790: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1790: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Variant.h:1797: Missing spaces around != [whitespace/operators] [3] ERROR: Source/WTF/wtf/Variant.h:1798: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1799: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1799: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Variant.h:1805: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1806: Use 'WTFMove()' instead of 'std::move()'. [runtime/wtf_move] [4] ERROR: Source/WTF/wtf/Variant.h:1806: Missing spaces around != [whitespace/operators] [3] ERROR: Source/WTF/wtf/Variant.h:1812: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1813: Use 'WTFMove()' instead of 'std::move()'. [runtime/wtf_move] [4] ERROR: Source/WTF/wtf/Variant.h:1813: Missing spaces around != [whitespace/operators] [3] ERROR: Source/WTF/wtf/Variant.h:1841: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1881: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1882: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1951: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:1951: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1951: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1952: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:2053: Missing space before { [whitespace/braces] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:41: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:41: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:42: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:42: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:45: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:50: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:50: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:51: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:58: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:59: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:65: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:66: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Source/WTF/wtf/StdLibExtras.h:323: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:324: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:326: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/StdLibExtras.h:336: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/StdLibExtras.h:409: Use 'using namespace std;' instead of 'using std::experimental::get;'. [build/using_std] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:410: Use 'using namespace std;' instead of 'using std::experimental::get_if;'. [build/using_std] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:411: Use 'using namespace std;' instead of 'using std::experimental::holds_alternative;'. [build/using_std] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:412: Use 'using namespace std;' instead of 'using std::experimental::visit;'. [build/using_std] [4] Total errors found: 70 in 13 files If any of these errors are false positives, please file a bug against check-webkit-style.

Sam Weinig

Comment 38 2016-08-09 11:32:48 PDT

Created attachment 285653 [details] Patch

WebKit Commit Bot

Comment 39 2016-08-09 11:34:46 PDT

Attachment 285653 [details] did not pass style-queue: ERROR: Source/WTF/wtf/Variant.h:117: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:117: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:539: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:539: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:539: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/Variant.h:539: __get_rref is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:630: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:630: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:630: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/Variant.h:630: __get_rref is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:649: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:649: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/Variant.h:1173: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:1173: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1173: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1173: __move_construct_func is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:1176: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:1176: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1176: __destroy_func is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Variant.h:1533: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1533: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WTF/wtf/Variant.h:1539: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WTF/wtf/Variant.h:1546: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WTF/wtf/Variant.h:1553: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WTF/wtf/Variant.h:1698: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1701: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1728: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1731: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1788: Missing spaces around != [whitespace/operators] [3] ERROR: Source/WTF/wtf/Variant.h:1789: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1790: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1790: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Variant.h:1797: Missing spaces around != [whitespace/operators] [3] ERROR: Source/WTF/wtf/Variant.h:1798: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1799: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1799: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Variant.h:1805: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1806: Use 'WTFMove()' instead of 'std::move()'. [runtime/wtf_move] [4] ERROR: Source/WTF/wtf/Variant.h:1806: Missing spaces around != [whitespace/operators] [3] ERROR: Source/WTF/wtf/Variant.h:1812: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1813: Use 'WTFMove()' instead of 'std::move()'. [runtime/wtf_move] [4] ERROR: Source/WTF/wtf/Variant.h:1813: Missing spaces around != [whitespace/operators] [3] ERROR: Source/WTF/wtf/Variant.h:1841: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1881: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1882: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1951: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/wtf/Variant.h:1951: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:1951: Missing space before { [whitespace/braces] [5] ERROR: Source/WTF/wtf/Variant.h:1952: Missing space after , [whitespace/comma] [3] ERROR: Source/WTF/wtf/Variant.h:2053: Missing space before { [whitespace/braces] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:41: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:41: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:42: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:42: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:45: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:50: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:50: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:51: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:58: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:59: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:65: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Variant.cpp:66: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Source/WTF/wtf/StdLibExtras.h:323: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:324: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:326: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/StdLibExtras.h:336: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WTF/wtf/StdLibExtras.h:409: Use 'using namespace std;' instead of 'using std::experimental::get;'. [build/using_std] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:410: Use 'using namespace std;' instead of 'using std::experimental::get_if;'. [build/using_std] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:411: Use 'using namespace std;' instead of 'using std::experimental::holds_alternative;'. [build/using_std] [4] ERROR: Source/WTF/wtf/StdLibExtras.h:412: Use 'using namespace std;' instead of 'using std::experimental::visit;'. [build/using_std] [4] Total errors found: 70 in 13 files If any of these errors are false positives, please file a bug against check-webkit-style.

Sam Weinig

Comment 40 2016-08-09 15:38:58 PDT

Landed with: https://trac.webkit.org/changeset/204291 https://trac.webkit.org/changeset/204302

Alexey Proskuryakov

Comment 41 2016-08-12 18:39:14 PDT

r204291 made WebKit build 1 to 2 minutes slower (on Mac Pro hardware). I think that that is a sufficient reason for rolling out, please let me know soon if you disagree.

Sam Weinig

Comment 42 2016-08-14 13:15:34 PDT

(In reply to comment #41) > r204291 made WebKit build 1 to 2 minutes slower (on Mac Pro hardware). > > I think that that is a sufficient reason for rolling out, please let me know > soon if you disagree. I disagree.

Sam Weinig

Comment 43 2016-08-14 14:45:36 PDT

(In reply to comment #42) > (In reply to comment #41) > > r204291 made WebKit build 1 to 2 minutes slower (on Mac Pro hardware). > > > > I think that that is a sufficient reason for rolling out, please let me know > > soon if you disagree. > > I disagree. I filed https://bugs.webkit.org/show_bug.cgi?id=160847 to track fixing the compile time regression and am working on a fix.

Alexey Proskuryakov

Comment 44 2016-08-14 15:33:14 PDT

Thanks. If the regression remains, I will roll out - build performance is a lot more important than this refactoring.

Sam Weinig

Comment 45 2016-08-14 17:46:51 PDT

(In reply to comment #44) > Thanks. If the regression remains, I will roll out - build performance is a > lot more important than this refactoring. I don't think that is your decision to make alone.

Alexey Proskuryakov

Comment 46 2016-08-14 18:10:04 PDT

Sam, I understand that you've put a lot of effort into this patch, and into fixing fallout from it. You can certainly start a discussion about value of build performance on webkit-dev if you feel that it is appropriate. To me, the case seems so exceedingly obvious that discussing it would not be a good use of everyone's time. It will also be unnecessary to debate this specific instance if bug 160847 brings build performance back to original level.

Alexey Proskuryakov

Comment 47 2016-08-16 13:17:30 PDT

The changes in bug 160847 were an improvement, but we are still 30-40 seconds worse than before the refactoring. Since that's on a Mac Pro, the cost would be higher on a laptop computer. Sam, do you have ideas for further improvement?

Alexey Proskuryakov

Comment 48 2016-08-17 13:52:21 PDT

Sam and I looked at the graphs, and there is not enough data to believe that this patch is responsible for the remaining regression. Sadly, it won't be easy to pinpoint.

Note You need to log in before you can comment on or make changes to this bug.