Some internal projects include wtf headers and build with C++11

[Keith Miller]

Some internal projects include wtf headers and build with C++11

[Keith Miller]

Created attachment 352887 [details]
Patch

[Alex Christensen]

Comment on attachment 352887 [details]
Patch

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

> Source/WTF/ChangeLog:3
> +        Some internal projects include wtf headers and build with C++11

Those projects should probably update *and* stop including wtf headers.

[Keith Miller]

Created attachment 352903 [details]
Patch

[Keith Miller]

Comment on attachment 352887 [details]
Patch

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

>> Source/WTF/ChangeLog:3
>> +        Some internal projects include wtf headers and build with C++11
> 
> Those projects should probably update *and* stop including wtf headers.

Ugh, I agree... I'm working on it internally <rdar://problem/45395767> but for now we need to make sure B&I doesn't roll us back again.

[Alexey Proskuryakov]

Comment on attachment 352903 [details]
Patch

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

> Source/WTF/wtf/MathExtras.h:215
>  template <typename T> constexpr unsigned getLSBSet(T value)

Downstream projects don't need getLSBSet. Would this more straightforward fix work?

// FIXME: Remove the #if check once downstream projects stop including WTF, and/or they upgrade to C++14 (rdar://........)
#if WTF_CPP_STD_VER >= 14
template <typename T> constexpr unsigned getLSBSet(T value)
{
...
}
#endif

[Keith Miller]

Comment on attachment 352903 [details]
Patch

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

>> Source/WTF/wtf/MathExtras.h:215
>>  template <typename T> constexpr unsigned getLSBSet(T value)
> 
> Downstream projects don't need getLSBSet. Would this more straightforward fix work?
> 
> // FIXME: Remove the #if check once downstream projects stop including WTF, and/or they upgrade to C++14 (rdar://........)
> #if WTF_CPP_STD_VER >= 14
> template <typename T> constexpr unsigned getLSBSet(T value)
> {
> ...
> }
> #endif

In theory but there's always the risk that some other header in WebKit uses getLSBSet and you'd get an undefined identifier build error.

[Alexey Proskuryakov]

Sure, but in theory some other header in WebKit could be using it and expecting it to be constexpr.

[Keith Miller]

(In reply to Alexey Proskuryakov from comment #7)
> Sure, but in theory some other header in WebKit could be using it and
> expecting it to be constexpr.

True, but then neither solution would work. This is mostly just a stopgap until projects including WTF headers either stop or switch to C++14.

[Keith Miller]

Created attachment 353029 [details]
Patch

[Alexey Proskuryakov]

The latest patch breaks the build.

Have you considered adding a canary file to WTF that builds these headers with C++11? That way we can catch build failures earlier.

[Keith Miller]

Created attachment 353059 [details]
Patch

[Keith Miller]

Created attachment 353078 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 353078 [details]
Patch

Clearing flags on attachment: 353078

Committed r237448: <https://trac.webkit.org/changeset/237448>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/45581277>