-
-
Notifications
You must be signed in to change notification settings - Fork 7.2k
Fix 'const' qualifier on bool& has no effect #3678
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
Conversation
|
Do we have a chance to add a regression test? |
|
Not without external dependencies. Edit: The warning isn't new, just poorly searchable. |
|
This does not work, unfortunately. |
|
Oh, that's because I made at least one mistake. |
3cc06f9 to
b69e4ee
Compare
|
Still no |
|
I could try flipping those checks, hoping the compiler short-circuits and doesn't get to the Edit: No. It would still trigger on the function prototype. I'll try something else. |
|
And why const_reference fix proposed #3677 would not work? UPD: I see, almost every CI build is broken. |
|
In the end, it would still conflict with the existing overload. |
|
One more try. I'm grasping at straws with this one. If it doesn't work, we may just have to suppress the warning for that function. |
|
It looks like the latest version will work for us. Thanks, @falbrechtskirchinger! |
|
Probably not a bad idea to also do the change from 3677 and use |
|
No, that doesn't work. Assuming my last fix attempt works, I was going to give Edit: Missed @georgthegreat's reply. Good to hear. I will make the change then, |
f5188df to
982edd9
Compare
|
I decided against dealing with |
|
And it turns out, that libc++ does use a different type for Guess I'll have to add it after all. |
9bcdfb2 to
3dac72e
Compare
Thanks, @georgthegreat, for pointing out this issue.
3dac72e to
e90dc44
Compare
|
Let me walk you through the
This covers all tested and reported STLs. |
falbrechtskirchinger
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
nlohmann
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
Disable the
to_jsonoverload forstd::vector<bool>::referencewhenstd::vector<bool>::referenceis the same asbasic_json::boolean_t&.Thanks, @georgthegreat, for pointing out this issue.
Closes #3677.