dropbox / json11

A tiny JSON library for C++11.
MIT License
2.55k stars 613 forks source link

Json11 fails to build with clang 4.0.0 #91

Closed cpu82 closed 7 years ago

cpu82 commented 7 years ago

json11 fails to build with clang 4.0 likely due to llvm-mirror/clang@4b6ad14

% /usr/bin/clang -v
FreeBSD clang version 4.0.0 (branches/release_40 292009) (based on LLVM 4.0.0)
Target: x86_64-unknown-freebsd12.0
Thread model: posix
InstalledDir: /usr/bin
ext/json11/json11.cpp:153:24: error: invalid operands to binary expression
      ('nullptr_t' and 'nullptr_t')
        return m_value < static_cast<const Value<tag, T> *>(other)->m_value;
               ~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
ext/json11/json11.cpp:209:5: note: in instantiation of member function
      'json11::Value<json11::Json::Type::NUL, nullptr_t>::less' requested here
    JsonNull() : Value(nullptr) {}
    ^

Also reported here: https://github.com/PowerDNS/pdns/issues/4913

artwyman commented 7 years ago

You're right that this seems to have been an intentional compiler change. I think the relevant commit is actually llvm-mirror/libcxx@e577a4c6 which removes operator< from nullptr_t. Seems like this was changed intentionally in the standard, at the same time as the broader comparison change you referenced. My digging found DR 583 which was resolved along with issue 1512 which was resolved by this wording in the standard. At the end, nullptr_t is being specifically redefined to have fewer built-in operators.

That's a bit inconvenient since it breaks the nice symmetry of the use of nullptr_t in JsonValue, but it doesn't affect anything public so it shouldn't be impossible to hack around. I haven't thought about it in detail, but you could likely do it by defining NullValue without using the Value template. Or you might make Value work by replacing nullptr_t with a helper struct which is comparable and castable to nullptr_t. Or there might be a more clever option. @j4cbo any thoughts?

@cpu82 do you want to take a stab at this an send a PR, or are you looking for someone else to do it?

j4cbo commented 7 years ago

I like the helper struct idea. It doesn't need to be castable to nullptr_t, just comparable with itself.

cpu82 commented 7 years ago

It builds fine after replace nullptr_t with the new empty struct.

@j4cbo thanks for the patch.