[SERVER-39327] BSONObjBuilder::append() should generate the same compile errors with all toolchains Created: 01/Feb/19  Updated: 27/Oct/23  Resolved: 08/Feb/19

Status: Closed
Project: Core Server
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Improvement Priority: Major - P3
Reporter: Louis Williams Assignee: Andrew Morrow (Inactive)
Resolution: Works as Designed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Related
Sprint: Dev Tools 2019-02-11
Participants:
Linked BF Score: 0

 Description   

If you pass a long type to BSONObjBuilder::append() on macOS, the following compile error is generated because the argument is ambiguous.

This causes unnecessary compile failures on non-required builders like macOS for no reason. My suggestion is one of the following:
a. The toolchain on required builders should also generate this same error
b. Modify BSONObjBuilder::append() so that passing a long type is not ambiguous on macOS.

 [2019/02/01 01:08:20.279] src/mongo/db/storage/bson_collection_catalog_entry.cpp:308:17: error: call to member function 'append' is ambiguous
 [2019/02/01 01:08:20.279]             sub.append("versionOfBuild", indexes[i].versionOfBuild);
 [2019/02/01 01:08:20.279]             ~~~~^~~~~~
 [2019/02/01 01:08:20.279] src/mongo/bson/bsonobjbuilder.h:281:21: note: candidate function
 [2019/02/01 01:08:20.279]     BSONObjBuilder& append(StringData fieldName, bool val) {
 [2019/02/01 01:08:20.279]                     ^
 [2019/02/01 01:08:20.279] src/mongo/bson/bsonobjbuilder.h:289:21: note: candidate function
 [2019/02/01 01:08:20.279]     BSONObjBuilder& append(StringData fieldName, int n) {
 [2019/02/01 01:08:20.279]                     ^
 [2019/02/01 01:08:20.280] src/mongo/bson/bsonobjbuilder.h:297:21: note: candidate function
 [2019/02/01 01:08:20.280]     BSONObjBuilder& append(StringData fieldName, unsigned n) {
 [2019/02/01 01:08:20.280]                     ^
 [2019/02/01 01:08:20.280] src/mongo/bson/bsonobjbuilder.h:311:21: note: candidate function
 [2019/02/01 01:08:20.280]     BSONObjBuilder& append(StringData fieldName, long long n) {
 [2019/02/01 01:08:20.280]                     ^
 [2019/02/01 01:08:20.281] src/mongo/bson/bsonobjbuilder.h:387:21: note: candidate function
 [2019/02/01 01:08:20.281]     BSONObjBuilder& append(StringData fieldName, double n) {
 [2019/02/01 01:08:20.281]                     ^
 [2019/02/01 01:08:20.281] src/mongo/bson/bsonobjbuilder.h:212:21: note: candidate function not viable: no known conversion from 'const long' to 'mongo::BSONObj' for 2nd argument
 [2019/02/01 01:08:20.281]     BSONObjBuilder& append(StringData fieldName, BSONObj subObj) {
 [2019/02/01 01:08:20.281]                     ^
 [2019/02/01 01:08:20.281] src/mongo/bson/bsonobjbuilder.h:260:21: note: candidate function not viable: no known conversion from 'const long' to 'mongo::BSONArray' for 2nd argument
 [2019/02/01 01:08:20.281]     BSONObjBuilder& append(StringData fieldName, BSONArray arr) {
 [2019/02/01 01:08:20.281]                     ^
 [2019/02/01 01:08:20.281] src/mongo/bson/bsonobjbuilder.h:302:21: note: candidate function not viable: no known conversion from 'const long' to 'mongo::Decimal128' for 2nd argument
 [2019/02/01 01:08:20.281]     BSONObjBuilder& append(StringData fieldName, Decimal128 n) {
 [2019/02/01 01:08:20.281]                     ^
 [2019/02/01 01:08:20.281] src/mongo/bson/bsonobjbuilder.h:419:21: note: candidate function not viable: no known conversion from 'const long' to 'mongo::OID' for 2nd argument
 [2019/02/01 01:08:20.281]     BSONObjBuilder& append(StringData fieldName, OID oid) {
 [2019/02/01 01:08:20.281]                     ^
 [2019/02/01 01:08:20.281] src/mongo/bson/bsonobjbuilder.h:449:21: note: candidate function not viable: no known conversion from 'const long' to 'mongo::Date_t' for 2nd argument
 [2019/02/01 01:08:20.281]     BSONObjBuilder& append(StringData fieldName, Date_t dt) {
 [2019/02/01 01:08:20.281]                     ^
 [2019/02/01 01:08:20.281] src/mongo/bson/bsonobjbuilder.h:465:21: note: candidate function not viable: no known conversion from 'const long' to 'const mongo::BSONRegEx' for 2nd argument
 [2019/02/01 01:08:20.281]     BSONObjBuilder& append(StringData fieldName, const BSONRegEx& regex) {
 [2019/02/01 01:08:20.281]                     ^
 [2019/02/01 01:08:20.281] src/mongo/bson/bsonobjbuilder.h:477:21: note: candidate function not viable: no known conversion from 'const long' to 'const mongo::BSONCode' for 2nd argument
 [2019/02/01 01:08:20.281]     BSONObjBuilder& append(StringData fieldName, const BSONCode& code) {
 [2019/02/01 01:08:20.281]                     ^
 [2019/02/01 01:08:20.282] src/mongo/bson/bsonobjbuilder.h:491:21: note: candidate function not viable: no known conversion from 'const long' to 'const char *' for 2nd argument
 [2019/02/01 01:08:20.282]     BSONObjBuilder& append(StringData fieldName, const char* str) {
 [2019/02/01 01:08:20.282]                     ^
 [2019/02/01 01:08:20.282] src/mongo/bson/bsonobjbuilder.h:495:21: note: candidate function not viable: no known conversion from 'const long' to 'mongo::StringData' for 2nd argument
 [2019/02/01 01:08:20.282]     BSONObjBuilder& append(StringData fieldName, StringData str) {
 [2019/02/01 01:08:20.282]                     ^
 [2019/02/01 01:08:20.282] src/mongo/bson/bsonobjbuilder.h:511:21: note: candidate function not viable: no known conversion from 'const long' to 'const mongo::BSONSymbol' for 2nd argument
 [2019/02/01 01:08:20.282]     BSONObjBuilder& append(StringData fieldName, const BSONSymbol& symbol) {
 [2019/02/01 01:08:20.282]                     ^
 [2019/02/01 01:08:20.282] src/mongo/bson/bsonobjbuilder.h:559:21: note: candidate function not viable: no known conversion from 'const long' to 'const mongo::BSONDBRef' for 2nd argument
 [2019/02/01 01:08:20.282]     BSONObjBuilder& append(StringData fieldName, const BSONDBRef& dbref) {
 [2019/02/01 01:08:20.282]                     ^
 [2019/02/01 01:08:20.282] src/mongo/bson/bsonobjbuilder.h:582:21: note: candidate function not viable: no known conversion from 'const long' to 'const mongo::BSONBinData' for 2nd argument
 [2019/02/01 01:08:20.282]     BSONObjBuilder& append(StringData fieldName, const BSONBinData& bd) {
 [2019/02/01 01:08:20.282]                     ^
 [2019/02/01 01:08:20.282] src/mongo/bson/bsonobjbuilder.h:615:21: note: candidate function not viable: no known conversion from 'const long' to 'const mongo::BSONCodeWScope' for 2nd argument
 [2019/02/01 01:08:20.282]     BSONObjBuilder& append(StringData fieldName, const BSONCodeWScope& cws) {
 [2019/02/01 01:08:20.282]                     ^
 [2019/02/01 01:08:20.282] src/mongo/bson/bsonobjbuilder.h:1060:40: note: candidate function not viable: no known conversion from 'const long' to 'mongo::Timestamp' for 2nd argument
 [2019/02/01 01:08:20.282] inline BSONObjBuilder& BSONObjBuilder::append(StringData fieldName, Timestamp optime) {
 [2019/02/01 01:08:20.282]                                        ^
 [2019/02/01 01:08:20.282] src/mongo/bson/bsonobjbuilder.h:324:21: note: candidate template ignored: requirement 'std::is_same<long, int64_t>::value' was not satisfied [with Int64_t = long]
 [2019/02/01 01:08:20.282]     BSONObjBuilder& append(StringData fieldName, Int64_t n) {
 [2019/02/01 01:08:20.282]                     ^
 [2019/02/01 01:08:20.282] src/mongo/bson/bsonobjbuilder.h:956:40: note: candidate template ignored: could not match 'vector<type-parameter-0-0, allocator<type-parameter-0-0> >' against 'const long'
 [2019/02/01 01:08:20.283] inline BSONObjBuilder& BSONObjBuilder::append(StringData fieldName, const std::vector<T>& vals) {
 [2019/02/01 01:08:20.283]                                        ^
 [2019/02/01 01:08:20.283] src/mongo/bson/bsonobjbuilder.h:974:40: note: candidate template ignored: could not match 'list<type-parameter-0-0, allocator<type-parameter-0-0> >' against 'const long'
 [2019/02/01 01:08:20.283] inline BSONObjBuilder& BSONObjBuilder::append(StringData fieldName, const std::list<T>& vals) {
 [2019/02/01 01:08:20.283]                                        ^
 [2019/02/01 01:08:20.283] src/mongo/bson/bsonobjbuilder.h:979:40: note: candidate template ignored: could not match 'set<type-parameter-0-0, less<type-parameter-0-0>, allocator<type-parameter-0-0> >' against 'const long'
 [2019/02/01 01:08:20.283] inline BSONObjBuilder& BSONObjBuilder::append(StringData fieldName, const std::set<T>& vals) {
 [2019/02/01 01:08:20.283]                                        ^
 [2019/02/01 01:08:20.283] src/mongo/bson/bsonobjbuilder.h:984:40: note: candidate template ignored: could not match 'map<type-parameter-0-0, type-parameter-0-1, less<type-parameter-0-0>, allocator<pair<const type-parameter-0-0, type-parameter-0-1> > >' against 'const long'
 [2019/02/01 01:08:20.283] inline BSONObjBuilder& BSONObjBuilder::append(StringData fieldName, const std::map<K, T>& vals) {
 [2019/02/01 01:08:20.283]                                        ^
 [2019/02/01 01:08:20.283] src/mongo/bson/bsonobjbuilder.h:194:21: note: candidate function not viable: requires single argument 'e', but 2 arguments were provided
 [2019/02/01 01:08:20.283]     BSONObjBuilder& append(const BSONElement& e) {
 [2019/02/01 01:08:20.283]                     ^
 [2019/02/01 01:08:20.283] src/mongo/bson/bsonobjbuilder.h:483:21: note: candidate function not viable: requires 3 arguments, but 2 were provided
 [2019/02/01 01:08:20.283]     BSONObjBuilder& append(StringData fieldName, const char* str, int sz) {
 [2019/02/01 01:08:20.283]                     ^
 [2019/02/01 01:08:20.699] 1 error generated.



 Comments   
Comment by Mathias Stearn [ 08/Feb/19 ]

Discussed in person with acm and we decided to leave it as is. The only real downside is that code using long} will compile on some platforms but not others. But since we don't want anyone using long, it is good that it fails to compile somewhere so we can fix that mistake. The 2 alternatives to the status quo are worse:

  • Remove the overload and stop supporting code using int64_t forcing it to use long long.
  • Remove the SFINAE and just have an over load that takes long. This would let you append to a 64-bit bson field, even on platforms where long is only 32-bits, so it isn't really an option.

I'm surprised this isn't failing on windows where long is only 32 bits. Did it compile correctly there?

Comment by Andrew Morrow (Inactive) [ 08/Feb/19 ]

redbeard0531 - I think maybe you are being too hasty. On Linux, for instance, that overload is selected for calls to BSONObjBuilder::append with an argument of type long. On macOS, it doesn't match because std::is_same<int64_t, long>::value is false, and without that overload in the set it results in a compiler error. louis.williams's not-unreasonable expectation is that the code should work the same way on both platforms: it should either fail to compile everywhere, or it should append the value as a signed 64-bit quantity everywhere. So I agree with him that there is a problem here. But the problem isn't in the toolchain.

For what it's worth, I also completely agree with you that we should strongly discourage use of long.

I suggest that we re-open this.

Comment by Mathias Stearn [ 08/Feb/19 ]

The point of that SFINAE isn't to make long work with BSON. The goal of that is to support both int64_t which is sometimes long and sometimes long long. long is by far the worst of the built in integer types because it is the only type that is likely to be different sizes on desktop platforms. We should never be explicitly using long, unless it is specifically for a third-party API, and we should scope its use as closely as possible.

If you need a 64-bit type, use either int64_t or long long, matching whatever you see in the code around you. Some of our apis will accept one but not the other, so matching local code is most likely to work. If in doubt and both work, use int64_t.

Comment by Andrew Morrow (Inactive) [ 08/Feb/19 ]

If anything, this is a bug in BSONObjBuilder, because on macOS, long isn't int64_t, it is ssize_t. Don't believe me?

#include <type_traits>
#include <cstdlib>
#include <iostream>
 
int main() {
   std::cout << sizeof(long) << "\n";
   std::cout << std::is_same<long, int32_t>::value << "\n";
   std::cout << std::is_same<long, int64_t>::value << "\n";
   std::cout << std::is_same<long, std::int64_t>::value << "\n";
   std::cout << std::is_same<long, ssize_t> ::value << "\n";
   std::cout << std::is_same<long, long long>::value << "\n";
   return 0;
}

As a result this SFINAE in BSONObjBuilder doesn't match: https://github.com/mongodb/mongo/blob/master/src/mongo/bson/bsonobjbuilder.h#L321-L328

CC redbeard0531

However, my more practical advice would be: don't use long.

Generated at Thu Feb 08 04:51:42 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.