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

Warning: unsequenced modification and access to 'range' #1674

Closed
dmax621 opened this issue Jul 17, 2019 · 10 comments
Closed

Warning: unsequenced modification and access to 'range' #1674

dmax621 opened this issue Jul 17, 2019 · 10 comments

Comments

@dmax621
Copy link

dmax621 commented Jul 17, 2019

warning: unsequenced modification and access to 'range' [-Wunsequenced]
    if (JSON_LIKELY(*range <= current and current <= *(++range)))
                     ^                                   ~
1 warning generated.
  • Which compiler and operating system are you using? Is it a supported compiler?
    clang, probably version 3.4.0
    CentOS 6 (probably, or possibly 7).
  • Did you use a released version of the library or the version from the develop branch?
    Version 3.2.0
  • If you experience a compilation error: can you compile and run the unit tests?
    Probably not. The warning was reported by our CI system, which runs tests on whichever of many servers is available, and they have different configurations, in particular different compiler & OS versions. I can't tell which specific machine reported this, and I can't reproduce it on my development system (CentOS 7, clang 3.4.2).

The offending line actually looks perfectly OK to me -- and should be a sequence point -- although not for overloaded operator&&, but this instance has a bool on each side (unless there's some bizarre overloading of operator<= going on).

It could be related to clang not quite treating and the same as &&. Or it could be related to the JSON_LIKELY() macro, and misbehaviour of __builtin_expect(x) when x has side-effects.

Anyway, I have fixed this locally by explicitly fetching the range elements in a well-defined sequence (low first, then high) before using them in the comparison (which is then guaranteed to have no side-effects).

@@ class lexer
-        for (auto range = ranges.begin(); range != ranges.end(); ++range)
+        for (auto range = ranges.begin(); range != ranges.end(); )
         {
+            // Fetch range elements in a well-defined order
+            const int low = *range++;
+            const int high = *range++;
 
             get();
-            if (JSON_LIKELY(*range <= current and current <= *(++range)))
+            if (JSON_LIKELY(low <= current and current <= high))
@stale
Copy link

stale bot commented Aug 16, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated label Aug 16, 2019
@stale stale bot closed this as completed Aug 23, 2019
@nlohmann nlohmann reopened this Oct 23, 2019
@stale stale bot removed the state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated label Oct 23, 2019
@nlohmann
Copy link
Owner

nlohmann commented Nov 5, 2019

I cannot reproduce this warning.

@nlohmann
Copy link
Owner

nlohmann commented Nov 5, 2019

@dmax621 Is the issue still visible to you?

@dmax621
Copy link
Author

dmax621 commented Nov 6, 2019

As I mentioned in my original report, I couldn't reproduce this on my development system in the first place. The original message appeared in a log from our CI system, and I wasn't entirely certain which node the tests had been run on, or what the specific node configuration was. I tend to regard it as most likely a compiler bug, possibly due to clang not quite treating and the same as &&, or some misbehaviour of __builtin_expect(x) when x has side-effects (there was a clang bug of this kind at some point). But I'd be inclined to change the source to the fix shown anyway, as a matter of preferring a policy of not using expressions with side-effects as arguments to macros, just in case the macro doesn't handle them correctly.

@nlohmann
Copy link
Owner

nlohmann commented Nov 6, 2019

Thanks for the response!

@nlohmann nlohmann closed this as completed Nov 6, 2019
@mprygh
Copy link

mprygh commented Dec 20, 2019

I am seeing this same warning.
It does not appear to matter, just seems to be a warning.
clang version 3.8.0-2ubuntu4 (tags/RELEASE_380/final)
From json-3.7.3.tar.gz
I am using the single_include.
Ubuntu16
CXXFLAGS=-g -std=c++14 -Wall -pedantic

./json.hpp:7336:71: warning: unsequenced modification and access to 'range'
[-Wunsequenced]
if (JSON_HEDLEY_LIKELY(*range <= current and current <= *(++range)))
~~~~~ ^
./json.hpp:1117:58: note: expanded from macro 'JSON_HEDLEY_LIKELY'

define JSON_HEDLEY_LIKELY(expr) __builtin_expect(!!(expr), 1)

                                                     ^~~~

@mprygh
Copy link

mprygh commented Dec 20, 2019

I upgraded to clang version 9, and the warning went away.

FYI: clang 3.8 is the default for Ubuntu 16.

@mprygh
Copy link

mprygh commented Dec 20, 2019

Also FYI:
clang version 9, with -std=c++17, spews a ton of errors.
gcc -std=c++17 works fine.

@nlohmann
Copy link
Owner

Can you please share the warnings/errors for Clang9 with C++17?

@mprygh
Copy link

mprygh commented Jan 7, 2020

(Sorry, was on vacation)

I could not reproduce the errors at first. Even on original clang 3.8.
But I have changed the code, in particular some includes.

Some interesting findings!
It turns out:, if you do a "using namespace std" BEFORE you include your library, it fails.
If I do the using after your library it is fine.
That is on clang 3.8. Error was same as above.

I still cannot reproduce the version 9, c++17 errors.
But I would guess it is something similar. Might be boost or some other thing.
Sorry I did not save those.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants