1492570 - Realm.h shadow variable warnings

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P3)

Description

[jbonnafo]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:62.0) Gecko/20100101 Firefox/62.0
Build ID: 20180830143136

Steps to reproduce:

Compiling with clang with -Wshadow, there are several compiler warnings about Realm.h file:

8:23.69 c:/mozilla-source/mozilla-central/js/src\vm/Realm.h(66,20):  warning: declaration shadows a field of 'js::DtoaCache' [-Wshadow]
 8:23.71     void cache(int base, double d, JSFlatString* s) {
 8:23.73                    ^
 8:23.75 c:/mozilla-source/mozilla-central/js/src\vm/Realm.h(55,18):  note: previous declaration is here
 8:23.75     int          base;
 8:23.75                  ^
 8:23.76 c:/mozilla-source/mozilla-central/js/src\vm/Realm.h(66,33):  warning: declaration shadows a field of 'js::DtoaCache' [-Wshadow]
 8:23.76     void cache(int base, double d, JSFlatString* s) {
 8:23.76                                 ^
 8:23.76 c:/mozilla-source/mozilla-central/js/src\vm/Realm.h(54,18):  note: previous declaration is here
 8:23.76     double       d;
 8:23.76                  ^
 8:23.76 c:/mozilla-source/mozilla-central/js/src\vm/Realm.h(66,50):  warning: declaration shadows a field of 'js::DtoaCache' [-Wshadow]
 8:23.76     void cache(int base, double d, JSFlatString* s) {
 8:23.76                                                  ^
 8:23.76 c:/mozilla-source/mozilla-central/js/src\vm/Realm.h(56,19):  note: previous declaration is here
 8:23.76     JSFlatString* s;      // if s==nullptr, d and base are not valid


Actual results:

Code looks fine to me.

Renaming args of the cache function to have them using a name different from the class members would avoid the compiler warnings.


Expected results:

Code should have compiled without warning

Comment 1

[Kannan Vijayan [:djvj]]

Nice find.

Comment 2

[Matthew Gaudet (he/him) [:mgaudet]]

(Still appears to be the case)

Comment 3

[sejun.bae]

Could I work on this ?

Comment 4

[mike.maksymowych]

Attachment: Bug 1492570 - Resolve shadow variable warnings. r?jonco

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2afbb35af4d7
Resolve shadow variable warnings. r=jonco

Comment 6

[Sandor Molnar[:smolnar]]

Backed out for causing build bustages in js/src/vm/Realm.cpp

Backout link: https://hg.mozilla.org/integration/autoland/rev/c11769369d46bd7037efd0430f9b549466f2298e

Push with failures

Failure log

ERROR -  /builds/worker/checkouts/gecko/js/src/vm/Realm.cpp:167:15: error: use of undeclared identifier 's'

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:mike.maksymowych, could you have a look please?
If you still have some work to do, you can add an action "Plan Changes" in Phabricator.
For more information, please visit auto_nag documentation.

Comment 8

[Jon Coppeard (:jonco)]

Tests look OK so I'll land this.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1386e60c9260
Resolve shadow variable warnings. r=jonco

Comment 10

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/1386e60c9260