-
-
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
xwidgets doesn't compile with version >3.10.3 #3602
Comments
I can reproduce the issue and trace it back to the same |
@falbrechtskirchinger is this a band-aid for |
For |
@falbrechtskirchinger also if you don't mind, I'd like to understand what's going on in here and why it's causing the error. I have no affiliation with either of the libraries and it took me a while to figure out what I have so far. |
The issue is this template <template <class> class B, class... P>
template <class... A>
inline xmaterialize<B, P...>::xmaterialize(A&&... args)
: xmaterialize(false, std::forward<A>(args)...)
{
} The band-aid consists of re-adding the check for a I've started a discussion (#3603) to try and fix issues like this one in 4.0. |
@falbrechtskirchinger thank you, I understand now. As a workaround, what if I replaced |
... or I suppose, I can roll my own version with your fresh-off-the-press patch, since you are so fast. 🥇 🥇 🥇 |
You could try calling Grabbing my patch is likely going to be more fruitful. |
OK I will try both. Thank you for such a fast response. 😃 |
Well, unless I'm using the wrong header, it's not actually working. :-( In that case, you'll have to wait until tomorrow. |
@falbrechtskirchinger yep, I can confirm that neither of the methods worked for me. |
@falbrechtskirchinger FWIW if I patch |
Commit 261cc4e is the other culprit that's causing failure on the diff --git a/include/nlohmann/detail/conversions/from_json.hpp b/include/nlohmann/detail/conversions/from_json.hpp
index 207d3e3024..cc06f198b4 100644
--- a/include/nlohmann/detail/conversions/from_json.hpp
+++ b/include/nlohmann/detail/conversions/from_json.hpp
@@ -105,13 +105,12 @@ void from_json(const BasicJsonType& j, typename BasicJsonType::string_t& s)
}
template <
- typename BasicJsonType, typename ConstructibleStringType,
+ typename BasicJsonType, typename StringType,
enable_if_t <
- is_constructible_string_type<BasicJsonType, ConstructibleStringType>::value&&
- !std::is_same<typename BasicJsonType::string_t,
- ConstructibleStringType>::value,
- int > = 0 >
-void from_json(const BasicJsonType& j, ConstructibleStringType& s)
+ std::is_assignable<StringType&, const typename BasicJsonType::string_t>::value
+ && !std::is_same<typename BasicJsonType::string_t, StringType>::value
+ && !is_json_ref<StringType>::value, int > = 0 >
+void from_json(const BasicJsonType& j, StringType& s)
{
if (JSON_HEDLEY_UNLIKELY(!j.is_string()))
{ |
Thanks for confirming. I already suspected that it might very well work with 3.10.5. Several issues caused by 0e694b4 were addressed in the 3.11.0 development cycle, and there were bound to be some mistakes. |
Adding the same |
Thanks @falbrechtskirchinger. I ended up throwing a patched version of Once 3.11 is out, I will undo all that. Thank you for your help. 🎩 🎩 🎩 |
Is this going to break other types that are actually string types and are now working but don't have a |
Edit: Nope, nonsense. The change was made for 3.10.4. We'd be going back to the 3.10.3 days. I don't really like it, but a proper solution requires a bit more thought. Basically, we need some trait that users are allowed to specialize for custom types. Edit: Maybe there's a better workaround? Related discussion post: #3603 |
Description
The xwidgets project, which uses this library stopped compiling after version 3.10.3. Specifically, commit 0e694b4 is responsible for the problem.
Even more specifically, redefinition of
is_constructible_string_type
from this:json/include/nlohmann/detail/meta/type_traits.hpp
Lines 331 to 348 in aa0e847
to this:
json/include/nlohmann/detail/meta/type_traits.hpp
Lines 319 to 325 in 0e694b4
is what's causing the error.
The exact part of
xwidgets
where the error happens is here:I know
xwidgets
says that they are only compatible withjson
version <3.10, but most up-to-date linux distros only supply 3.10.5. Plus, everything used to work up until the above commit.Reproduction steps
Install
xeus-dev
andxproperty-dev
dependencies from my ppa:ppa-verse/cling repo.Download and unpack: xwidgets_0.26.1.orig.tar.gz
Download (remove .txt) and unpack inside
xwidgets-0.26.1
above: xwidgets_0.26.1-0ppa2.debian.tar.xz.txtExecute
dpkg-buildpackage -uc -us -b
Expected vs. actual results
Expected: Successful compilation and creation of
xwidgets
debian package.Actual: Failed compilation. (See error messages below and attached build log).
buildlog_ubuntu-jammy-amd64.xwidgets_0.26.1-0ppa2_ubuntu22.04_BUILDING.zip
Minimal code example
No response
Error messages
Compiler and operating system
Ubuntu 22.04, gcc 11.2.0
Library version
v3.10.3 + 0e694, v3.10.5, develop
Validation
develop
branch is used.The text was updated successfully, but these errors were encountered: