964074 - clang build warning: docshell/base/nsDocShell.cpp:192:22: warning: unused variable 'kAppShellCID' [Wunused-const-variable]

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect)

Description

[Daniel Holbert [:dholbert]]

Clang 3.4 and 3.5 (at least) have a warning for "-Wunused-const-variable", and we're hitting some instances of it that trigger errors in --enable-warnings-as-errors builds.

One instance:
{
docshell/base/nsDocShell.cpp:192:22: error: unused variable 'kAppShellCID' [-Werror,-Wunused-const-variable]
static NS_DEFINE_CID(kAppShellCID, NS_APPSHELL_CID);
                     ^
../../dist/include/nsID.h:86:15: note: expanded from macro 'NS_DEFINE_CID'
  const nsCID _name = _cidspec
              ^
}

Just an unused static variable that can be dropped, basically.

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: fix v1

Comment 2

[Daniel Holbert [:dholbert]]

Comment on attachment 8365697 [details] [diff] [review]
fix v1

(sorry; my browser initially autocompleted the patch name from "fix v1" to a longer string that I'd used for an fix somewhere else)

Comment 3

[Daniel Holbert [:dholbert]]

MXR search demonstrating that kAppShellCID is never used in this file:
http://mxr.mozilla.org/mozilla-central/search?string=kAppShellCID&find=nsDocShell.cpp

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

But please don't land this yet. I may need to backout the patch which caused this.

Comment 5

[Daniel Holbert [:dholbert]]

OK. Setting needinfo=you, to update this bug when you're sure whether that patch will/won't be backed out.

(Thanks for the review!)

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

It was backed out long ago, and haven't had time to fix b2g reftest framework in order to land it again.

Comment 7

[Daniel Holbert [:dholbert]]

OK. I don't think there's any value in leaving this open and trying to remember that this should land after that bug (eventually) re-lands.

Would you mind just removing the unnecessary NS_DEFINE_CID line from your local version of the patch, so that this bug doesn't get re-introduced when bug 930793 lands?  Assuming that's all right with you, I'm resolving as FIXED by backout in bug 930793 comment 44. (and I'm assuming this will still be fixed when bug 930793 re-lands.)