Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1077)

Unified Diff: base/strings/string_number_conversions.h

Issue 1212653005: [ADANDONED] Helper code for finding problematic uses of IntToString variants. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@int_to_string_cleanup_net
Patch Set: Disable warnings on IntToString's own tests. Created 5 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | base/strings/string_number_conversions.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: base/strings/string_number_conversions.h
diff --git a/base/strings/string_number_conversions.h b/base/strings/string_number_conversions.h
index cf1c3b467ddb269bb5a6c4ec61f7325110d83c2d..ee4807cd85c9116535b94cbdacef44f7db99e168 100644
--- a/base/strings/string_number_conversions.h
+++ b/base/strings/string_number_conversions.h
@@ -8,6 +8,11 @@
#include <string>
#include <vector>
+#if defined(STRING_NUMBER_CONVERSIONS_DETECT_RISKY_CONVERSIONS)
+#include <limits>
+#include <type_traits>
+#endif
+
#include "base/base_export.h"
#include "base/basictypes.h"
#include "base/strings/string16.h"
@@ -29,6 +34,10 @@ namespace base {
// Number -> string conversions ------------------------------------------------
+#if defined(STRING_NUMBER_CONVERSIONS_DETECT_RISKY_CONVERSIONS)
+namespace internal {
+#endif
+
BASE_EXPORT std::string IntToString(int value);
BASE_EXPORT string16 IntToString16(int value);
@@ -48,6 +57,10 @@ BASE_EXPORT string16 SizeTToString16(size_t value);
// locale. If you want to use locale specific formatting, use ICU.
BASE_EXPORT std::string DoubleToString(double value);
+#if defined(STRING_NUMBER_CONVERSIONS_DETECT_RISKY_CONVERSIONS)
+} // namespace internal
+#endif
+
// String -> number conversions ------------------------------------------------
// Perform a best-effort conversion of the input string to a numeric type,
@@ -130,6 +143,128 @@ BASE_EXPORT bool HexStringToUInt64(const StringPiece& input, uint64* output);
BASE_EXPORT bool HexStringToBytes(const std::string& input,
std::vector<uint8>* output);
+// Detection of potentially risky conversions. This is not enabled by default
+// because it has too many false positives.
+//
+// How to use:
+// 1. Define STRING_NUMBER_CONVERSIONS_DETECT_RISKY_CONVERSIONS (probably the
+// easiest way is to make a local modification to this file, but make sure
+// you don't check it in).
+// 2. Compile the code you want to check.
+// 3. You will see errors like "error: temporary of type
+// 'SignedUnsignedMismatch' (aka 'base::ErrorOnUse') has private destructor"
+// 4. Check the actual type of the variable being passed in.
+// 5. Attempt to determine whether the "wrong" conversion function is being used
+// intentionally. Pay particular attention to 32-bit/64-bit issues and
+// cross-platform compatibility.
+// 6. Decide if it would be better to use a different conversion function.
+// 7. If so, make the change, compile it, and test it.
+//
+#if defined(STRING_NUMBER_CONVERSIONS_DETECT_RISKY_CONVERSIONS)
+
+// Enums are weird. They have an "underlying type", but that is
+// implementation-specific (unless explicitly specified). However, the overload
+// resolution order for enums *is* specified in the standard, specifically:
+//
+// """ A prvalue of an unscoped enumeration type whose underlying type is not
+// fixed (7.2) can be converted to a prvalue of the first of the following types
+// that can represent all the values of the enumeration (i.e., the values in the
+// range b min to b max as described in 7.2): int, unsigned int, long int,
+// unsigned long int, long long int, or unsigned long long int. """
+//
+// Since that is portable, decide the correct function to pass an enum to based
+// on overload resolution.
+template <typename T>
+class EnumOverloadOrOriginalType {
+ private:
+ template <typename T2, bool is_enum>
+ struct Impl;
+
+ template <typename T2>
+ struct Impl<T2, false> {
+ typedef T2 type;
+ };
+
+ template <typename T2>
+ struct Impl<T2, true> {
+ static int deduce(int);
+ static uint32 deduce(uint32);
+ static int64 deduce(int64);
+ static uint64 deduce(uint64);
+
+ typedef decltype(deduce(T2())) type;
+ };
+
+ public:
+ typedef typename Impl<T, std::is_enum<T>::value>::type type;
+};
+
+template <typename ReturnType,
+ typename ExpectedType,
+ ReturnType(Function)(ExpectedType)>
+struct LooseMatchFunctor {
+ constexpr LooseMatchFunctor() {}
+
+ template <typename T>
+ ReturnType operator()(T value) const {
+ static_assert(
+ std::is_arithmetic<T>::value || std::is_enum<T>::value,
+ "Type passed to numeric conversions must be arithmetic or enum");
+ // If T is an enum, then it is the underlying type we are interested in.
+ typedef typename EnumOverloadOrOriginalType<T>::type ActualType;
+ static_assert(std::is_arithmetic<ActualType>::value,
+ "ActualType must be arithmetic");
+ static_assert(sizeof(ActualType) <= sizeof(ExpectedType),
+ "Silent truncation of larger integer type");
+ static_assert(std::is_enum<ActualType>::value ||
+ std::numeric_limits<ActualType>::is_signed ==
+ std::numeric_limits<ExpectedType>::is_signed,
+ "Signed/unsigned mismatch");
+ static_assert(std::is_enum<ActualType>::value ||
+ std::numeric_limits<ActualType>::is_integer ==
+ std::numeric_limits<ExpectedType>::is_integer,
+ "Integer/floating point mismatch");
+ return Function(value);
+ }
+};
+
+template <typename ReturnType,
+ typename ExpectedType,
+ ReturnType(Function)(ExpectedType)>
+struct ExactMatchFunctor {
+ constexpr ExactMatchFunctor() {}
+
+ template <typename ActualType>
+ ReturnType operator()(ActualType value) const {
+ static_assert(std::is_same<ActualType, ExpectedType>::value,
+ "Types do not match");
+ return Function(value);
+ }
+};
+
+#define DEFINE_INT_TO_STRING_FUNCTION(RETURN_TYPE, EXPECTED_TYPE, FUNCTION) \
+ constexpr LooseMatchFunctor<RETURN_TYPE, EXPECTED_TYPE, internal::FUNCTION> \
+ FUNCTION
+
+#define DEFINE_EXACT_MATCH_INT_TO_STRING_FUNCTION(RETURN_TYPE, EXPECTED_TYPE, \
+ FUNCTION) \
+ constexpr ExactMatchFunctor<RETURN_TYPE, EXPECTED_TYPE, internal::FUNCTION> \
+ FUNCTION
+
+DEFINE_INT_TO_STRING_FUNCTION(std::string, int, IntToString);
+DEFINE_INT_TO_STRING_FUNCTION(string16, int, IntToString16);
+DEFINE_INT_TO_STRING_FUNCTION(std::string, uint32, UintToString);
+DEFINE_INT_TO_STRING_FUNCTION(string16, uint32, UintToString16);
+DEFINE_INT_TO_STRING_FUNCTION(std::string, int64, Int64ToString);
+DEFINE_INT_TO_STRING_FUNCTION(string16, int64, Int64ToString16);
+DEFINE_INT_TO_STRING_FUNCTION(std::string, uint64, Uint64ToString);
+DEFINE_INT_TO_STRING_FUNCTION(string16, uint64, Uint64ToString16);
+DEFINE_EXACT_MATCH_INT_TO_STRING_FUNCTION(std::string, size_t, SizeTToString);
+DEFINE_EXACT_MATCH_INT_TO_STRING_FUNCTION(string16, size_t, SizeTToString16);
+DEFINE_INT_TO_STRING_FUNCTION(std::string, double, DoubleToString);
+
+#endif
+
} // namespace base
#endif // BASE_STRINGS_STRING_NUMBER_CONVERSIONS_H_
« no previous file with comments | « no previous file | base/strings/string_number_conversions.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698