Bug 83999 - Make NullPtr.h compile with clang -std=c++11 and libstdc++4.2
Summary: Make NullPtr.h compile with clang -std=c++11 and libstdc++4.2
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nico Weber
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-04-15 13:25 PDT by Nico Weber
Modified: 2012-04-16 00:14 PDT (History)
3 users (show)

See Also:


Attachments
Patch (1.89 KB, patch)
2012-04-15 13:27 PDT, Nico Weber
no flags Details | Formatted Diff | Diff
Patch (1.86 KB, patch)
2012-04-15 13:35 PDT, Nico Weber
no flags Details | Formatted Diff | Diff
Patch (1.73 KB, patch)
2012-04-15 13:49 PDT, Nico Weber
no flags Details | Formatted Diff | Diff
Patch (1.72 KB, patch)
2012-04-15 13:53 PDT, Nico Weber
no flags Details | Formatted Diff | Diff
Patch (1.72 KB, patch)
2012-04-15 13:55 PDT, Nico Weber
no flags Details | Formatted Diff | Diff
Patch (1.54 KB, patch)
2012-04-15 16:02 PDT, Nico Weber
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ec2-cr-linux-03 (6.23 MB, application/zip)
2012-04-15 17:15 PDT, WebKit Review Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Nico Weber 2012-04-15 13:25:18 PDT
Make NullPtr.h compile with clang -std=c++11 and libstdc++4.2
Comment 1 Nico Weber 2012-04-15 13:27:42 PDT
Created attachment 137244 [details]
Patch
Comment 2 Nico Weber 2012-04-15 13:35:40 PDT
Created attachment 137245 [details]
Patch
Comment 3 Nico Weber 2012-04-15 13:37:31 PDT
Comment on attachment 137244 [details]
Patch

cpp file too
Comment 4 Nico Weber 2012-04-15 13:49:49 PDT
Created attachment 137246 [details]
Patch
Comment 5 WebKit Review Bot 2012-04-15 13:51:50 PDT
Attachment 137246 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WTF/ChangeLog', u'Source/WTF/wtf/Nu..." exit_code: 1
Source/WTF/wtf/NullPtr.h:50:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 6 Nico Weber 2012-04-15 13:53:16 PDT
Created attachment 137247 [details]
Patch
Comment 7 Nico Weber 2012-04-15 13:55:12 PDT
Created attachment 137248 [details]
Patch
Comment 8 Nico Weber 2012-04-15 13:59:43 PDT
(The __GLIBCXX__ version that belongs to gcc4.6 is from http://gcc.gnu.org/onlinedocs/libstdc++/manual/abi.html )
Comment 9 Sam Weinig 2012-04-15 15:44:17 PDT
Comment on attachment 137248 [details]
Patch

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

> Source/WTF/wtf/NullPtr.h:48
> +// libstdc++ supports nullptr_t starting with gcc 4.6.
> +#if defined(__GLIBCXX__) && __GLIBCXX__ < 20110325
> +#define HAS_NULLPTR_T 0
>  #else
> +#define HAS_NULLPTR_T 1
> +#endif
>  
> +#if !HAS_NULLPTR_T
> +// Compiler supports nullptr, but standard library doesn't have nullptr_t.

I don't think this extra HAS_NULLPTR_T macro is helpful.  I think the typedef can just go in the correct macro branch.  Also, is there a case where this might be hit, but the compiler doesn't support decltype?
Comment 10 Nico Weber 2012-04-15 15:59:52 PDT
(In reply to comment #9)
> (From update of attachment 137248 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=137248&action=review
> 
> > Source/WTF/wtf/NullPtr.h:48
> > +// libstdc++ supports nullptr_t starting with gcc 4.6.
> > +#if defined(__GLIBCXX__) && __GLIBCXX__ < 20110325
> > +#define HAS_NULLPTR_T 0
> >  #else
> > +#define HAS_NULLPTR_T 1
> > +#endif
> >  
> > +#if !HAS_NULLPTR_T
> > +// Compiler supports nullptr, but standard library doesn't have nullptr_t.
> 
> I don't think this extra HAS_NULLPTR_T macro is helpful.  I think the typedef can just go in the correct macro branch.

Done, thanks.

>  Also, is there a case where this might be hit, but the compiler doesn't support decltype?

It's possible :-) gcc added decltype in 4.3 and nullptr in 4.6 (http://en.wikipedia.org/wiki/Decltype , http://gcc.gnu.org/projects/cxx0x.html). MSVC added decltype in 2010, which is what Compiler.h checks for to set WTF_COMPILER_SUPPORTS_CXX_NULLPTR too. (_MSC_VER 1600 is 2010 according to http://msdn.microsoft.com/en-us/library/b0084kay.aspx ). I tested that this works with clang.

So it should be fine at least with gcc, msvc, and clang.
Comment 11 Nico Weber 2012-04-15 16:02:00 PDT
Created attachment 137251 [details]
Patch
Comment 12 Nico Weber 2012-04-15 16:07:44 PDT
> >  Also, is there a case where this might be hit, but the compiler doesn't support decltype?
> 
> It's possible :-) gcc added decltype in 4.3 and nullptr in 4.6 (http://en.wikipedia.org/wiki/Decltype , http://gcc.gnu.org/projects/cxx0x.html). MSVC added decltype in 2010, which is what Compiler.h checks for to set WTF_COMPILER_SUPPORTS_CXX_NULLPTR too. (_MSC_VER 1600 is 2010 according to http://msdn.microsoft.com/en-us/library/b0084kay.aspx ). I tested that this works with clang.
> 
> So it should be fine at least with gcc, msvc, and clang.

…also, that code is only compiled when libstdc++ from a gcc earlier than gcc4.6 is used. If gcc is used, this means its version is < 4.6 too, so it doesn't support nullptr and won't hit this block. So this can only happen with clang and libstdc++ (and other compilers that use libstdc++ that I don't know about).
Comment 13 WebKit Review Bot 2012-04-15 17:14:56 PDT
Comment on attachment 137251 [details]
Patch

Attachment 137251 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/12412076

New failing tests:
fast/repaint/fixed-table-overflow-zindex.html
fast/repaint/scroll-fixed-reflected-layer.html
fast/repaint/fixed-scale.html
fast/repaint/fixed-tranformed.html
Comment 14 WebKit Review Bot 2012-04-15 17:15:02 PDT
Created attachment 137254 [details]
Archive of layout-test-results from ec2-cr-linux-03

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-03  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment 15 WebKit Review Bot 2012-04-16 00:14:46 PDT
Comment on attachment 137251 [details]
Patch

Clearing flags on attachment: 137251

Committed r114227: <http://trac.webkit.org/changeset/114227>
Comment 16 WebKit Review Bot 2012-04-16 00:14:59 PDT
All reviewed patches have been landed.  Closing bug.