1377951 - Unused variable warning-as-error in Mingw in ArrayBufferObject.cpp

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement)

Description

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

In js/src/vm/ArrayBufferObject.cpp, mappedSizeWithHeader is not used unless we are on non-Windows or are using Valgrind. (I don't believe we are ever using valgrind on Windows.)

This causes a warning-as-error:

> /home/worker/workspace/build/src/js/src/vm/ArrayBufferObject.cpp:685:12: error: unused variable 'mappedSizeWithHeader' [-Werror=unused-variable]
>      size_t mappedSizeWithHeader = header->mappedSize() + gc::SystemPageSize();

Comment 1

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Attachment: Bug 1377951 Move a variable so it does not generate unused variable warnings (as errors) in MinGW

Review commit: https://reviewboard.mozilla.org/r/154046/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/154046/

Comment 2

[Tooru Fujisawa [:arai]]

Comment on attachment 8883085 [details]
Bug 1377951 Move a variable so it does not generate unused variable warnings (as errors) in MinGW

https://reviewboard.mozilla.org/r/154046/#review159182

::: js/src/vm/ArrayBufferObject.cpp:690
(Diff revision 1)
>  
>      MemProfiler::RemoveNative(base);
>  # ifdef XP_WIN
>      VirtualFree(base, 0, MEM_RELEASE);
>  # else  // XP_WIN
> +    size_t mappedSizeWithHeader = header->mappedSize() + gc::SystemPageSize();

mappedSizeWithHeader is used also in VALGRIND_ENABLE_ADDR_ERROR_REPORTING_IN_RANGE, so moving the definition here is not correct.

Comment 3

[Tooru Fujisawa [:arai]]

Comment on attachment 8883085 [details]
Bug 1377951 Move a variable so it does not generate unused variable warnings (as errors) in MinGW

https://reviewboard.mozilla.org/r/154046/#review159186

sorry, I overlooked the reasoning in comment #0.
clearing r- for now

Comment 4

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Comment on attachment 8883085 [details]
Bug 1377951 Move a variable so it does not generate unused variable warnings (as errors) in MinGW

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/154046/diff/1-2/

Comment 5

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Rebased patch, try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1b076ab1f7cc18856ea52237c5f975ed1b56ffd9

Comment 6

[Tooru Fujisawa [:arai]]

Comment on attachment 8883085 [details]
Bug 1377951 Move a variable so it does not generate unused variable warnings (as errors) in MinGW

https://reviewboard.mozilla.org/r/154046/#review176272

we could re-think about the conditions once someone hit the issue on windows with configuration that valgrind is enabled (I cannot think of the case tho, there might be some use case).

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/66d6a3d2b97f
Move a variable so it does not generate unused variable warnings (as errors) in MinGW r=arai

Comment 8

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/66d6a3d2b97f