WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
116352
REGRESSION (
r149184
): Build errors in RefPtr.h when building with Clang, C++98 standard
https://bugs.webkit.org/show_bug.cgi?id=116352
Summary
REGRESSION (r149184): Build errors in RefPtr.h when building with Clang, C++9...
Zan Dobersek
Reported
2013-05-17 14:31:05 PDT
r149184
introduced move semantics for the RefPtr, conditioning them with the compiler's rvalue references support.
http://trac.webkit.org/changeset/149184
The implementation uses the std::move method, supported only in C++11. Still, the Clang compiler can support rvalue references even in C++98 mode due to the extension that is provided and detected via the __has_extension macro in Compiler.h[1]. The __has_extension macro is used intentionally instead of __has_feature, the switch was made in
r119162
.
http://trac.webkit.org/changeset/119162
This causes compilation errors due to undeclared std::move when building JSC (which AFAIK nobody builds in C++11 std), using Clang 3.2, libstdc++ on the GTK port. Like said, with this configuration, the rvalue references support is enabled, but the C++11 standard is not, ergo the failures. Same errors are occurring in WTFString.cpp, where std::move is used in String::isolatedCopy when reference-qualified functions support is present. Again the __has_extension macro is used, so the support is enabled even when not building in C++11 std, resulting in build errors due to undeclared std::move. That change was introduced in
r149372
.
http://trac.webkit.org/changeset/149372
I've verified that with the given configuration, using __has_feature instead of __has_extension fixes the problem, but I'm not sure if that's OK given the change log in
r119132
which specifically switched to __has_extension for some Clang features so they could be used even when using the C++98 standard.
http://trac.webkit.org/changeset/119132
[1]
http://trac.webkit.org/browser/trunk/Source/WTF/wtf/Compiler.h#L56
[2]
http://trac.webkit.org/browser/trunk/Source/WTF/wtf/text/WTFString.cpp#L657
Attachments
Patch
(4.15 KB, patch)
2013-06-17 07:52 PDT
,
Zan Dobersek
no flags
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Anders Carlsson
Comment 1
2013-05-17 15:35:07 PDT
I’m fine with changing it; on Mac we build all of WebKit with C++11.
Zan Dobersek
Comment 2
2013-05-18 11:57:15 PDT
Would it be OK to replace all the uses of __has_extension with __has_feature? This would basically enforce to build either in strict C++98 or strict C++11 standard mode, with no extensions provided by Clang for C++98 taken into account.
Sam Weinig
Comment 3
2013-05-19 13:20:36 PDT
(In reply to
comment #2
)
> Would it be OK to replace all the uses of __has_extension with __has_feature? > > This would basically enforce to build either in strict C++98 or strict C++11 standard mode, with no extensions provided by Clang for C++98 taken into account.
Why is anyone building with Clang and C++98. I think we should strive for Clang plus at least C++11.
Zan Dobersek
Comment 4
2013-05-20 04:29:08 PDT
That would work for the GTK port due to limiting the supported compilers. I'm not sure how ambitious other ports are regarding Clang support. Then again, if C++11 is made mandatory when using Clang, using __has_extension doesn't have much meaning at the moment since these features are part of C++11 so __has_feature should cover proper detection.
Zan Dobersek
Comment 5
2013-06-17 07:52:23 PDT
Created
attachment 204823
[details]
Patch
Zan Dobersek
Comment 6
2013-06-18 00:00:34 PDT
Comment on
attachment 204823
[details]
Patch Clearing flags on attachment: 204823 Committed
r151670
: <
http://trac.webkit.org/changeset/151670
>
Zan Dobersek
Comment 7
2013-06-18 00:00:41 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug