-
-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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 and add test's for SFINAE problem #1741
Conversation
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.
Could you please add the code from #1727 (comment) as regression test? Otherwise, the code looks find and I am looking forward to merging! Thanks!
The Ci broke for Xcode 8.3 because of hombrew 😢 |
I restarted the job and it succeeded now. |
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.
Thanks a lot! |
🔖 Release itemThis issue/PR will be part of the next release of the library. This template helps preparing the release notes. Type
Description
|
This Pull Requests fixes #1727
And adds a test that would have not worked before to avoid regressions 😄
This was simply due to a pretty typical SFINAE bug. Since the function signature was
When a rvalue is passed (say int for simplicity reasons) you want this to be transformed into
So far so good, trouble begins when given a lvalue, since the end result should be
This can only be achieved by the special rule of type deduction in c++ where:
&
+&&
->&
In conclussion:
KeyT && key
->int & && key
->int & key
More specific to the issue
KeyT
was being deducted tojson::json_pointer &
which is clearly not the same asjson::json_pointer
so our lovely friend thestd::enable_if
wasn't doing his job properly.Decaying the type should fix this 😄
God... metaprogramming is hard...
Pull request checklist
Read the Contribution Guidelines for detailed information.
include/nlohmann
directory, runmake amalgamate
to create the single-header filesingle_include/nlohmann/json.hpp
. The whole process is described here.Please don't
#ifdef
s or other means.