Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

some static analysis warning at line 11317 #1390

Closed
ipodipad opened this issue Dec 10, 2018 · 5 comments · Fixed by #3227
Closed

some static analysis warning at line 11317 #1390

ipodipad opened this issue Dec 10, 2018 · 5 comments · Fixed by #3227
Assignees
Labels
release item: 🔨 further change solution: proposed fix a fix for the issue has been proposed and waits for confirmation

Comments

@ipodipad
Copy link

ipodipad commented Dec 10, 2018

  • What is the issue you have?
Firstly, thanks for making this great easy json utility!
I'm not sure this is false positive or something, but I just report this warning.
May this be helpful~

at line 11317 in json.hpp
const bool is_negative = std::is_same<NumberType, number_integer_t>::value and not (x >= 0);  // see issue #755
static analysis tool says (codesonar),
Redundant Condition
x >= 0 always evaluates to true.
The condition is testing whether an unsigned value is nonnegative, but unsigned values are always nonnegative.
  • Please describe the steps to reproduce the issue. Can you provide a small but working code example?

  • What is the expected behavior?

  • And what is the actual behavior instead?

  • Which compiler and operating system are you using? Is it a supported compiler?

arm-linux-gnueabihf-g++ (Linaro GCC 6.3-2017.05) 6.3.1 20170404
  • Did you use a released version of the library or the version from the develop branch?
version: 3.4.0 from develop branch 
@nlohmann
Copy link
Owner

The function is templated for integer types. For signed integers, the line can become false, whereas it is always true for unsigned integer types. To "fix" this issue, I would need to artificially make two functions from it and remove some code from the unsigned version. Therefore, I would classify this as false positive. What do you think?

@nlohmann nlohmann added the solution: wontfix the issue will not be fixed (either it is impossible or deemed out of scope) label Dec 20, 2018
@mgerhardy
Copy link

This is not just static analysis - the compiler prints this, too. A proper fix is attached at #755

    template<typename NumberType, detail::enable_if_t<
                 std::is_same<NumberType, number_unsigned_t>::value, int> = 0>
    bool is_negative_integer(NumberType x) { return false; }

    template<typename NumberType, detail::enable_if_t<
                 std::is_same<NumberType, number_integer_t>::value, int> = 0>
    bool is_negative_integer(NumberType x) { return x<0; }

and in dump_integer(NumberType x):
const bool is_negative = is_negative_integer(x); // see issue #755

@mgerhardy
Copy link

oh.. the compiler that reports this as a warning is clang-10 btw. and as we are compiling with -Werror - this is a problem for us

@t-b
Copy link
Contributor

t-b commented May 8, 2020

@mgerhardy Can you provide a PR?

@darkdragon-001
Copy link

This was fixed in c61a907.

@nlohmann nlohmann reopened this Dec 29, 2021
@nlohmann nlohmann linked a pull request Dec 29, 2021 that will close this issue
@nlohmann nlohmann added release item: 🔨 further change solution: proposed fix a fix for the issue has been proposed and waits for confirmation and removed solution: wontfix the issue will not be fixed (either it is impossible or deemed out of scope) labels Dec 30, 2021
@nlohmann nlohmann self-assigned this Dec 30, 2021
@nlohmann nlohmann added this to the Release 3.10.5 milestone Dec 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release item: 🔨 further change solution: proposed fix a fix for the issue has been proposed and waits for confirmation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants