Bug 220189

Summary: x11-toolkits/mygui: stop redefining nullptr
Product: Ports & Packages Reporter: Dimitry Andric <dim>
Component: Individual Port(s)Assignee: Dmitry Marakasov <amdmi3>
Status: Closed FIXED    
Severity: Affects Some People CC: amdmi3, tobik, val
Priority: --- Flags: bugzilla: maintainer-feedback? (amdmi3)
Version: Latest   
Hardware: Any   
OS: Any   
Bug Depends on:    
Bug Blocks: 219139    
Attachments:
Description Flags
Stop redefining nullptr
none
mygui.diff
tobik: maintainer-approval? (amdmi3)
mygui-git.patch tobik: maintainer-approval? (amdmi3)

Description Dimitry Andric freebsd_committer freebsd_triage 2017-06-21 17:14:31 UTC 
Created attachment 183680 [details]
Stop redefining nullptr

During an exp-run for the projects/clang500-import branch (bug 219139), it turned out that x11-toolkits/mygui does not build against libc++ 5.0.0 [1]:

In file included from /wrkdirs/usr/ports/x11-toolkits/mygui/work/mygui-MyGUI3.2.2/MyGUIEngine/src/MyGUI_ActionController.cpp:7:
In file included from /wrkdirs/usr/ports/x11-toolkits/mygui/work/mygui-MyGUI3.2.2/MyGUIEngine/include/MyGUI_Precompiled.h:11:
In file included from /wrkdirs/usr/ports/x11-toolkits/mygui/work/mygui-MyGUI3.2.2/MyGUIEngine/include/MyGUI_Common.h:12:
In file included from /usr/include/c++/v1/string:470:
In file included from /usr/include/c++/v1/string_view:170:
In file included from /usr/include/c++/v1/__string:56:
In file included from /usr/include/c++/v1/algorithm:640:
/usr/include/c++/v1/memory:2089:9: error: cannot initialize a member subobject of type 'std::__1::basic_string<char> *' with an rvalue of type 'int'
      : __value_(_VSTD::forward<_Up>(__u)){};
        ^        ~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/c++/v1/memory:2174:9: note: in instantiation of function template specialization 'std::__1::__compressed_pair_elem<std::__1::basic_string<char> *, 0, false>::__compressed_pair_elem<int, void>' requested here
      : _Base1(std::forward<_Tp>(__t)), _Base2() {}
        ^
/usr/include/c++/v1/vector:428:7: note: in instantiation of function template specialization 'std::__1::__compressed_pair<std::__1::basic_string<char> *, std::__1::allocator<std::__1::basic_string<char> > >::__compressed_pair<int, true>' requested here
      __end_cap_(nullptr)
      ^
/usr/include/c++/v1/vector:478:5: note: in instantiation of member function 'std::__1::__vector_base<std::__1::basic_string<char>, std::__1::allocator<std::__1::basic_string<char> > >::__vector_base' requested here
    vector() _NOEXCEPT_(is_nothrow_default_constructible<allocator_type>::value)
    ^
/wrkdirs/usr/ports/x11-toolkits/mygui/work/mygui-MyGUI3.2.2/MyGUIEngine/include/MyGUI_StringUtility.h:293:29: note: in instantiation of member function 'std::__1::vector<std::__1::basic_string<char>, std::__1::allocator<std::__1::basic_string<char> > >::vector' requested here
                        std::vector<std::string> result;
                                                 ^

These errors are because mygui by default redefines 'nullptr' to 0, even while it is being compiled as C++11.  This is not supported.  Luckily it supports a define MYGUI_DONT_REPLACE_NULLPTR to stop doing that.

(I don't know why the port is compiled with C++11 though, it does not seem to be necessary.)

[1] http://package18.nyi.freebsd.org/data/headamd64PR219139-default/2017-05-22_13h01m42s/logs/errors/mygui-3.2.2_2.log
Comment 1 Dmitry Marakasov freebsd_committer freebsd_triage 2017-06-26 19:33:28 UTC 
I don't think that's correct fix since the client code will remain broken. I think it can be solved by adding a __cplusplus check instead. I'll set up clang50 jail and test.
Comment 2 Dimitry Andric freebsd_committer freebsd_triage 2017-06-26 20:21:00 UTC 
The problem is that the code is fundamentally broken, as the char type for basic_string *must* be a POD type.  It's just that newer libc++ enforces this with a static assertion.

I tried making this work in before-C++11 mode, but it is next to impossible. You'd have to remove the constructors and destructors, the assignment operator, and make the (only) private member public.
Comment 3 Dimitry Andric freebsd_committer freebsd_triage 2017-06-26 20:22:18 UTC 
(In reply to Dimitry Andric from comment #2)
> The problem is that the code is fundamentally broken, as the char type for
> basic_string *must* be a POD type.  It's just that newer libc++ enforces
> this with a static assertion.
> 
> I tried making this work in before-C++11 mode, but it is next to impossible.
> You'd have to remove the constructors and destructors, the assignment
> operator, and make the (only) private member public.

Scratch that, I was thinking about a completely different issue with another port. Sorry for the noise. :)
Comment 4 Val Packett 2017-09-08 19:16:38 UTC 
Current master of mygui is much better, seems like they don't do that anymore, and they actually do use C++11 now.

I've sent them a tiny patch https://github.com/MyGUI/mygui/pull/129 — only had to replace one occurrence of NULL instead of 0.

Release OpenMW doesn't build with master mygui, but master OpenMW does.
Comment 5 Tobias Kortkamp freebsd_committer freebsd_triage 2017-11-29 11:46:42 UTC 
Can we do something about this?

For OpenMW I'm currently working around it by using dim@'s patch and
adding games/openmw/Makefile.local with

CXXFLAGS+=	-DMYGUI_DONT_REPLACE_NULLPTR
Comment 6 Val Packett 2017-11-29 15:36:12 UTC 
(In reply to Tobias Kortkamp from comment #5)
We can switch both mygui and openmw to git master :)

https://github.com/myfreeweb/freebsd-ports-dank/commit/76b2e08afa11ca5c3368e1c5860baf9163523c80
Comment 7 Val Packett 2017-11-30 19:16:43 UTC 
ooh, OpenMW 0.43 came out. Yeah I think now we can just update mygui to git master
Comment 8 Tobias Kortkamp freebsd_committer freebsd_triage 2017-12-06 16:48:49 UTC 
Created attachment 188589 [details]
mygui.diff

Since defining MYGUI_DONT_REPLACE_NULLPTR fixes the problem and since I have
no idea what the right __cplusplus check to add would be: There is
only one place in the code where it's even used, how about just deleting
the section entirely.  That way client code will not remain broken like
with the initial patch.
Comment 9 Val Packett 2017-12-06 18:59:55 UTC 
Created attachment 188596 [details]
mygui-git.patch

mygui git master requires no patches, just re-tested with OpenMW 0.43.0, works perfectly. Here's the simple patch for upgrading to git master.
Comment 10 Tobias Kortkamp freebsd_committer freebsd_triage 2017-12-06 19:05:35 UTC 
Comment on attachment 188596 [details]
mygui-git.patch

I'm fine with either fix. Yours might be better. I'm hoping that Dmitry can make a choice here.
Comment 11 commit-hook freebsd_committer freebsd_triage 2017-12-19 20:19:24 UTC 
A commit references this bug:

Author: amdmi3
Date: Tue Dec 19 20:18:24 UTC 2017
New revision: 456766
URL: https://svnweb.freebsd.org/changeset/ports/456766

Log:
  - Update to latest git, fixes c++11 problems

  PR:		220189

Changes:
  head/x11-toolkits/mygui/Makefile
  head/x11-toolkits/mygui/distinfo
  head/x11-toolkits/mygui/pkg-plist