Skip to content

Commit

Permalink
apacheGH-35134: [C++] Add arrow_vendored namespace around double-co…
Browse files Browse the repository at this point in the history
…nversion library (apache#35135)

### Rationale for this change

See apache#35134

This change avoids potential symbol collisions when linking Arrow within other libraries.

This actually seems like the original intention (a private namespace) per the following merge: 767c953
The message in that merge is confusing, because it implies that the merge made the library private, when it actually did the opposite (specifically in 3b89b19). A quote is below:

>  Also, make its use private, because of Windows DLL exports. 

### What changes are included in this PR?

* an `update.sh` script to 
  * automatically pull a new version, 
  * apply the namespace change, 
  * apply a custom patch for seemingly Gandiva (15137e2)
    * this patch will break in future versions of double-conversion, but it is at least documented in the update script
* the update script is run for the current version 

The script is based on: https://github.com/apache/arrow/blob/main/cpp/src/arrow/vendored/fast_float/update.sh

### Are these changes tested?

Yes. Built and tests run

### Are there any user-facing changes?

Only if users used to depend on using the internal vendored double-conversion libraries for their own code. In such a case, when they upgrade, they would have to specify the `arrow_vendored` namespace. 
* Closes: apache#35134

Authored-by: Mike Lui <mikelui@meta.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
  • Loading branch information
mikelui authored and ArgusLi committed May 15, 2023
1 parent 191a734 commit b086d50
Show file tree
Hide file tree
Showing 24 changed files with 151 additions and 5 deletions.
6 changes: 3 additions & 3 deletions cpp/src/arrow/util/double_conversion.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ namespace arrow {
namespace util {
namespace double_conversion {

using ::double_conversion::DoubleToStringConverter;
using ::double_conversion::StringBuilder;
using ::double_conversion::StringToDoubleConverter;
using ::arrow_vendored::double_conversion::DoubleToStringConverter;
using ::arrow_vendored::double_conversion::StringBuilder;
using ::arrow_vendored::double_conversion::StringToDoubleConverter;

} // namespace double_conversion
} // namespace util
Expand Down
17 changes: 16 additions & 1 deletion cpp/src/arrow/vendored/double-conversion/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,19 @@
under the License.
-->

The files in this directory are vendored from double-conversion git tag v3.1.5.
The files in this directory are vendored from double-conversion git tag v3.1.5

To update, run:

```
./update.sh VERSION
```

For example:

```
./update.sh 3.1.5
```

If there are errors patching changes, you may have to manually intervene and resolve patch errors.
If so, please update the patches so the modifications are explicit.
2 changes: 2 additions & 0 deletions cpp/src/arrow/vendored/double-conversion/bignum-dtoa.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
#include "bignum.h"
#include "ieee.h"

namespace arrow_vendored {
namespace double_conversion {

static int NormalizedExponent(uint64_t significand, int exponent) {
Expand Down Expand Up @@ -639,3 +640,4 @@ static void FixupMultiply10(int estimated_power, bool is_even,
}

} // namespace double_conversion
} // namespace arrow_vendored
2 changes: 2 additions & 0 deletions cpp/src/arrow/vendored/double-conversion/bignum-dtoa.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@

#include "utils.h"

namespace arrow_vendored {
namespace double_conversion {

enum BignumDtoaMode {
Expand Down Expand Up @@ -80,5 +81,6 @@ void BignumDtoa(double v, BignumDtoaMode mode, int requested_digits,
Vector<char> buffer, int* length, int* point);

} // namespace double_conversion
} // namespace arrow_vendored

#endif // DOUBLE_CONVERSION_BIGNUM_DTOA_H_
2 changes: 2 additions & 0 deletions cpp/src/arrow/vendored/double-conversion/bignum.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include "bignum.h"
#include "utils.h"

namespace arrow_vendored {
namespace double_conversion {

Bignum::Bignum()
Expand Down Expand Up @@ -765,3 +766,4 @@ void Bignum::SubtractTimes(const Bignum& other, int factor) {


} // namespace double_conversion
} // namespace arrow_vendored
2 changes: 2 additions & 0 deletions cpp/src/arrow/vendored/double-conversion/bignum.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@

#include "utils.h"

namespace arrow_vendored {
namespace double_conversion {

class Bignum {
Expand Down Expand Up @@ -140,5 +141,6 @@ class Bignum {
};

} // namespace double_conversion
} // namespace arrow_vendored

#endif // DOUBLE_CONVERSION_BIGNUM_H_
2 changes: 2 additions & 0 deletions cpp/src/arrow/vendored/double-conversion/cached-powers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@

#include "cached-powers.h"

namespace arrow_vendored {
namespace double_conversion {

struct CachedPower {
Expand Down Expand Up @@ -173,3 +174,4 @@ void PowersOfTenCache::GetCachedPowerForDecimalExponent(int requested_exponent,
}

} // namespace double_conversion
} // namespace arrow_vendored
2 changes: 2 additions & 0 deletions cpp/src/arrow/vendored/double-conversion/cached-powers.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@

#include "diy-fp.h"

namespace arrow_vendored {
namespace double_conversion {

class PowersOfTenCache {
Expand Down Expand Up @@ -60,5 +61,6 @@ class PowersOfTenCache {
};

} // namespace double_conversion
} // namespace arrow_vendored

#endif // DOUBLE_CONVERSION_CACHED_POWERS_H_
2 changes: 2 additions & 0 deletions cpp/src/arrow/vendored/double-conversion/diy-fp.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include "diy-fp.h"
#include "utils.h"

namespace arrow_vendored {
namespace double_conversion {

void DiyFp::Multiply(const DiyFp& other) {
Expand All @@ -55,3 +56,4 @@ void DiyFp::Multiply(const DiyFp& other) {
}

} // namespace double_conversion
} // namespace arrow_vendored
2 changes: 2 additions & 0 deletions cpp/src/arrow/vendored/double-conversion/diy-fp.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@

#include "utils.h"

namespace arrow_vendored {
namespace double_conversion {

// This "Do It Yourself Floating Point" class implements a floating-point number
Expand Down Expand Up @@ -114,5 +115,6 @@ class DiyFp {
};

} // namespace double_conversion
} // namespace arrow_vendored

#endif // DOUBLE_CONVERSION_DIY_FP_H_
2 changes: 2 additions & 0 deletions cpp/src/arrow/vendored/double-conversion/double-conversion.cc
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
#pragma warning(disable : 4244)
#endif

namespace arrow_vendored {
namespace double_conversion {

const DoubleToStringConverter& DoubleToStringConverter::EcmaScriptConverter() {
Expand Down Expand Up @@ -1169,3 +1170,4 @@ float StringToDoubleConverter::StringToFloat(
}

} // namespace double_conversion
} // namespace arrow_vendored
2 changes: 2 additions & 0 deletions cpp/src/arrow/vendored/double-conversion/double-conversion.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@

#include "utils.h"

namespace arrow_vendored {
namespace double_conversion {

class DoubleToStringConverter {
Expand Down Expand Up @@ -583,5 +584,6 @@ class StringToDoubleConverter {
};

} // namespace double_conversion
} // namespace arrow_vendored

#endif // DOUBLE_CONVERSION_DOUBLE_CONVERSION_H_
2 changes: 2 additions & 0 deletions cpp/src/arrow/vendored/double-conversion/fast-dtoa.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#include "diy-fp.h"
#include "ieee.h"

namespace arrow_vendored {
namespace double_conversion {

// The minimal and maximal target exponent define the range of w's binary
Expand Down Expand Up @@ -663,3 +664,4 @@ bool FastDtoa(double v,
}

} // namespace double_conversion
} // namespace arrow_vendored
2 changes: 2 additions & 0 deletions cpp/src/arrow/vendored/double-conversion/fast-dtoa.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@

#include "utils.h"

namespace arrow_vendored {
namespace double_conversion {

enum FastDtoaMode {
Expand Down Expand Up @@ -84,5 +85,6 @@ bool FastDtoa(double d,
int* decimal_point);

} // namespace double_conversion
} // namespace arrow_vendored

#endif // DOUBLE_CONVERSION_FAST_DTOA_H_
2 changes: 2 additions & 0 deletions cpp/src/arrow/vendored/double-conversion/fixed-dtoa.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include "fixed-dtoa.h"
#include "ieee.h"

namespace arrow_vendored {
namespace double_conversion {

// Represents a 128bit type. This class should be replaced by a native type on
Expand Down Expand Up @@ -403,3 +404,4 @@ bool FastFixedDtoa(double v,
}

} // namespace double_conversion
} // namespace arrow_vendored
2 changes: 2 additions & 0 deletions cpp/src/arrow/vendored/double-conversion/fixed-dtoa.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@

#include "utils.h"

namespace arrow_vendored {
namespace double_conversion {

// Produces digits necessary to print a given number with
Expand All @@ -52,5 +53,6 @@ bool FastFixedDtoa(double v, int fractional_count,
Vector<char> buffer, int* length, int* decimal_point);

} // namespace double_conversion
} // namespace arrow_vendored

#endif // DOUBLE_CONVERSION_FIXED_DTOA_H_
2 changes: 2 additions & 0 deletions cpp/src/arrow/vendored/double-conversion/ieee.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@

#include "diy-fp.h"

namespace arrow_vendored {
namespace double_conversion {

// We assume that doubles and uint64_t have the same endianness.
Expand Down Expand Up @@ -398,5 +399,6 @@ class Single {
};

} // namespace double_conversion
} // namespace arrow_vendored

#endif // DOUBLE_CONVERSION_DOUBLE_H_
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
40a41,44
> #if defined(_MSC_VER)
> #pragma warning(disable : 4244)
> #endif
>
84c88,106
< if (length != 1) {
---
>
> /* If the mantissa of the scientific notation representation is an integer number,
> * the EMIT_TRAILING_DECIMAL_POINT flag will add a '.' character at the end of the
> * representation:
> * - With EMIT_TRAILING_DECIMAL_POINT enabled -> 0.0009 => 9.E-4
> * - With EMIT_TRAILING_DECIMAL_POINT disabled -> 0.0009 => 9E-4
> *
> * If the mantissa is an integer and the EMIT_TRAILING_ZERO_AFTER_POINT flag is enabled
> * it will add a '0' character at the end of the mantissa representation. Note that that
> * flag depends on EMIT_TRAILING_DECIMAL_POINT flag be enabled.*/
> if(length == 1){
> if ((flags_ & EMIT_TRAILING_DECIMAL_POINT) != 0) {
> result_builder->AddCharacter('.');
>
> if ((flags_ & EMIT_TRAILING_ZERO_AFTER_POINT) != 0) {
> result_builder->AddCharacter('0');
> }
> }
> } else {
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
107a108,118
> //
> // When converting numbers to scientific notation representation, if the mantissa of
> // the representation is an integer number, the EMIT_TRAILING_DECIMAL_POINT flag will
> // add a '.' character at the end of the representation:
> // - With EMIT_TRAILING_DECIMAL_POINT enabled -> 0.0009 => 9.E-4
> // - With EMIT_TRAILING_DECIMAL_POINT disabled -> 0.0009 => 9E-4
> //
> // If the mantissa is an integer and the EMIT_TRAILING_ZERO_AFTER_POINT flag is enabled
> // it will add a '0' character at the end of the mantissa representation. Note that that
> // flag depends on EMIT_TRAILING_DECIMAL_POINT flag be enabled.
> // - With EMIT_TRAILING_ZERO_AFTER_POINT enabled -> 0.0009 => 9.0E-4
2 changes: 2 additions & 0 deletions cpp/src/arrow/vendored/double-conversion/strtod.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#include "ieee.h"
#include "strtod.h"

namespace arrow_vendored {
namespace double_conversion {

// 2^53 = 9007199254740992.
Expand Down Expand Up @@ -578,3 +579,4 @@ float Strtof(Vector<const char> buffer, int exponent) {
}

} // namespace double_conversion
} // namespace arrow_vendored
2 changes: 2 additions & 0 deletions cpp/src/arrow/vendored/double-conversion/strtod.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@

#include "utils.h"

namespace arrow_vendored {
namespace double_conversion {

// The buffer must only contain digits in the range [0-9]. It must not
Expand All @@ -41,5 +42,6 @@ double Strtod(Vector<const char> buffer, int exponent);
float Strtof(Vector<const char> buffer, int exponent);

} // namespace double_conversion
} // namespace arrow_vendored

#endif // DOUBLE_CONVERSION_STRTOD_H_
56 changes: 56 additions & 0 deletions cpp/src/arrow/vendored/double-conversion/update.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
#!/usr/bin/env bash
#
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.

set -eu

source_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"

if [ "$#" -ne 1 ]; then
echo "Usage: $0 VERSION"
echo " e.g.: $0 3.2.1"
exit 1
fi

version="$1"

pushd "${source_dir}"
rm -rf double-conversion
git clone \
--branch "v${version}" \
--depth 1 \
https://github.com//google/double-conversion.git
rm *.cc *.h
for f in double-conversion/double-conversion/*{.h,.cc}; do
mv "${f}" ./
done
rm -rf double-conversion
sed -i.bak -E -e "s/v[0-9.]+/v${version}/g" *.md
sed -i.bak -E \
-e '/^namespace double_conversion \{/ i\
namespace arrow_vendored {' \
-e '/^} \/\/ namespace double_conversion/ a\
} // namespace arrow_vendored' \
*.{h,cc}
rm *.bak

# Custom changes for Arrow
patch double-conversion.cc patches/double-conversion.cc.patch
patch double-conversion.h patches/double-conversion.h.patch

popd
2 changes: 2 additions & 0 deletions cpp/src/arrow/vendored/double-conversion/utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ typedef uint16_t uc16;
DC_DISALLOW_COPY_AND_ASSIGN(TypeName)
#endif

namespace arrow_vendored {
namespace double_conversion {

static const int kCharSize = sizeof(char);
Expand Down Expand Up @@ -363,5 +364,6 @@ inline Dest BitCast(Source* source) {
}

} // namespace double_conversion
} // namespace arrow_vendored

#endif // DOUBLE_CONVERSION_UTILS_H_
2 changes: 1 addition & 1 deletion cpp/src/gandiva/formatting_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ namespace gandiva {
template <typename ARROW_TYPE, typename Enable = void>
class GdvStringFormatter;

using double_conversion::DoubleToStringConverter;
using arrow_vendored::double_conversion::DoubleToStringConverter;

template <typename ARROW_TYPE>
class FloatToStringGdvMixin
Expand Down

0 comments on commit b086d50

Please sign in to comment.