Bug 176301 – [WTF] Add C++03 allocator interface for GCC < 6

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Yusuke Suzuki]

[WTF] Add C++03 allocator interface for GCC < 6

[Yusuke Suzuki]

Created attachment 319771 [details]
Patch

[Build Bot]

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


ERROR: Source/WTF/wtf/FastMalloc.h:162:  max_size is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WTF/wtf/FastMalloc.h:167:  select_on_container_copy_construction is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 2 in 4 files


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

[Yusuke Suzuki]

Created attachment 319772 [details]
Patch

[Yusuke Suzuki]

Comment on attachment 319772 [details]
Patch

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

> Source/WTF/wtf/FastMalloc.h:153
> +        new (const_cast<void*>(p)) U(std::forward<Args>(args)...);

https://webkit-queues.webkit.org/results/4441104 This is required. U* can be const pointer. But the caller ensures that this can be mutable memory area. So const_cast is OK.

[Build Bot]

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


ERROR: Source/WTF/wtf/FastMalloc.h:162:  max_size is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WTF/wtf/FastMalloc.h:167:  select_on_container_copy_construction is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 2 in 4 files


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

[Yusuke Suzuki]

Created attachment 319773 [details]
Patch

[Build Bot]

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


ERROR: Source/WTF/wtf/FastMalloc.h:162:  max_size is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WTF/wtf/FastMalloc.h:167:  select_on_container_copy_construction is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 2 in 4 files


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

[Darin Adler]

Comment on attachment 319773 [details]
Patch

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

> Source/WTF/wtf/FastMalloc.h:129
> +    // This allocator also supports pre-C++11 STL allocator interface. This is a workaround for GCC < 6, which std::list
> +    // does not support C++11 allocator.

Can we put an #if around this that enables it only for that older GCC? That way we will remember to delete the code later once we drop support for that older compiler, and we will also know it’s safe to do so since we won’t accidentally depend on it, on other platforms.

[Yusuke Suzuki]

Comment on attachment 319773 [details]
Patch

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

Thanks!

>> Source/WTF/wtf/FastMalloc.h:129
>> +    // does not support C++11 allocator.
> 
> Can we put an #if around this that enables it only for that older GCC? That way we will remember to delete the code later once we drop support for that older compiler, and we will also know it’s safe to do so since we won’t accidentally depend on it, on other platforms.

Make sense. Changed!

[Yusuke Suzuki]

Committed r221552: <http://trac.webkit.org/changeset/221552>

[Chris Dumez]

Broke my build:
CompileC AtomicStringImplCF.o
In file included from /Volumes/Data/WebKit/OpenSource/Source/WTF/wtf/text/cf/AtomicStringImplCF.cpp:26:
In file included from /Volumes/Data/WebKit/OpenSource/Source/WTF/config.h:31:
/Volumes/Data/WebKit/OpenSource/WebKitBuild/Debug/usr/local/include/wtf/FastMalloc.h:128:43: error: token is not a valid binary operator in a preprocessor subexpression
#if COMPILER(GCC) && !GCC_VERSION_AT_LEAST(6, 0, 0)

[Chris Dumez]

Reverted r221552 for reason:

Broke the build

Committed r221553: <http://trac.webkit.org/changeset/221553>

[Yusuke Suzuki]

