Closed Bug 1249896 Opened 4 years ago Closed 4 years ago

Build with FILES_PER_UNIFIED_FILE=1 fails.

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox47 --- fixed

People

(Reporter: arai, Assigned: arai)

Details

Attachments

(9 files)

Same as bug 1234025.

Building SpiderMonkey with FILES_PER_UNIFIED_FILE=1 fails because of some missing #include, and unused variable/functions, etc.
> /Users/arai/projects/mozilla-central/js/src/asmjs/AsmJS.cpp:657:1: error: unused function 'AndOrLeft'
>       [-Werror,-Wunused-function]
> AndOrLeft(ParseNode* pn)
> ^
> /Users/arai/projects/mozilla-central/js/src/asmjs/AsmJS.cpp:663:1: error: unused function 'AndOrRight'
>       [-Werror,-Wunused-function]
> AndOrRight(ParseNode* pn)
> ^
> /Users/arai/projects/mozilla-central/js/src/asmjs/AsmJS.cpp:669:1: error: unused function 'RelationalLeft'
>       [-Werror,-Wunused-function]
> RelationalLeft(ParseNode* pn)
> ^
> /Users/arai/projects/mozilla-central/js/src/asmjs/AsmJS.cpp:675:1: error: unused function 'RelationalRight'
>       [-Werror,-Wunused-function]
> RelationalRight(ParseNode* pn)
> ^

Removed unused functions.
Attachment #8721751 - Flags: review?(jwalden+bmo)
> /Users/arai/projects/mozilla-central/js/src/builtin/SIMD.cpp:207:15: error: unused variable 'type'
>       [-Werror,-Wunused-const-variable]
> FOR_EACH_SIMD(DEFINE_DEFN_)
>               ^
> /Users/arai/projects/mozilla-central/js/src/builtin/SIMD.cpp:67:3: note: expanded from macro 'FOR_EACH_SIMD'
>   macro(Int8x16)             \
>   ^
> /Users/arai/projects/mozilla-central/js/src/builtin/SIMD.cpp:203:27: note: expanded from macro 'DEFINE_DEFN_'
>     static const SimdType type = SimdType::TypeName;                 \
>                           ^

looks like type member is no more used.
perhaps, it's still used while debugging?
Attachment #8721752 - Flags: review?(bbouvier)
> /Users/arai/projects/mozilla-central/js/src/jit/MCallOptimize.cpp:3309:53: error: use of undeclared identifier
>       'GenericNaN'; did you mean 'JS::GenericNaN'?
>             defVal = MConstant::NewFloat32(alloc(), GenericNaN());
>                                                     ^~~~~~~~~~
>                                                     JS::GenericNaN

added namespace.
Attachment #8721753 - Flags: review?(luke)
> /Users/arai/projects/mozilla-central/js/src/jsopcode.cpp:950:9: error: code will never be executed
>       [-Werror,-Wunreachable-code]
>         int i;
>         ^~~~~~

-Wunreachable-code complains about this style.  moved `int i` declaration out of switch block.

>    switch (JOF_TYPE(cs->format)) {
> ...
>       {
>         int i;
> 
>       case JOF_UINT16:
>         i = (int)GET_UINT16(pc);
>         goto print_int;
> ...
>       print_int:
>         Sprint(sp, " %d", i);
>         break;
>       }
> ...
>     }
Attachment #8721754 - Flags: review?(luke)
> /Users/arai/projects/mozilla-central/js/src/proxy/ScriptedDirectProxyHandler.cpp:132:1: error: unused function
>       'ReportInvalidTrapResult' [-Werror,-Wunused-function]
> ReportInvalidTrapResult(JSContext* cx, JSObject* proxy, JSAtom* atom)
> ^

removed unused function
Attachment #8721755 - Flags: review?(efaustbmo)
> /Users/arai/projects/mozilla-central/js/src/vm/MemoryMetrics.cpp:359:30: error: use of undeclared identifier 'Arena'
>     size_t allocationSpace = Arena::thingsSpan(arena->aheader.getAllocKind());
>                              ^

added `#include "gc/Heap.h"` and gc namespace.
Attachment #8721756 - Flags: review?(terrence)
> /Users/arai/projects/mozilla-central/js/src/shell/jsshell.h:27:1: error: unused function 'my_ErrorReporter'
>       [-Werror,-Wunused-function]
> my_ErrorReporter(JSContext* cx, const char* message, JSErrorReport* report);
> ^

removed "static" from declaration.
Attachment #8721757 - Flags: review?(sphink)
> /Users/arai/projects/mozilla-central/js/src/shell/js.cpp:392:9: error: code will never be executed
>       [-Werror,-Wunreachable-code]
>     if (len && !ferror(file))
>         ^~~

"while" loop before this line does not break.

