Bug 160847 – Speed up compile times by not including wtf/Variant.h so much

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Template Framework

[Sam Weinig]

Speed up compile times by not including wtf/Variant.h so much.

[Sam Weinig]

Created attachment 286042 [details]
Patch

[WebKit Commit Bot]

Attachment 286042 [details] did not pass style-queue:


ERROR: Source/WTF/wtf/Variant.h:2064:  Use 'using namespace std;' instead of 'using std::experimental::get;'.  [build/using_std] [4]
ERROR: Source/WTF/wtf/Variant.h:2065:  Use 'using namespace std;' instead of 'using std::experimental::get_if;'.  [build/using_std] [4]
ERROR: Source/WTF/wtf/Variant.h:2066:  Use 'using namespace std;' instead of 'using std::experimental::holds_alternative;'.  [build/using_std] [4]
ERROR: Source/WTF/wtf/Variant.h:2067:  Use 'using namespace std;' instead of 'using std::experimental::visit;'.  [build/using_std] [4]
Total errors found: 4 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Sam Weinig]

Created attachment 286044 [details]
Patch

[WebKit Commit Bot]

Attachment 286044 [details] did not pass style-queue:


ERROR: Source/WTF/wtf/Variant.h:2064:  Use 'using namespace std;' instead of 'using std::experimental::get;'.  [build/using_std] [4]
ERROR: Source/WTF/wtf/Variant.h:2065:  Use 'using namespace std;' instead of 'using std::experimental::get_if;'.  [build/using_std] [4]
ERROR: Source/WTF/wtf/Variant.h:2066:  Use 'using namespace std;' instead of 'using std::experimental::holds_alternative;'.  [build/using_std] [4]
ERROR: Source/WTF/wtf/Variant.h:2067:  Use 'using namespace std;' instead of 'using std::experimental::visit;'.  [build/using_std] [4]
Total errors found: 4 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Saam Barati]

Comment on attachment 286044 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=286044&action=review

r=me with Windows building

> Source/WTF/wtf/Forward.h:21
> +#pragma once

Might as well update the copyright as well

[Alex Christensen]

When I apply this patch, 100% of the time I get this error on Visual Studio when building Variant.cpp in TestWebKitAPI:

[3/6] Building CXX object Tools\TestWebKitAPI\CMakeFiles\TestWTFLib.dir\Tests\WTF\Variant.cpp.obj
FAILED: C:\PROGRA~2\MICROS~3.0\VC\bin\amd64\cl.exe <lots of build flags> ..\..\Tools\TestWebKitAPI\Tests\WTF\Variant.cpp
C:\Users\Alex\Documents\WinCairoBot\win-cairo-release\build\WebKitBuild\Release\DerivedSources\ForwardingHeaders\wtf/Hasher.h(43): fatal error C1001: An internal error has occurred in the compiler.
(compiler file 'f:\dd\vctools\compiler\cxxfe\sl\p1\c\symbols.c', line 23294)
 To work around this problem, try simplifying or changing the program near the locations listed above.

[Alex Christensen]

Comment on attachment 286044 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=286044&action=review

> Source/WTF/wtf/Variant.h:2067
> +template<typename... Types>
> +using variant = std::experimental::variant<Types...>;
> +
> +using std::experimental::get;
> +using std::experimental::get_if;
> +using std::experimental::holds_alternative;
> +using std::experimental::visit;

I verified that getting rid of all of this and explicitly using std::experimental::... fixes the Windows build.

[Alex Christensen]

closing namespace std and reopening it doesn't fix the windows build, either.

[Alex Christensen]

Getting rid of the experimental namespace also fixes the windows build.  We will run into issues when we start using compilers that have std::variant implementations, though.

[Sam Weinig]

Created attachment 286118 [details]
Patch

[Alex Christensen]

Created attachment 286120 [details]
Patch

[WebKit Commit Bot]

Attachment 286120 [details] did not pass style-queue:


ERROR: Source/WTF/wtf/Variant.h:2039:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WTF/wtf/Variant.h:2040:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WTF/wtf/Variant.h:2046:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WTF/wtf/Variant.h:2049:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 4 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Alex Christensen]

Created attachment 286130 [details]
Patch

[WebKit Commit Bot]

Attachment 286130 [details] did not pass style-queue:


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: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:66:  Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b)  [readability/check] [2]
Total errors found: 6 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Commit Bot]

Comment on attachment 286130 [details]
Patch

Clearing flags on attachment: 286130

Committed r204493: <http://trac.webkit.org/changeset/204493>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.