(In reply to Chris Dumez from comment #11)
> Broke my build:
> CompileC AtomicStringImplCF.o
> In file included from
> /Volumes/Data/WebKit/OpenSource/Source/WTF/wtf/text/cf/AtomicStringImplCF.
> cpp:26:
> In file included from /Volumes/Data/WebKit/OpenSource/Source/WTF/config.h:31:
> /Volumes/Data/WebKit/OpenSource/WebKitBuild/Debug/usr/local/include/wtf/
> FastMalloc.h:128:43: error: token is not a valid binary operator in a
> preprocessor subexpression
> #if COMPILER(GCC) && !GCC_VERSION_AT_LEAST(6, 0, 0)

It seems that Compiler.h is not included here.

[Yusuke Suzuki]

(In reply to Yusuke Suzuki from comment #13)
> (In reply to Chris Dumez from comment #11)
> > Broke my build:
> > CompileC AtomicStringImplCF.o
> > In file included from
> > /Volumes/Data/WebKit/OpenSource/Source/WTF/wtf/text/cf/AtomicStringImplCF.
> > cpp:26:
> > In file included from /Volumes/Data/WebKit/OpenSource/Source/WTF/config.h:31:
> > /Volumes/Data/WebKit/OpenSource/WebKitBuild/Debug/usr/local/include/wtf/
> > FastMalloc.h:128:43: error: token is not a valid binary operator in a
> > preprocessor subexpression
> > #if COMPILER(GCC) && !GCC_VERSION_AT_LEAST(6, 0, 0)
> 
> It seems that Compiler.h is not included here.

Ah, no. we should do,

#if COMPILER(GCC)
#if !GCC_VERSION_AT_LEAST(6, 0, 0)

[Chris Dumez]

(In reply to Yusuke Suzuki from comment #14)
> (In reply to Yusuke Suzuki from comment #13)
> > (In reply to Chris Dumez from comment #11)
> > > Broke my build:
> > > CompileC AtomicStringImplCF.o
> > > In file included from
> > > /Volumes/Data/WebKit/OpenSource/Source/WTF/wtf/text/cf/AtomicStringImplCF.
> > > cpp:26:
> > > In file included from /Volumes/Data/WebKit/OpenSource/Source/WTF/config.h:31:
> > > /Volumes/Data/WebKit/OpenSource/WebKitBuild/Debug/usr/local/include/wtf/
> > > FastMalloc.h:128:43: error: token is not a valid binary operator in a
> > > preprocessor subexpression
> > > #if COMPILER(GCC) && !GCC_VERSION_AT_LEAST(6, 0, 0)
> > 
> > It seems that Compiler.h is not included here.
> 
> Ah, no. we should do,
> 
> #if COMPILER(GCC)
> #if !GCC_VERSION_AT_LEAST(6, 0, 0)

Please go through EWS before re-landing :)

[Yusuke Suzuki]

Created attachment 319793 [details]
Patch for landing

[Yusuke Suzuki]

(In reply to Chris Dumez from comment #15)
> (In reply to Yusuke Suzuki from comment #14)
> > (In reply to Yusuke Suzuki from comment #13)
> > > (In reply to Chris Dumez from comment #11)
> > > > Broke my build:
> > > > CompileC AtomicStringImplCF.o
> > > > In file included from
> > > > /Volumes/Data/WebKit/OpenSource/Source/WTF/wtf/text/cf/AtomicStringImplCF.
> > > > cpp:26:
> > > > In file included from /Volumes/Data/WebKit/OpenSource/Source/WTF/config.h:31:
> > > > /Volumes/Data/WebKit/OpenSource/WebKitBuild/Debug/usr/local/include/wtf/
> > > > FastMalloc.h:128:43: error: token is not a valid binary operator in a
> > > > preprocessor subexpression
> > > > #if COMPILER(GCC) && !GCC_VERSION_AT_LEAST(6, 0, 0)
> > > 
> > > It seems that Compiler.h is not included here.
> > 
> > Ah, no. we should do,
> > 
> > #if COMPILER(GCC)
> > #if !GCC_VERSION_AT_LEAST(6, 0, 0)
> 
> Please go through EWS before re-landing :)

Thanks, I'll land it after EWS becomes green :)

[Build Bot]

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


ERROR: Source/WTF/wtf/FastMalloc.h:166:  max_size is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WTF/wtf/FastMalloc.h:171:  select_on_container_copy_construction is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 2 in 4 files


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

[Yusuke Suzuki]

Comment on attachment 319793 [details]
Patch for landing

OK, let's go.

[WebKit Commit Bot]

Comment on attachment 319793 [details]
Patch for landing

Rejecting attachment 319793 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'validate-changelog', '--check-oops', '--non-interactive', 319793, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

ChangeLog entry in Source/JavaScriptCore/ChangeLog contains OOPS!.

Full output: http://webkit-queues.webkit.org/results/4444761

[Yusuke Suzuki]

Committed r221560: <http://trac.webkit.org/changeset/221560>

[Yusuke Suzuki]

Committed r221575: <http://trac.webkit.org/changeset/221575>

[Radar WebKit Bug Importer]

<rdar://problem/34693673>