[PATCH] D113946: [libc][clang-tidy] fix namespace check for externals
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG155f5a6dac62: [libc][clang-tidy] fix namespace check for externals (authored by michaelrj). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113946/new/ https://reviews.llvm.org/D113946 Files: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp clang-tools-extra/test/clang-tidy/checkers/llvmlibc-callee-namespace.cpp libc/docs/clang_tidy_checks.rst libc/src/__support/FPUtil/NearestIntegerOperations.h libc/src/__support/str_to_float.h libc/src/__support/str_to_integer.h libc/src/math/generic/math_utils.h libc/src/string/strdup.cpp libc/src/string/strndup.cpp Index: libc/src/string/strndup.cpp === --- libc/src/string/strndup.cpp +++ libc/src/string/strndup.cpp @@ -23,7 +23,7 @@ size_t len = internal::string_length(src); if (len > size) len = size; - char *dest = reinterpret_cast(::malloc(len + 1)); // NOLINT + char *dest = reinterpret_cast(::malloc(len + 1)); if (dest == nullptr) return nullptr; inline_memcpy(dest, src, len + 1); Index: libc/src/string/strdup.cpp === --- libc/src/string/strdup.cpp +++ libc/src/string/strdup.cpp @@ -21,7 +21,7 @@ return nullptr; } size_t len = internal::string_length(src) + 1; - char *dest = reinterpret_cast(::malloc(len)); // NOLINT + char *dest = reinterpret_cast(::malloc(len)); if (dest == nullptr) { return nullptr; } Index: libc/src/math/generic/math_utils.h === --- libc/src/math/generic/math_utils.h +++ libc/src/math/generic/math_utils.h @@ -55,7 +55,7 @@ template static inline T with_errno(T x, int err) { if (math_errhandling & MATH_ERRNO) -errno = err; // NOLINT +errno = err; return x; } Index: libc/src/__support/str_to_integer.h === --- libc/src/__support/str_to_integer.h +++ libc/src/__support/str_to_integer.h @@ -74,7 +74,7 @@ const char *original_src = src; if (base < 0 || base == 1 || base > 36) { -errno = EINVAL; // NOLINT +errno = EINVAL; return 0; } @@ -114,19 +114,19 @@ // the result cannot change, but we still need to advance src to the end of // the number. if (result == ABS_MAX) { - errno = ERANGE; // NOLINT + errno = ERANGE; continue; } if (result > ABS_MAX_DIV_BY_BASE) { result = ABS_MAX; - errno = ERANGE; // NOLINT + errno = ERANGE; } else { result = result * base; } if (result > ABS_MAX - cur_digit) { result = ABS_MAX; - errno = ERANGE; // NOLINT + errno = ERANGE; } else { result = result + cur_digit; } Index: libc/src/__support/str_to_float.h === --- libc/src/__support/str_to_float.h +++ libc/src/__support/str_to_float.h @@ -200,7 +200,7 @@ static_cast(fputil::FloatProperties::exponentBias)) { *outputMantissa = 0; *outputExp2 = fputil::FPBits::maxExponent; -errno = ERANGE; // NOLINT +errno = ERANGE; return; } // If the exponent is too small even for a subnormal, return 0. @@ -210,7 +210,7 @@ fputil::FloatProperties::mantissaWidth)) { *outputMantissa = 0; *outputExp2 = 0; -errno = ERANGE; // NOLINT +errno = ERANGE; return; } @@ -253,7 +253,7 @@ if (exp2 >= fputil::FPBits::maxExponent) { *outputMantissa = 0; *outputExp2 = fputil::FPBits::maxExponent; -errno = ERANGE; // NOLINT +errno = ERANGE; return; } @@ -289,7 +289,7 @@ } if (exp2 == 0) { -errno = ERANGE; // NOLINT +errno = ERANGE; } *outputMantissa = finalMantissa; @@ -391,7 +391,7 @@ static_cast(fputil::FloatProperties::exponentBias) / 3) { *outputMantissa = 0; *outputExp2 = fputil::FPBits::maxExponent; -errno = ERANGE; // NOLINT +errno = ERANGE; return; } // If the exponent is too small even for a subnormal, return 0. @@ -402,7 +402,7 @@ 2) { *outputMantissa = 0; *outputExp2 = 0; -errno = ERANGE; // NOLINT +errno = ERANGE; return; } @@ -467,7 +467,7 @@ // This indicates an overflow, so we make the result INF and set errno. *outputExp2 = (1 << fputil::FloatProperties::exponentWidth) - 1; *outputMantissa = 0; -errno = ERANGE; // NOLINT +errno = ERANGE; return; } @@ -483,7 +483,7 @@ // Return 0 if the exponent is too small. *outputMantissa = 0; *outputExp2 = 0; - errno = ERANGE; // NOLINT + errno = ERANGE; return; } } @@ -511,12 +511,12 @@
[PATCH] D113946: [libc][clang-tidy] fix namespace check for externals
michaelrj updated this revision to Diff 390769. michaelrj added a comment. fix final comments before commit Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113946/new/ https://reviews.llvm.org/D113946 Files: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp clang-tools-extra/test/clang-tidy/checkers/llvmlibc-callee-namespace.cpp libc/docs/clang_tidy_checks.rst libc/src/__support/FPUtil/NearestIntegerOperations.h libc/src/__support/str_to_float.h libc/src/__support/str_to_integer.h libc/src/math/generic/math_utils.h libc/src/string/strdup.cpp libc/src/string/strndup.cpp Index: libc/src/string/strndup.cpp === --- libc/src/string/strndup.cpp +++ libc/src/string/strndup.cpp @@ -23,7 +23,7 @@ size_t len = internal::string_length(src); if (len > size) len = size; - char *dest = reinterpret_cast(::malloc(len + 1)); // NOLINT + char *dest = reinterpret_cast(::malloc(len + 1)); if (dest == nullptr) return nullptr; inline_memcpy(dest, src, len + 1); Index: libc/src/string/strdup.cpp === --- libc/src/string/strdup.cpp +++ libc/src/string/strdup.cpp @@ -21,7 +21,7 @@ return nullptr; } size_t len = internal::string_length(src) + 1; - char *dest = reinterpret_cast(::malloc(len)); // NOLINT + char *dest = reinterpret_cast(::malloc(len)); if (dest == nullptr) { return nullptr; } Index: libc/src/math/generic/math_utils.h === --- libc/src/math/generic/math_utils.h +++ libc/src/math/generic/math_utils.h @@ -55,7 +55,7 @@ template static inline T with_errno(T x, int err) { if (math_errhandling & MATH_ERRNO) -errno = err; // NOLINT +errno = err; return x; } Index: libc/src/__support/str_to_integer.h === --- libc/src/__support/str_to_integer.h +++ libc/src/__support/str_to_integer.h @@ -74,7 +74,7 @@ const char *original_src = src; if (base < 0 || base == 1 || base > 36) { -errno = EINVAL; // NOLINT +errno = EINVAL; return 0; } @@ -114,19 +114,19 @@ // the result cannot change, but we still need to advance src to the end of // the number. if (result == ABS_MAX) { - errno = ERANGE; // NOLINT + errno = ERANGE; continue; } if (result > ABS_MAX_DIV_BY_BASE) { result = ABS_MAX; - errno = ERANGE; // NOLINT + errno = ERANGE; } else { result = result * base; } if (result > ABS_MAX - cur_digit) { result = ABS_MAX; - errno = ERANGE; // NOLINT + errno = ERANGE; } else { result = result + cur_digit; } Index: libc/src/__support/str_to_float.h === --- libc/src/__support/str_to_float.h +++ libc/src/__support/str_to_float.h @@ -200,7 +200,7 @@ static_cast(fputil::FloatProperties::exponentBias)) { *outputMantissa = 0; *outputExp2 = fputil::FPBits::maxExponent; -errno = ERANGE; // NOLINT +errno = ERANGE; return; } // If the exponent is too small even for a subnormal, return 0. @@ -210,7 +210,7 @@ fputil::FloatProperties::mantissaWidth)) { *outputMantissa = 0; *outputExp2 = 0; -errno = ERANGE; // NOLINT +errno = ERANGE; return; } @@ -253,7 +253,7 @@ if (exp2 >= fputil::FPBits::maxExponent) { *outputMantissa = 0; *outputExp2 = fputil::FPBits::maxExponent; -errno = ERANGE; // NOLINT +errno = ERANGE; return; } @@ -289,7 +289,7 @@ } if (exp2 == 0) { -errno = ERANGE; // NOLINT +errno = ERANGE; } *outputMantissa = finalMantissa; @@ -391,7 +391,7 @@ static_cast(fputil::FloatProperties::exponentBias) / 3) { *outputMantissa = 0; *outputExp2 = fputil::FPBits::maxExponent; -errno = ERANGE; // NOLINT +errno = ERANGE; return; } // If the exponent is too small even for a subnormal, return 0. @@ -402,7 +402,7 @@ 2) { *outputMantissa = 0; *outputExp2 = 0; -errno = ERANGE; // NOLINT +errno = ERANGE; return; } @@ -467,7 +467,7 @@ // This indicates an overflow, so we make the result INF and set errno. *outputExp2 = (1 << fputil::FloatProperties::exponentWidth) - 1; *outputMantissa = 0; -errno = ERANGE; // NOLINT +errno = ERANGE; return; } @@ -483,7 +483,7 @@ // Return 0 if the exponent is too small. *outputMantissa = 0; *outputExp2 = 0; - errno = ERANGE; // NOLINT + errno = ERANGE; return; } } @@ -511,12 +511,12 @@ ++biasedExponent; if (biasedExponent == INF_EXP) { - errno = ERANGE; // NOLINT + errno = ERANGE; } } if
[PATCH] D113946: [libc][clang-tidy] fix namespace check for externals
aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM aside from a tiny nit with the documentation. Comment at: libc/docs/clang_tidy_checks.rst:71 +There are exceptions for the following functions: +``__errno_location`` so that errno can be set +``malloc``, ``calloc``, ``realloc``, and ``free`` since they are always external Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113946/new/ https://reviews.llvm.org/D113946 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D113946: [libc][clang-tidy] fix namespace check for externals
michaelrj updated this revision to Diff 388588. michaelrj marked 3 inline comments as done. michaelrj added a comment. address comments and rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113946/new/ https://reviews.llvm.org/D113946 Files: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp clang-tools-extra/test/clang-tidy/checkers/llvmlibc-callee-namespace.cpp libc/docs/clang_tidy_checks.rst libc/src/__support/FPUtil/NearestIntegerOperations.h libc/src/__support/str_to_float.h libc/src/__support/str_to_integer.h libc/src/math/generic/math_utils.h libc/src/string/strdup.cpp libc/src/string/strndup.cpp Index: libc/src/string/strndup.cpp === --- libc/src/string/strndup.cpp +++ libc/src/string/strndup.cpp @@ -23,7 +23,7 @@ size_t len = internal::string_length(src); if (len > size) len = size; - char *dest = reinterpret_cast(::malloc(len + 1)); // NOLINT + char *dest = reinterpret_cast(::malloc(len + 1)); if (dest == nullptr) return nullptr; char *result = Index: libc/src/string/strdup.cpp === --- libc/src/string/strdup.cpp +++ libc/src/string/strdup.cpp @@ -21,7 +21,7 @@ return nullptr; } size_t len = internal::string_length(src) + 1; - char *dest = reinterpret_cast(::malloc(len)); // NOLINT + char *dest = reinterpret_cast(::malloc(len)); if (dest == nullptr) { return nullptr; } Index: libc/src/math/generic/math_utils.h === --- libc/src/math/generic/math_utils.h +++ libc/src/math/generic/math_utils.h @@ -55,7 +55,7 @@ template static inline T with_errno(T x, int err) { if (math_errhandling & MATH_ERRNO) -errno = err; // NOLINT +errno = err; return x; } Index: libc/src/__support/str_to_integer.h === --- libc/src/__support/str_to_integer.h +++ libc/src/__support/str_to_integer.h @@ -74,7 +74,7 @@ const char *original_src = src; if (base < 0 || base == 1 || base > 36) { -errno = EINVAL; // NOLINT +errno = EINVAL; return 0; } @@ -114,19 +114,19 @@ // the result cannot change, but we still need to advance src to the end of // the number. if (result == ABS_MAX) { - errno = ERANGE; // NOLINT + errno = ERANGE; continue; } if (result > ABS_MAX_DIV_BY_BASE) { result = ABS_MAX; - errno = ERANGE; // NOLINT + errno = ERANGE; } else { result = result * base; } if (result > ABS_MAX - cur_digit) { result = ABS_MAX; - errno = ERANGE; // NOLINT + errno = ERANGE; } else { result = result + cur_digit; } Index: libc/src/__support/str_to_float.h === --- libc/src/__support/str_to_float.h +++ libc/src/__support/str_to_float.h @@ -200,7 +200,7 @@ static_cast(fputil::FloatProperties::exponentBias)) { *outputMantissa = 0; *outputExp2 = fputil::FPBits::maxExponent; -errno = ERANGE; // NOLINT +errno = ERANGE; return; } // If the exponent is too small even for a subnormal, return 0. @@ -210,7 +210,7 @@ fputil::FloatProperties::mantissaWidth)) { *outputMantissa = 0; *outputExp2 = 0; -errno = ERANGE; // NOLINT +errno = ERANGE; return; } @@ -253,7 +253,7 @@ if (exp2 >= fputil::FPBits::maxExponent) { *outputMantissa = 0; *outputExp2 = fputil::FPBits::maxExponent; -errno = ERANGE; // NOLINT +errno = ERANGE; return; } @@ -289,7 +289,7 @@ } if (exp2 == 0) { -errno = ERANGE; // NOLINT +errno = ERANGE; } *outputMantissa = finalMantissa; @@ -391,7 +391,7 @@ static_cast(fputil::FloatProperties::exponentBias) / 3) { *outputMantissa = 0; *outputExp2 = fputil::FPBits::maxExponent; -errno = ERANGE; // NOLINT +errno = ERANGE; return; } // If the exponent is too small even for a subnormal, return 0. @@ -402,7 +402,7 @@ 2) { *outputMantissa = 0; *outputExp2 = 0; -errno = ERANGE; // NOLINT +errno = ERANGE; return; } @@ -467,7 +467,7 @@ // This indicates an overflow, so we make the result INF and set errno. *outputExp2 = (1 << fputil::FloatProperties::exponentWidth) - 1; *outputMantissa = 0; -errno = ERANGE; // NOLINT +errno = ERANGE; return; } @@ -483,7 +483,7 @@ // Return 0 if the exponent is too small. *outputMantissa = 0; *outputExp2 = 0; - errno = ERANGE; // NOLINT + errno = ERANGE; return; } } @@ -511,12 +511,12 @@ ++biasedExponent; if (biasedExponent == INF_EXP) { - errno = ERANGE; // NOLINT + errno = ERANGE;
[PATCH] D113946: [libc][clang-tidy] fix namespace check for externals
aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp:39 +// intercepted. +static const llvm::StringSet<> FUNCTIONS_TO_IGNORE_NAMESPACE = { +"__errno_location", "malloc", "calloc", "realloc", "free"}; This is more in line with the coding style guidelines. Comment at: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp:40 +static const llvm::StringSet<> FUNCTIONS_TO_IGNORE_NAMESPACE = { +"__errno_location", "malloc", "calloc", "realloc", "free"}; + Is there a reason that `aligned_alloc` function is excluded? Comment at: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp:55-57 + if (FUNCTIONS_TO_IGNORE_NAMESPACE.contains(FuncDecl->getName())) { +return; + } Style guideline nits. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113946/new/ https://reviews.llvm.org/D113946 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D113946: [libc][clang-tidy] fix namespace check for externals
michaelrj added a comment. ping for clang-tidy review Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113946/new/ https://reviews.llvm.org/D113946 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D113946: [libc][clang-tidy] fix namespace check for externals
michaelrj added inline comments. Comment at: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp:39 +// intercepted. +static const char *FUNCTIONS_TO_IGNORE_NAMESPACE[] = { +"__errno_location", "malloc", "calloc", "realloc", "free"}; sivachandra wrote: > michaelrj wrote: > > sivachandra wrote: > > > Eugene.Zelenko wrote: > > > > Why not `std::array` of appropriate LLVM container? > > > May be `static const std::uordered_set`? It would likely > > > make the lookup below much neater. > > an unordered set is a good idea, but the documentation for `StringRef` says > > it's best not to use them for storage, so I went with `std::string` > > instead. The code is still a lot nicer. > Literal strings will not count as "storage". They are global data. On the > other hand, `std::string` will make copies of the literals and require > "storage". When I tried to put a `StringRef` into an `unordered_set` it told me that `StringRef` doesn't have a hash function, so I went looking for what the proper, LLVM way of doing this was. In the end, I discovered that `unordered_set` is not recommended, and that `StringSet` does exactly what we need here, so I've switched to that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113946/new/ https://reviews.llvm.org/D113946 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D113946: [libc][clang-tidy] fix namespace check for externals
michaelrj updated this revision to Diff 387759. michaelrj marked an inline comment as done. michaelrj added a comment. update the set type Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113946/new/ https://reviews.llvm.org/D113946 Files: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp clang-tools-extra/test/clang-tidy/checkers/llvmlibc-callee-namespace.cpp libc/docs/clang_tidy_checks.rst libc/src/__support/FPUtil/NearestIntegerOperations.h libc/src/__support/str_to_float.h libc/src/__support/str_to_integer.h libc/src/math/generic/math_utils.h libc/src/string/strdup.cpp libc/src/string/strndup.cpp Index: libc/src/string/strndup.cpp === --- libc/src/string/strndup.cpp +++ libc/src/string/strndup.cpp @@ -23,7 +23,7 @@ size_t len = internal::string_length(src); if (len > size) len = size; - char *dest = reinterpret_cast(::malloc(len + 1)); // NOLINT + char *dest = reinterpret_cast(::malloc(len + 1)); if (dest == nullptr) return nullptr; char *result = Index: libc/src/string/strdup.cpp === --- libc/src/string/strdup.cpp +++ libc/src/string/strdup.cpp @@ -21,7 +21,7 @@ return nullptr; } size_t len = internal::string_length(src) + 1; - char *dest = reinterpret_cast(::malloc(len)); // NOLINT + char *dest = reinterpret_cast(::malloc(len)); if (dest == nullptr) { return nullptr; } Index: libc/src/math/generic/math_utils.h === --- libc/src/math/generic/math_utils.h +++ libc/src/math/generic/math_utils.h @@ -55,7 +55,7 @@ template static inline T with_errno(T x, int err) { if (math_errhandling & MATH_ERRNO) -errno = err; // NOLINT +errno = err; return x; } Index: libc/src/__support/str_to_integer.h === --- libc/src/__support/str_to_integer.h +++ libc/src/__support/str_to_integer.h @@ -74,7 +74,7 @@ const char *original_src = src; if (base < 0 || base == 1 || base > 36) { -errno = EINVAL; // NOLINT +errno = EINVAL; return 0; } @@ -114,19 +114,19 @@ // the result cannot change, but we still need to advance src to the end of // the number. if (result == ABS_MAX) { - errno = ERANGE; // NOLINT + errno = ERANGE; continue; } if (result > ABS_MAX_DIV_BY_BASE) { result = ABS_MAX; - errno = ERANGE; // NOLINT + errno = ERANGE; } else { result = result * base; } if (result > ABS_MAX - cur_digit) { result = ABS_MAX; - errno = ERANGE; // NOLINT + errno = ERANGE; } else { result = result + cur_digit; } Index: libc/src/__support/str_to_float.h === --- libc/src/__support/str_to_float.h +++ libc/src/__support/str_to_float.h @@ -220,7 +220,7 @@ static_cast(fputil::FloatProperties::exponentBias)) { *outputMantissa = 0; *outputExp2 = fputil::FPBits::maxExponent; -errno = ERANGE; // NOLINT +errno = ERANGE; return; } // If the exponent is too small even for a subnormal, return 0. @@ -230,7 +230,7 @@ fputil::FloatProperties::mantissaWidth)) { *outputMantissa = 0; *outputExp2 = 0; -errno = ERANGE; // NOLINT +errno = ERANGE; return; } @@ -273,7 +273,7 @@ if (exp2 >= fputil::FPBits::maxExponent) { *outputMantissa = 0; *outputExp2 = fputil::FPBits::maxExponent; -errno = ERANGE; // NOLINT +errno = ERANGE; return; } @@ -309,7 +309,7 @@ } if (exp2 == 0) { -errno = ERANGE; // NOLINT +errno = ERANGE; } *outputMantissa = finalMantissa; @@ -411,7 +411,7 @@ static_cast(fputil::FloatProperties::exponentBias) / 3) { *outputMantissa = 0; *outputExp2 = fputil::FPBits::maxExponent; -errno = ERANGE; // NOLINT +errno = ERANGE; return; } // If the exponent is too small even for a subnormal, return 0. @@ -422,7 +422,7 @@ 2) { *outputMantissa = 0; *outputExp2 = 0; -errno = ERANGE; // NOLINT +errno = ERANGE; return; } @@ -508,7 +508,7 @@ // If we cut off any bits to fit this number into a subnormal, then it's // out of range for this size of float. if ((mantissa & ((1 << amountToShift) - 1)) > 0) { - errno = ERANGE; // NOLINT + errno = ERANGE; } mantissa = shiftRightAndRound(mantissa, amountToShift); if (mantissa == OVERFLOWED_MANTISSA) { @@ -524,7 +524,7 @@ // This indicates an overflow, so we make the result INF and set errno. biasedExponent = (1 << fputil::FloatProperties::exponentWidth) - 1; mantissa = 0; -errno = ERANGE; // NOLINT +errno = ERANGE; }
[PATCH] D113946: [libc][clang-tidy] fix namespace check for externals
sivachandra accepted this revision. sivachandra added a comment. For libc requirements, LGTM. Please wait for @aaron.ballman for stamping the clang-tidy parts. Comment at: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp:39 +// intercepted. +static const char *FUNCTIONS_TO_IGNORE_NAMESPACE[] = { +"__errno_location", "malloc", "calloc", "realloc", "free"}; michaelrj wrote: > sivachandra wrote: > > Eugene.Zelenko wrote: > > > Why not `std::array` of appropriate LLVM container? > > May be `static const std::uordered_set`? It would likely > > make the lookup below much neater. > an unordered set is a good idea, but the documentation for `StringRef` says > it's best not to use them for storage, so I went with `std::string` instead. > The code is still a lot nicer. Literal strings will not count as "storage". They are global data. On the other hand, `std::string` will make copies of the literals and require "storage". Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113946/new/ https://reviews.llvm.org/D113946 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D113946: [libc][clang-tidy] fix namespace check for externals
michaelrj updated this revision to Diff 387713. michaelrj added a comment. add test for the new lint behavior: Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113946/new/ https://reviews.llvm.org/D113946 Files: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp clang-tools-extra/test/clang-tidy/checkers/llvmlibc-callee-namespace.cpp libc/docs/clang_tidy_checks.rst libc/src/__support/FPUtil/NearestIntegerOperations.h libc/src/__support/str_to_float.h libc/src/__support/str_to_integer.h libc/src/math/generic/math_utils.h libc/src/string/strdup.cpp libc/src/string/strndup.cpp Index: libc/src/string/strndup.cpp === --- libc/src/string/strndup.cpp +++ libc/src/string/strndup.cpp @@ -23,7 +23,7 @@ size_t len = internal::string_length(src); if (len > size) len = size; - char *dest = reinterpret_cast(::malloc(len + 1)); // NOLINT + char *dest = reinterpret_cast(::malloc(len + 1)); if (dest == nullptr) return nullptr; char *result = Index: libc/src/string/strdup.cpp === --- libc/src/string/strdup.cpp +++ libc/src/string/strdup.cpp @@ -21,7 +21,7 @@ return nullptr; } size_t len = internal::string_length(src) + 1; - char *dest = reinterpret_cast(::malloc(len)); // NOLINT + char *dest = reinterpret_cast(::malloc(len)); if (dest == nullptr) { return nullptr; } Index: libc/src/math/generic/math_utils.h === --- libc/src/math/generic/math_utils.h +++ libc/src/math/generic/math_utils.h @@ -55,7 +55,7 @@ template static inline T with_errno(T x, int err) { if (math_errhandling & MATH_ERRNO) -errno = err; // NOLINT +errno = err; return x; } Index: libc/src/__support/str_to_integer.h === --- libc/src/__support/str_to_integer.h +++ libc/src/__support/str_to_integer.h @@ -74,7 +74,7 @@ const char *original_src = src; if (base < 0 || base == 1 || base > 36) { -errno = EINVAL; // NOLINT +errno = EINVAL; return 0; } @@ -114,19 +114,19 @@ // the result cannot change, but we still need to advance src to the end of // the number. if (result == ABS_MAX) { - errno = ERANGE; // NOLINT + errno = ERANGE; continue; } if (result > ABS_MAX_DIV_BY_BASE) { result = ABS_MAX; - errno = ERANGE; // NOLINT + errno = ERANGE; } else { result = result * base; } if (result > ABS_MAX - cur_digit) { result = ABS_MAX; - errno = ERANGE; // NOLINT + errno = ERANGE; } else { result = result + cur_digit; } Index: libc/src/__support/str_to_float.h === --- libc/src/__support/str_to_float.h +++ libc/src/__support/str_to_float.h @@ -220,7 +220,7 @@ static_cast(fputil::FloatProperties::exponentBias)) { *outputMantissa = 0; *outputExp2 = fputil::FPBits::maxExponent; -errno = ERANGE; // NOLINT +errno = ERANGE; return; } // If the exponent is too small even for a subnormal, return 0. @@ -230,7 +230,7 @@ fputil::FloatProperties::mantissaWidth)) { *outputMantissa = 0; *outputExp2 = 0; -errno = ERANGE; // NOLINT +errno = ERANGE; return; } @@ -273,7 +273,7 @@ if (exp2 >= fputil::FPBits::maxExponent) { *outputMantissa = 0; *outputExp2 = fputil::FPBits::maxExponent; -errno = ERANGE; // NOLINT +errno = ERANGE; return; } @@ -309,7 +309,7 @@ } if (exp2 == 0) { -errno = ERANGE; // NOLINT +errno = ERANGE; } *outputMantissa = finalMantissa; @@ -411,7 +411,7 @@ static_cast(fputil::FloatProperties::exponentBias) / 3) { *outputMantissa = 0; *outputExp2 = fputil::FPBits::maxExponent; -errno = ERANGE; // NOLINT +errno = ERANGE; return; } // If the exponent is too small even for a subnormal, return 0. @@ -422,7 +422,7 @@ 2) { *outputMantissa = 0; *outputExp2 = 0; -errno = ERANGE; // NOLINT +errno = ERANGE; return; } @@ -508,7 +508,7 @@ // If we cut off any bits to fit this number into a subnormal, then it's // out of range for this size of float. if ((mantissa & ((1 << amountToShift) - 1)) > 0) { - errno = ERANGE; // NOLINT + errno = ERANGE; } mantissa = shiftRightAndRound(mantissa, amountToShift); if (mantissa == OVERFLOWED_MANTISSA) { @@ -524,7 +524,7 @@ // This indicates an overflow, so we make the result INF and set errno. biasedExponent = (1 << fputil::FloatProperties::exponentWidth) - 1; mantissa = 0; -errno = ERANGE; // NOLINT +errno = ERANGE; } *outputMantissa = mantissa;
[PATCH] D113946: [libc][clang-tidy] fix namespace check for externals
michaelrj added inline comments. Comment at: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp:13 +#include + sivachandra wrote: > Eugene.Zelenko wrote: > > Should be `` and without newline separation form rest of headers. > Looks like this is present only for the debug `printf`? yes, I didn't mean to leave that in there. Comment at: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp:39 +// intercepted. +static const char *FUNCTIONS_TO_IGNORE_NAMESPACE[] = { +"__errno_location", "malloc", "calloc", "realloc", "free"}; sivachandra wrote: > Eugene.Zelenko wrote: > > Why not `std::array` of appropriate LLVM container? > May be `static const std::uordered_set`? It would likely > make the lookup below much neater. an unordered set is a good idea, but the documentation for `StringRef` says it's best not to use them for storage, so I went with `std::string` instead. The code is still a lot nicer. Comment at: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp:59 +llvm::StringRef(FUNCTIONS_TO_IGNORE_NAMESPACE[i]))) { + printf("String found %s\n", FuncDecl->getName().str().c_str()); + return; sivachandra wrote: > lntue wrote: > > Look like diag() is used to print messages in this module? > This looks like a debug `printf`? yes, I didn't mean to leave that in there. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113946/new/ https://reviews.llvm.org/D113946 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D113946: [libc][clang-tidy] fix namespace check for externals
michaelrj updated this revision to Diff 387703. michaelrj marked 6 inline comments as done. michaelrj added a comment. clean up the code and remove debug statements Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113946/new/ https://reviews.llvm.org/D113946 Files: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp libc/docs/clang_tidy_checks.rst libc/src/__support/FPUtil/NearestIntegerOperations.h libc/src/__support/str_to_float.h libc/src/__support/str_to_integer.h libc/src/math/generic/math_utils.h libc/src/string/strdup.cpp libc/src/string/strndup.cpp Index: libc/src/string/strndup.cpp === --- libc/src/string/strndup.cpp +++ libc/src/string/strndup.cpp @@ -23,7 +23,7 @@ size_t len = internal::string_length(src); if (len > size) len = size; - char *dest = reinterpret_cast(::malloc(len + 1)); // NOLINT + char *dest = reinterpret_cast(::malloc(len + 1)); if (dest == nullptr) return nullptr; char *result = Index: libc/src/string/strdup.cpp === --- libc/src/string/strdup.cpp +++ libc/src/string/strdup.cpp @@ -21,7 +21,7 @@ return nullptr; } size_t len = internal::string_length(src) + 1; - char *dest = reinterpret_cast(::malloc(len)); // NOLINT + char *dest = reinterpret_cast(::malloc(len)); if (dest == nullptr) { return nullptr; } Index: libc/src/math/generic/math_utils.h === --- libc/src/math/generic/math_utils.h +++ libc/src/math/generic/math_utils.h @@ -55,7 +55,7 @@ template static inline T with_errno(T x, int err) { if (math_errhandling & MATH_ERRNO) -errno = err; // NOLINT +errno = err; return x; } Index: libc/src/__support/str_to_integer.h === --- libc/src/__support/str_to_integer.h +++ libc/src/__support/str_to_integer.h @@ -74,7 +74,7 @@ const char *original_src = src; if (base < 0 || base == 1 || base > 36) { -errno = EINVAL; // NOLINT +errno = EINVAL; return 0; } @@ -114,19 +114,19 @@ // the result cannot change, but we still need to advance src to the end of // the number. if (result == ABS_MAX) { - errno = ERANGE; // NOLINT + errno = ERANGE; continue; } if (result > ABS_MAX_DIV_BY_BASE) { result = ABS_MAX; - errno = ERANGE; // NOLINT + errno = ERANGE; } else { result = result * base; } if (result > ABS_MAX - cur_digit) { result = ABS_MAX; - errno = ERANGE; // NOLINT + errno = ERANGE; } else { result = result + cur_digit; } Index: libc/src/__support/str_to_float.h === --- libc/src/__support/str_to_float.h +++ libc/src/__support/str_to_float.h @@ -220,7 +220,7 @@ static_cast(fputil::FloatProperties::exponentBias)) { *outputMantissa = 0; *outputExp2 = fputil::FPBits::maxExponent; -errno = ERANGE; // NOLINT +errno = ERANGE; return; } // If the exponent is too small even for a subnormal, return 0. @@ -230,7 +230,7 @@ fputil::FloatProperties::mantissaWidth)) { *outputMantissa = 0; *outputExp2 = 0; -errno = ERANGE; // NOLINT +errno = ERANGE; return; } @@ -273,7 +273,7 @@ if (exp2 >= fputil::FPBits::maxExponent) { *outputMantissa = 0; *outputExp2 = fputil::FPBits::maxExponent; -errno = ERANGE; // NOLINT +errno = ERANGE; return; } @@ -309,7 +309,7 @@ } if (exp2 == 0) { -errno = ERANGE; // NOLINT +errno = ERANGE; } *outputMantissa = finalMantissa; @@ -411,7 +411,7 @@ static_cast(fputil::FloatProperties::exponentBias) / 3) { *outputMantissa = 0; *outputExp2 = fputil::FPBits::maxExponent; -errno = ERANGE; // NOLINT +errno = ERANGE; return; } // If the exponent is too small even for a subnormal, return 0. @@ -422,7 +422,7 @@ 2) { *outputMantissa = 0; *outputExp2 = 0; -errno = ERANGE; // NOLINT +errno = ERANGE; return; } @@ -508,7 +508,7 @@ // If we cut off any bits to fit this number into a subnormal, then it's // out of range for this size of float. if ((mantissa & ((1 << amountToShift) - 1)) > 0) { - errno = ERANGE; // NOLINT + errno = ERANGE; } mantissa = shiftRightAndRound(mantissa, amountToShift); if (mantissa == OVERFLOWED_MANTISSA) { @@ -524,7 +524,7 @@ // This indicates an overflow, so we make the result INF and set errno. biasedExponent = (1 << fputil::FloatProperties::exponentWidth) - 1; mantissa = 0; -errno = ERANGE; // NOLINT +errno = ERANGE; } *outputMantissa = mantissa; *outputExp2 =
[PATCH] D113946: [libc][clang-tidy] fix namespace check for externals
sivachandra added inline comments. Comment at: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp:1 //===-- CalleeNamespaceCheck.cpp --===// // You should add tests for this exception check. See https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/test/clang-tidy/checkers/llvmlibc-callee-namespace.cpp. Comment at: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp:13 +#include + Eugene.Zelenko wrote: > Should be `` and without newline separation form rest of headers. Looks like this is present only for the debug `printf`? Comment at: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp:39 +// intercepted. +static const char *FUNCTIONS_TO_IGNORE_NAMESPACE[] = { +"__errno_location", "malloc", "calloc", "realloc", "free"}; Eugene.Zelenko wrote: > Why not `std::array` of appropriate LLVM container? May be `static const std::uordered_set`? It would likely make the lookup below much neater. Comment at: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp:59 +llvm::StringRef(FUNCTIONS_TO_IGNORE_NAMESPACE[i]))) { + printf("String found %s\n", FuncDecl->getName().str().c_str()); + return; lntue wrote: > Look like diag() is used to print messages in this module? This looks like a debug `printf`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113946/new/ https://reviews.llvm.org/D113946 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D113946: [libc][clang-tidy] fix namespace check for externals
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp:13 +#include + Should be `` and without newline separation form rest of headers. Comment at: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp:39 +// intercepted. +static const char *FUNCTIONS_TO_IGNORE_NAMESPACE[] = { +"__errno_location", "malloc", "calloc", "realloc", "free"}; Why not `std::array` of appropriate LLVM container? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113946/new/ https://reviews.llvm.org/D113946 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D113946: [libc][clang-tidy] fix namespace check for externals
lntue added inline comments. Comment at: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp:59 +llvm::StringRef(FUNCTIONS_TO_IGNORE_NAMESPACE[i]))) { + printf("String found %s\n", FuncDecl->getName().str().c_str()); + return; Look like diag() is used to print messages in this module? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113946/new/ https://reviews.llvm.org/D113946 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D113946: [libc][clang-tidy] fix namespace check for externals
michaelrj created this revision. michaelrj added reviewers: sivachandra, lntue. Herald added subscribers: libc-commits, carlosgalvezp, ecnelises, tschuett, xazax.hun. Herald added a project: libc-project. michaelrj requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. Up until now, all references to `errno` were marked with `NOLINT`, since it was technically calling an external function. This fixes the lint rules so that `errno`, as well as `malloc`, `calloc`, `realloc`, and `free` are all allowed to be called as external functions. All of the relevant `NOLINT` comments have been removed, and the documentation has been updated. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D113946 Files: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp libc/docs/clang_tidy_checks.rst libc/src/__support/FPUtil/NearestIntegerOperations.h libc/src/__support/str_to_float.h libc/src/__support/str_to_integer.h libc/src/math/generic/math_utils.h libc/src/string/strdup.cpp libc/src/string/strndup.cpp Index: libc/src/string/strndup.cpp === --- libc/src/string/strndup.cpp +++ libc/src/string/strndup.cpp @@ -23,7 +23,7 @@ size_t len = internal::string_length(src); if (len > size) len = size; - char *dest = reinterpret_cast(::malloc(len + 1)); // NOLINT + char *dest = reinterpret_cast(::malloc(len + 1)); if (dest == nullptr) return nullptr; char *result = Index: libc/src/string/strdup.cpp === --- libc/src/string/strdup.cpp +++ libc/src/string/strdup.cpp @@ -21,7 +21,7 @@ return nullptr; } size_t len = internal::string_length(src) + 1; - char *dest = reinterpret_cast(::malloc(len)); // NOLINT + char *dest = reinterpret_cast(::malloc(len)); if (dest == nullptr) { return nullptr; } Index: libc/src/math/generic/math_utils.h === --- libc/src/math/generic/math_utils.h +++ libc/src/math/generic/math_utils.h @@ -55,7 +55,7 @@ template static inline T with_errno(T x, int err) { if (math_errhandling & MATH_ERRNO) -errno = err; // NOLINT +errno = err; return x; } Index: libc/src/__support/str_to_integer.h === --- libc/src/__support/str_to_integer.h +++ libc/src/__support/str_to_integer.h @@ -74,7 +74,7 @@ const char *original_src = src; if (base < 0 || base == 1 || base > 36) { -errno = EINVAL; // NOLINT +errno = EINVAL; return 0; } @@ -114,19 +114,19 @@ // the result cannot change, but we still need to advance src to the end of // the number. if (result == ABS_MAX) { - errno = ERANGE; // NOLINT + errno = ERANGE; continue; } if (result > ABS_MAX_DIV_BY_BASE) { result = ABS_MAX; - errno = ERANGE; // NOLINT + errno = ERANGE; } else { result = result * base; } if (result > ABS_MAX - cur_digit) { result = ABS_MAX; - errno = ERANGE; // NOLINT + errno = ERANGE; } else { result = result + cur_digit; } Index: libc/src/__support/str_to_float.h === --- libc/src/__support/str_to_float.h +++ libc/src/__support/str_to_float.h @@ -220,7 +220,7 @@ static_cast(fputil::FloatProperties::exponentBias)) { *outputMantissa = 0; *outputExp2 = fputil::FPBits::maxExponent; -errno = ERANGE; // NOLINT +errno = ERANGE; return; } // If the exponent is too small even for a subnormal, return 0. @@ -230,7 +230,7 @@ fputil::FloatProperties::mantissaWidth)) { *outputMantissa = 0; *outputExp2 = 0; -errno = ERANGE; // NOLINT +errno = ERANGE; return; } @@ -273,7 +273,7 @@ if (exp2 >= fputil::FPBits::maxExponent) { *outputMantissa = 0; *outputExp2 = fputil::FPBits::maxExponent; -errno = ERANGE; // NOLINT +errno = ERANGE; return; } @@ -309,7 +309,7 @@ } if (exp2 == 0) { -errno = ERANGE; // NOLINT +errno = ERANGE; } *outputMantissa = finalMantissa; @@ -411,7 +411,7 @@ static_cast(fputil::FloatProperties::exponentBias) / 3) { *outputMantissa = 0; *outputExp2 = fputil::FPBits::maxExponent; -errno = ERANGE; // NOLINT +errno = ERANGE; return; } // If the exponent is too small even for a subnormal, return 0. @@ -422,7 +422,7 @@ 2) { *outputMantissa = 0; *outputExp2 = 0; -errno = ERANGE; // NOLINT +errno = ERANGE; return; } @@ -508,7 +508,7 @@ // If we cut off any bits to fit this number into a subnormal, then it's // out of range for this size of float. if ((mantissa & ((1 << amountToShift) - 1)) > 0) { -