1480306 - Doesn't compile: TokenStream.cpp:629:18: error: unused variable 'hasLineOfContext'

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

I get this error using Apple LLVM version 8.0.0 (clang-800.0.42.1). Seems legit [1], or is this some new accepted convention?

15:35.20 In file included from /Users/Jan/moz/mozilla-central/obj-x86_64-apple-darwin12.2.1-opt/js/src/Unified_cpp_js_src7.cpp:11:
15:35.20 /Users/Jan/moz/mozilla-central/js/src/frontend/TokenStream.cpp:629:18: error: unused variable 'hasLineOfContext' [-Werror,-Wunused-variable]
15:35.20         if (bool hasLineOfContext = anyChars.fillExcludingContext(&err, offset)) {
15:35.20                  ^
15:35.20 1 error generated.
15:35.21 make[4]: *** [Unified_cpp_js_src7.o] Error 1

[1] https://searchfox.org/mozilla-central/rev/d0a0ae30d534dc55e712bd3eedc1b4553ba56323/js/src/frontend/TokenStream.cpp#629

Comment 1

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Attachment: Bug 1480306: Fix unused variable variable 'hasLineOfContext' in TokenStream.cpp.

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

Comment 2

[Jeff Walden [:Waldo]]

Comment on attachment 8997504 [details]
Bug 1480306: Fix unused variable variable 'hasLineOfContext' in TokenStream.cpp.

https://reviewboard.mozilla.org/r/261244/#review268334

::: js/src/frontend/TokenStream.cpp:629
(Diff revision 1)
>  
>          ErrorMetadata err;
>  
>          TokenStreamAnyChars& anyChars = anyCharsAccess();
>  
> -        if (bool hasLineOfContext = anyChars.fillExcludingContext(&err, offset)) {
> +        if (anyChars.fillExcludingContext(&err, offset)) {

Bah, I was afraid of this, but it wasn't breaking locally or on try, so...

The reason I did the existing thing is that the function name doesn't imply a clear semantics to the test.  And as things are now, the calls to that function are a bit confusing to read for that reason.  I didn't want to add more.

Could you instead move the decl/assign outside the if, then test the variable?  Not as concise, but it keeps the meaning-ascribing bits I want here.

Comment 3

[Jeff Walden [:Waldo]]

Oh, FYI the syntax is legal and does indeed use the variable.  IMO the warning is a compiler bug, but we go to war with the compilers we have...

Comment 4

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Comment on attachment 8997504 [details]
Bug 1480306: Fix unused variable variable 'hasLineOfContext' in TokenStream.cpp.

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

Comment 5

[Jeff Walden [:Waldo]]

Comment on attachment 8997504 [details]
Bug 1480306: Fix unused variable variable 'hasLineOfContext' in TokenStream.cpp.

https://reviewboard.mozilla.org/r/261244/#review268674

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8f69423e522d
Fix unused variable variable 'hasLineOfContext' in TokenStream.cpp. r=Waldo

Comment 7

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/8f69423e522d