1068102 - unused variables introduced by bug 994190: ActorsChild.h:224:19: warning: private field 'mPersistenceType' is not used [-Wunused-private-field], and two unused const variables in ActorsParent.cpp

Status: RESOLVED WORKSFORME

(Core :: Storage: IndexedDB, defect)

Description

[Daniel Holbert [:dholbert]]

New build warning, with clang 3.5 on Linux:
{
dom/indexedDB/ActorsChild.cpp:5:
dom/indexedDB/ActorsChild.h:224:19: warning: private field 'mPersistenceType' is not used [-Wunused-private-field]
}

Indeed, this private member-var is set but unused, as a quick skim through http://mxr.mozilla.org/mozilla-central/source/dom/indexedDB/ActorsChild.cpp shows.

bent, can this variable be removed?

Comment 1

[Daniel Holbert [:dholbert]]

This warning was introduced by bug 994190. While we're at it, that bug also introduced these unused-const-variable warnings:
{
dom/indexedDB/ActorsParent.cpp:161:12: warning: unused variable 'kPermissionString' [-Wunused-const-variable]

 const char kPermissionString[] = IDB_PREFIX;
            ^

dom/indexedDB/ActorsParent.cpp:163:12: warning: unused variable 'kChromeOrigin' [-Wunused-const-variable]

 const char kChromeOrigin[] = CHROME_PREFIX;
            ^
}

Presumably those also want to be removed.

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: part 1: remove unused const variables

The const variables are easy enough to remove, at least.

(Handling mPersistenceType separately, as I'm not sure if there's a plan to use it.)

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: part 1: remove unused const variables

Comment 4

[Daniel Holbert [:dholbert]]

Attachment: part 2: Remove unused member vars

Here's a patch to remove the unused mPersistenceType vars, but per my XXX comment in the patch, this leaves the constructor with an unused argument. (which doesn't trigger a warning, but is still messy & worth removing)

I'm not sure what's supposed to be happening with these mPersistenceType variables; it looks like it's passed down from somewhere higher up (most directly, from the params passed to IDBFactory::InitiateRequest); should we remove them up the stack as much as possible, or should we really be reacting to them here?

Comment 5

[Jan Varga [:janv]]

Yeah, I removed all those variables in my local patches.
Everything works fine.

Comment 6

[Daniel Holbert [:dholbert]]

Right, removing the variables should have no effect on behavior -- that's what the compiler is telling us with these warnings.

The question, though, is: are we *supposed* to be checking these variables? (or, are we intending to start using them in some subsequent landing-soon patch?)

(If we do want to use them, then perhaps "Part 2" should be replaced with an unused<< hack, with a reference to a followup bug where we intend to add the usages.)

Comment 7

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Bug 994190 was backed out so this is WFM.

I'll fix this on jamun before bug 994190 is fixed.