884022 - Clang build fails in test_AsXXX_helpers.cpp (comparison between pointer and integer)

Status: RESOLVED FIXED

(Core :: General, defect)

Description

[Jesse Ruderman]

With a fairly recent clang (llvm svn 184056), the build fails in test_AsXXX_helpers.cpp.

Daniel Holbert and Nathan Froyd looked a little bit on IRC.  They suspect the fix for bug 787730 was incorrect.

/Users/jruderman/trees/mozilla-central/storage/test/test_AsXXX_helpers.cpp:39:3: error: comparison between pointer and integer
      ('const char *' and 'int')
  do_check_eq(row->AsSharedUTF8String(0, &len), '\0');
  ^           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~  ~~~~

/Users/jruderman/trees/mozilla-central/storage/test/storage_test_harness.h:66:19: note: expanded from macro 'do_check_eq'
    if (aExpected == aActual) { \
                  ^

Comment 1

[(no longer active)]

Attachment: Patch (v1)

Comment 2

[Jesse Ruderman]

Comment on attachment 763779 [details] [diff] [review]
Patch (v1)

Nope, this reintroduces the problem bug 787730 was trying to fix.

/Users/jruderman/trees/mozilla-central/storage/test/test_AsXXX_helpers.cpp:39:3: error: use of overloaded operator '<<' is ambiguous (with operand types 'std::ostringstream'
      (aka 'basic_ostringstream<char>') and 'nullptr_t')
  do_check_eq(row->AsSharedUTF8String(0, &len), nullptr);
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/jruderman/trees/mozilla-central/storage/test/storage_test_harness.h:71:12: note: expanded from macro 'do_check_eq'
      temp << aActual <<"' at line " << __LINE__; \
      ~~~~ ^
/usr/include/c++/4.2.1/ostream:112:7: note: candidate function
      operator<<(__ostream_type& (*__pf)(__ostream_type&))
      ^
/usr/include/c++/4.2.1/ostream:121:7: note: candidate function
      operator<<(__ios_type& (*__pf)(__ios_type&))
      ^

Comment 3

[Jesse Ruderman]

Maybe it would be better to use do_check_true, or introduce a do_check_null.

http://mxr.mozilla.org/mozilla-central/source/storage/test/storage_test_harness.h#26

Comment 4

[Nathan Froyd [:froydnj]]

Attachment: fix clang warnings about pointer-to-int comparisons with test_AsXXX_helpers.cpp

We can cast nullptr to the correct type or we can use do_check_false.  I went with
the first solution.

Comment 5

[(no longer active)]

Ah, you're using an old libstdc++... :(

Comment 6

[Daniel Holbert [:dholbert]]

Comment on attachment 763794 [details] [diff] [review]
fix clang warnings about pointer-to-int comparisons with test_AsXXX_helpers.cpp

I'd prefer static_cast<>, since this is C++, but it doesn't matter too much.

Comment 7

[Jesse Ruderman]

For int, I guess this is ok.  But for char, (const char *) is another way of saying "C string".  What happens if you try to send a nullptr C string to an iostream?

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/f53918bafe2a

Comment 9

[(no longer active)]

(In reply to comment #7)
> For int, I guess this is ok.  But for char, (const char *) is another way of
> saying "C string".  What happens if you try to send a nullptr C string to an
> iostream?

You get no output?

Comment 10

[Daniel Holbert [:dholbert]]

From some quick googling, it looks like the behavior of cout << (const char*)NULL might be undefined, which does seem problematic.

That's how the test was written, though, AFAICT; this bug didn't impact that at all. If you're worried about that, take it up with the test author. :)

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/5dbe9ea88a5a
https://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/5dbe9ea88a5a