-
-
Notifications
You must be signed in to change notification settings - Fork 6.9k
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
Fix 'const' qualifier on bool& has no effect #3678
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. |
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.
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_json
overload forstd::vector<bool>::reference
whenstd::vector<bool>::reference
is the same asbasic_json::boolean_t&
.Thanks, @georgthegreat, for pointing out this issue.
Closes #3677.