701522 - nsAString.h's kNotFound conflicts with LevelDB

Status: NEW

(Core :: XPCOM, defect)

Description

[Matthew McPherrin]

I am working on integrating LevelDB (bug 679852).

The "#define kNotFound -1" conflicts with an element of an enum in LevelDB.

If nsAString.h is #included before any LevelDB headers, at best we get a compile error (Because enum Code { ..., -1 = 1, ...}; is invalid syntax).

At worst, somebody in the leveldb:: namespace who uses kNotFound compares against -1 instead of 1, leading to incorrect behaviour.

Correct use of #undef kNotFound before #including LevelDB headers avoids this problem, however it may lead to future problems.

One possibility is changing nsAString.h. That's what this bug is for.

Comment 1

[(not currently active) Ted Mielczarek]

Looks like we could probably change that to const PRInt32 kNotFound = -1;

Comment 2

[Boris Zbarsky [:bzbarsky]]

Or even to a static member on nsAString, and fix our consumers.

Comment 3

[Matthew McPherrin]

Attachment: Global constant kNotFound

Seems to build on Linux, at least. Pushing to try to find out what happens.

Comment 4

[Mozilla RelEng Bot]

Try run for 7b687eed8c27 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=7b687eed8c27
Results (out of 209 total builds):
    success: 190
    warnings: 18
    failure: 1
Builds available at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-7b687eed8c27

Comment 5

[Benjamin Smedberg]

I've been working on a patch to make nsAString::kNotFound instead of a global, which I think is preferable but requires some treewide changes.

Comment 6

[Benjamin Smedberg]

I'm not going to have the time to finish what I started here. If there's a simpler patch and this is still an issue we can probably take that.