https://dxr.mozilla.org/mozilla-central/rev/69ec3dc408a2a720cb2b8210fea33e3504aeec22/js/src/shell/js.cpp#362
Attachment #8721758 - Flags: review?(jwalden+bmo)
> /Users/arai/projects/mozilla-central/js/src/shell/js.cpp:736:38: error: use of undeclared identifier
>       'my_GetErrorMessage'; did you mean 'GetErrorMessage'?
>             JS_ReportErrorNumber(cx, my_GetErrorMessage, nullptr,
>                                      ^~~~~~~~~~~~~~~~~~
>                                      GetErrorMessage

js.cpp also needs declaration in jsshell.h.
Attachment #8721759 - Flags: review?(sphink)
Comment on attachment 8721751 [details] [diff] [review]
Part 1: Remove unused AndOrLeft, AndOrRight, RelationalLeft, and RelationalRight.

Review of attachment 8721751 [details] [diff] [review]:
-----------------------------------------------------------------

Bah.  These methods really all should be just plain statics, not static inlines -- that would have caught this.
Attachment #8721751 - Flags: review?(jwalden+bmo) → review+
Comment on attachment 8721758 [details] [diff] [review]
Part 8: Remove unreachable code from GetLine.

Review of attachment 8721758 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/shell/js.cpp
@@ -386,5 @@
>              }
>              buffer = tmp;
>          }
>          current = buffer + len;
>      }

Could you change this loop from while (true) to do ... while (true), so the reader sees falling off the end is to keep looping?  With my limited context-seeing here this was a question I had to look up to answer.
Attachment #8721758 - Flags: review?(jwalden+bmo) → review+
Attachment #8721753 - Flags: review?(luke) → review+
Attachment #8721754 - Flags: review?(luke) → review+
Comment on attachment 8721756 [details] [diff] [review]
Part 6: Add gc namespace for Arena::thingsSpan.

Review of attachment 8721756 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!
Attachment #8721756 - Flags: review?(terrence) → review+
Comment on attachment 8721757 [details] [diff] [review]
Part 7: Remove unnecessary static from my_ErrorReporter.

Review of attachment 8721757 [details] [diff] [review]:
-----------------------------------------------------------------

Ah. First I was going to say this should be removed from the header file entirely, but it looks like it gets used in a test program, so r=me.
Attachment #8721757 - Flags: review?(sphink) → review+
Attachment #8721759 - Flags: review?(sphink) → review+
Comment on attachment 8721755 [details] [diff] [review]
Part 5: Remove unused ReportInvalidTrapResult.

Review of attachment 8721755 [details] [diff] [review]:
-----------------------------------------------------------------

r=me
Attachment #8721755 - Flags: review?(efaustbmo) → review+
Comment on attachment 8721752 [details] [diff] [review]
Part 2: Remove unused type member from SIMD Int8x16Defn etc.

Review of attachment 8721752 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry for the long review time. If it builds, then feel free to remove it indeed. Thanks for doing this!
Attachment #8721752 - Flags: review?(bbouvier) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/6ae0e2d591f61d8d248ce7d404a63bbe52843cbc
Bug 1249896 - Part 1: Remove unused AndOrLeft, AndOrRight, RelationalLeft, and RelationalRight. r=jwalden

https://hg.mozilla.org/integration/mozilla-inbound/rev/914adf5734666066a75c751a462f06f19ecc4fb7
Bug 1249896 - Part 2: Remove unused type member from SIMD Int8x16Defn etc. r=bbouvier

https://hg.mozilla.org/integration/mozilla-inbound/rev/e286e902c15ef24d3a28cfb4dd56f22a182cedb4
Bug 1249896 - Part 3: Add JS namespace for GenericNaN. r=luke

https://hg.mozilla.org/integration/mozilla-inbound/rev/0a46418cbe6d9d5c173eefe11baaedb837bd0538
Bug 1249896 - Part 4: Avoid declaring variable between cases in switch. r=luke

https://hg.mozilla.org/integration/mozilla-inbound/rev/f16b3ba56be8d4965c2b4d9110689843e06b1353
Bug 1249896 - Part 5: Remove unused ReportInvalidTrapResult. r=efaust

https://hg.mozilla.org/integration/mozilla-inbound/rev/d0a434a6cae4fa0891bc7b6a0fd95310ad4ac816
Bug 1249896 - Part 6: Add gc namespace for Arena::thingsSpan. r=terrence

https://hg.mozilla.org/integration/mozilla-inbound/rev/39d4cf16c55451b2eb7b1aa8cdf8891c48077ddb
Bug 1249896 - Part 7: Remove unnecessary static from my_ErrorReporter. r=sfink

https://hg.mozilla.org/integration/mozilla-inbound/rev/38683766636ad43d13fe2f1bb81bbdafbe288d4b
Bug 1249896 - Part 8: Remove unreachable code from GetLine. r=jwalden

https://hg.mozilla.org/integration/mozilla-inbound/rev/b76de3f353b8e84dd404f3e8080d0f0c85efbca2
Bug 1249896 - Part 9: Include shell/jsshell.h in js.cpp. r=sfink
Might as well make this leave-open, I'm sure it's gonna break again.
You need to log in before you can comment on or make changes to this bug.