Bug 156892 - [JSC] Fix build break since r199866
Summary: [JSC] Fix build break since r199866
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Joonghun Park
URL:
Keywords:
Depends on:
Blocks: 156871
  Show dependency treegraph
 
Reported: 2016-04-21 22:50 PDT by Joonghun Park
Modified: 2016-04-22 18:11 PDT (History)
9 users (show)

See Also:


Attachments
To check EWS build status (1.99 KB, patch)
2016-04-22 04:02 PDT, Joonghun Park
no flags Details | Formatted Diff | Diff
Fix a typo ignoring style check warning. cloop build was successful (2.03 KB, patch)
2016-04-22 04:14 PDT, Joonghun Park
no flags Details | Formatted Diff | Diff
Change commit log to let this patch be reviewed. (2.00 KB, patch)
2016-04-22 04:30 PDT, Joonghun Park
no flags Details | Formatted Diff | Diff
Follow up to Bejamin's quick fix (1.09 KB, patch)
2016-04-22 15:13 PDT, Joonghun Park
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joonghun Park 2016-04-21 22:50:45 PDT
Add using namespace std statement.
Comment 1 Joonghun Park 2016-04-21 22:56:21 PDT
Committed r199868: <http://trac.webkit.org/changeset/199868>
Comment 2 Csaba Osztrogonác 2016-04-22 02:05:16 PDT
(In reply to comment #1)
> Committed r199868: <http://trac.webkit.org/changeset/199868>

And Darin landed one more fix: (I don't konw which build it fixed.)
http://trac.webkit.org/changeset/199870

But now the Apple Mac cloop build is broken:

/Volumes/Data/slave/elcapitan-cloop-debug/build/Source/JavaScriptCore/runtime/MathCommon.h:61:28: error: expected unqualified-id
    if (!constant || !std::isnormal(constant))
                           ^
/Volumes/Data/slave/elcapitan-cloop-debug/build/Source/JavaScriptCore/runtime/MathCommon.h:65:9: error: no member named 'frexp' in namespace 'std'; did you mean simply 'frexp'?
    if (std::frexp(constant, &exponent) != 0.5)
        ^~~~~~~~~~
        frexp

/Volumes/Data/slave/elcapitan-cloop-debug/build/Source/JavaScriptCore/runtime/MathCommon.h:77:25: error: no member named 'ldexp' in namespace 'std'; did you mean simply 'ldexp'?
    double reciprocal = std::ldexp(1, -exponent);
                        ^~~~~~~~~~
                        ldexp

/Volumes/Data/slave/elcapitan-cloop-debug/build/Source/JavaScriptCore/runtime/MathCommon.h:78:17: error: expected unqualified-id
    ASSERT(std::isnormal(reciprocal));
                ^

Common, nobody watches bots after landing random patches? Why ????
The original patch broke all Linux builds, the buildfixes broke the 
Apple Mac cloop build.
Comment 3 Joonghun Park 2016-04-22 02:35:26 PDT
(In reply to comment #2)
> (In reply to comment #1)
> > Committed r199868: <http://trac.webkit.org/changeset/199868>
> 
> And Darin landed one more fix: (I don't konw which build it fixed.)
> http://trac.webkit.org/changeset/199870
> 
> But now the Apple Mac cloop build is broken:
> 
> /Volumes/Data/slave/elcapitan-cloop-debug/build/Source/JavaScriptCore/
> runtime/MathCommon.h:61:28: error: expected unqualified-id
>     if (!constant || !std::isnormal(constant))
>                            ^
> /Volumes/Data/slave/elcapitan-cloop-debug/build/Source/JavaScriptCore/
> runtime/MathCommon.h:65:9: error: no member named 'frexp' in namespace
> 'std'; did you mean simply 'frexp'?
>     if (std::frexp(constant, &exponent) != 0.5)
>         ^~~~~~~~~~
>         frexp
> 
> /Volumes/Data/slave/elcapitan-cloop-debug/build/Source/JavaScriptCore/
> runtime/MathCommon.h:77:25: error: no member named 'ldexp' in namespace
> 'std'; did you mean simply 'ldexp'?
>     double reciprocal = std::ldexp(1, -exponent);
>                         ^~~~~~~~~~
>                         ldexp
> 
> /Volumes/Data/slave/elcapitan-cloop-debug/build/Source/JavaScriptCore/
> runtime/MathCommon.h:78:17: error: expected unqualified-id
>     ASSERT(std::isnormal(reciprocal));
>                 ^
> 
> Common, nobody watches bots after landing random patches? Why ????
> The original patch broke all Linux builds, the buildfixes broke the 
> Apple Mac cloop build.

I'm sorry, Ossy. If it is not fixed soon, let me check it out again. My mac is at home, so I couldn't check mac cloop port in advance.
Comment 4 Joonghun Park 2016-04-22 04:02:39 PDT
Reopening to attach new patch.
Comment 5 Joonghun Park 2016-04-22 04:02:43 PDT
Created attachment 277039 [details]
To check EWS build status
Comment 6 Csaba Osztrogonác 2016-04-22 04:11:41 PDT
(In reply to comment #5)
> Created attachment 277039 [details]
> To check EWS build status

FYI: there is no EWS for cloop build.
Comment 7 Joonghun Park 2016-04-22 04:14:36 PDT
Created attachment 277040 [details]
Fix a typo ignoring style check warning. cloop build was successful
Comment 8 WebKit Commit Bot 2016-04-22 04:16:37 PDT
Attachment 277040 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/runtime/MathCommon.h:36:  Do not use 'using namespace std;'.  [build/using_namespace] [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 9 Joonghun Park 2016-04-22 04:30:00 PDT
Created attachment 277041 [details]
Change commit log to let this patch be reviewed.
Comment 10 WebKit Commit Bot 2016-04-22 04:31:20 PDT
Attachment 277041 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/runtime/MathCommon.h:36:  Do not use 'using namespace std;'.  [build/using_namespace] [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 11 Mark Lam 2016-04-22 09:49:33 PDT
Comment on attachment 277041 [details]
Change commit log to let this patch be reviewed.

r=me
Comment 12 Darin Adler 2016-04-22 09:50:28 PDT
Comment on attachment 277041 [details]
Change commit log to let this patch be reviewed.

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

> Source/JavaScriptCore/runtime/MathCommon.h:36
> +using namespace std;

I think the correct fix is to add an include so we get the "std::" versions of these rather than doing this.
Comment 13 Mark Lam 2016-04-22 09:51:54 PDT
Comment on attachment 277041 [details]
Change commit log to let this patch be reviewed.

r- per Darin's comment.
Comment 14 Benjamin Poulain 2016-04-22 15:02:18 PDT
Quick fix: http://trac.webkit.org/changeset/199913
Let's see if the bots like that better.
Comment 15 Joonghun Park 2016-04-22 15:13:42 PDT
Created attachment 277105 [details]
Follow up to Bejamin's quick fix
Comment 16 WebKit Commit Bot 2016-04-22 18:11:22 PDT
Comment on attachment 277105 [details]
Follow up to Bejamin's quick fix

Clearing flags on attachment: 277105

Committed r199943: <http://trac.webkit.org/changeset/199943>
Comment 17 WebKit Commit Bot 2016-04-22 18:11:25 PDT
All reviewed patches have been landed.  Closing bug.