Bug 213773 - lang/clang38: Compilation produces an error on 10.3 and succeeds on 11 on the same port
Summary: lang/clang38: Compilation produces an error on 10.3 and succeeds on 11 on the...
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Brooks Davis
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-10-25 16:13 UTC by Yuri Victorovich
Modified: 2016-12-10 08:50 UTC (History)
2 users (show)

See Also:
bugzilla: maintainer-feedback? (brooks)


Attachments
Headers from the base that were used (3.19 KB, text/plain)
2016-10-25 21:14 UTC, Yuri Victorovich
no flags Details
Full error message (4.67 KB, text/plain)
2016-10-25 22:21 UTC, Yuri Victorovich
no flags Details
Workaround for bug 213773 (libc++ PR 22468) (2.43 KB, patch)
2016-10-31 16:47 UTC, Dimitry Andric
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yuri Victorovich freebsd_committer 2016-10-25 16:13:41 UTC
Getting the error while trying to update graphics/aseprite (https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=213756).

Same clang-38 compiler, same port directory, but the outcome is different.

On 10.3 this error is produced:
> /usr/ports/graphics/aseprite/work/aseprite-1.1.9/src/observable/obs/slot.h:51:17: error: no matching constructor for initialization of 'std::function<void ()>'
>   slot(F&& f) : f(std::forward<F>(f)) { }

On 11.0 build succeeds with both base and port clang-38.

The same compiler version should work the same way across OS versions, imo.
Comment 1 Brooks Davis freebsd_committer 2016-10-25 17:27:13 UTC
This is almost certainly a difference in the system header files.
Comment 2 Yuri Victorovich freebsd_committer 2016-10-25 21:14:49 UTC
Created attachment 176161 [details]
Headers from the base that were used
Comment 3 Dimitry Andric freebsd_committer 2016-10-25 21:28:15 UTC
It's not possible to support this.  The libc++ in 10.3 is approximately from the same time as clang 3.4, which is now almost 2 years old, and it is not likely to be updated very quickly.  You cannot just mix and match any versions of compilers and C++ libraries.
Comment 4 Yuri Victorovich freebsd_committer 2016-10-25 21:38:10 UTC
I think many C++ ports will be eventually broken by this on the older OS versions.

Do you think it would have been better if compatible libc++ was included into clang-NN?
Comment 5 Dimitry Andric freebsd_committer 2016-10-25 21:51:09 UTC
(In reply to yuri from comment #4)
> I think many C++ ports will be eventually broken by this on the older OS
> versions.

I think you are the only person attempting to compile ports with non-base clang.  If you use base clang, there is no issue.


> Do you think it would have been better if compatible libc++ was included
> into clang-NN?

Please, let's not repeat the gcc ports fiasco.
Comment 6 Yuri Victorovich freebsd_committer 2016-10-25 22:08:28 UTC
> I think you are the only person attempting to compile ports with non-base clang.  > If you use base clang, there is no issue.

No, this port fails with the default clang on 10.3 the same way.
Comment 7 Yuri Victorovich freebsd_committer 2016-10-25 22:10:56 UTC
Actually, USES=compiler:c++14-lang switches to clang-36 and it fails the same way. c++14 was added/can be added at any time.
Comment 8 Yuri Victorovich freebsd_committer 2016-10-25 22:12:03 UTC
So this is a bug in clang on 10.3 that it can't compile some projects.
Comment 9 Dimitry Andric freebsd_committer 2016-10-25 22:13:10 UTC
(In reply to yuri from comment #6)
> No, this port fails with the default clang on 10.3 the same way.

Isn't it a problem in the port then?  The error message you showed is very generic, and can happen because of many different reasons.  At least paste the full error, so it is clearer where the error originated.
Comment 10 Yuri Victorovich freebsd_committer 2016-10-25 22:21:24 UTC
Created attachment 176163 [details]
Full error message

Attaching the full error message.

The upstream author says that it builds fine everywhere, including on FreeBSD 11. So C++ code should be considered correct. But 10.3 base fails to build it. The outdated c++ library causing build failures in some valid projects constitutes a bug, IMO.
Comment 11 Dimitry Andric freebsd_committer 2016-10-26 18:06:36 UTC
(In reply to yuri from comment #10)
> The upstream author says that it builds fine everywhere, including on
> FreeBSD 11. So C++ code should be considered correct. But 10.3 base fails to
> build it. The outdated c++ library causing build failures in some valid
> projects constitutes a bug, IMO.

Interesting, I've just built the port on 10-STABLE r303552, with CC=clang38 CXX=clang++38 (from the llvm38-3.8.1 port), and it compiled successfully for me.

Are you testing with a newer version of the port? Or configuring it differently, somehow?
Comment 12 Yuri Victorovich freebsd_committer 2016-10-26 18:57:04 UTC
I am talking about the newest version, with the patch from bug#213756 .
Comment 13 Yuri Victorovich freebsd_committer 2016-10-31 05:16:35 UTC
I think this should be reopened. I can't update graphics/aseprite (bug#213756) because it fails on 10. I can label it broken in 10, but what is the reason? "Broken C++ support on 10". But it isn't supposed to be broken.
Comment 14 Dimitry Andric freebsd_committer 2016-10-31 12:00:38 UTC
Sure, let's reopen.  I found the upstream bug, which is <https://llvm.org/bugs/show_bug.cgi?id=22468> ("std::function<void()> does not accept non-void-returning functions").

I think the fix can be backported to stable/10, but this won't fix it for any existing 10.x releases.  So it is still interesting to see whether we can work around it in the port itself.  I am currently looking at aseprite's upstream history to see where the problem got introduced.
Comment 15 Dimitry Andric freebsd_committer 2016-10-31 16:47:07 UTC
Created attachment 176348 [details]
Workaround for bug 213773 (libc++ PR 22468)

Okay, here is at least a possible workaround for aseprite itself.  The upstream libc++ bug is really about binding a std::function<void()> with a function that doesn't return void.

In case of aseprite, this is about bool AppMenus::rebuildRecentList(), which is bound in the AppMenus constructor.  This used to work before upstream commit 6377b55 [1], where they replaced base::Signal/Observable* with obs::signal/observable.

So changing rebuildRecentList()'s return type to void fixes the build with older libc++ headers, by avoiding the bool -> void conversion.  It does not look like anything was actually interested in the bool return value anyway.

Maybe you can consider this fix, which will work even for 10.1, 10.2 and 10.3 releases?

Note that I will commit the upstream libc++ fix for std::function anyway, but I can't apply it retroactively to older releases.

[1] https://github.com/aseprite/aseprite/commit/6377b550e36dd1fb1147f91220a2cded8e7fd334
Comment 16 commit-hook freebsd_committer 2016-10-31 18:38:32 UTC
A commit references this bug:

Author: dim
Date: Mon Oct 31 18:37:45 UTC 2016
New revision: 308143
URL: https://svnweb.freebsd.org/changeset/base/308143

Log:
  Pull in r228705 from upstream libc++ trunk (by Eric Fiselier):

    [libcxx] Fix PR 22468 - std::function<void()> does not accept
    non-void-returning functions

    Summary:
    The bug can be found here: https://llvm.org/bugs/show_bug.cgi?id=22468

    `__invoke_void_return_wrapper` is needed to properly handle calling a
    function that returns a value but where the std::function return type
    is void. Without this '-Wsystem-headers' will cause
    `function::operator()(...)` to not compile.

    Reviewers: eugenis, K-ballo, mclow.lists

    Reviewed By: mclow.lists

    Subscribers: cfe-commits

    Differential Revision: https://reviews.llvm.org/D7444

  This should allow newer versions of the graphics/aseprite port to
  compile without modification.

  Direct commit to stable/10, since stable/11 and head already have this
  change.

  Reported by:	yuri@rawbw.com
  PR:		213773

Changes:
  stable/10/contrib/libc++/include/__functional_03
  stable/10/contrib/libc++/include/__functional_base
  stable/10/contrib/libc++/include/__functional_base_03
  stable/10/contrib/libc++/include/functional
Comment 17 commit-hook freebsd_committer 2016-10-31 18:45:35 UTC
A commit references this bug:

Author: dim
Date: Mon Oct 31 18:45:02 UTC 2016
New revision: 308146
URL: https://svnweb.freebsd.org/changeset/base/308146

Log:
  Merge r308143 from stable/10:

  Pull in r228705 from upstream libc++ trunk (by Eric Fiselier):

    [libcxx] Fix PR 22468 - std::function<void()> does not accept
    non-void-returning functions

    Summary:
    The bug can be found here: https://llvm.org/bugs/show_bug.cgi?id=22468

    `__invoke_void_return_wrapper` is needed to properly handle calling a
    function that returns a value but where the std::function return type
    is void. Without this '-Wsystem-headers' will cause
    `function::operator()(...)` to not compile.

    Reviewers: eugenis, K-ballo, mclow.lists

    Reviewed By: mclow.lists

    Subscribers: cfe-commits

    Differential Revision: https://reviews.llvm.org/D7444

  This should allow newer versions of the graphics/aseprite port to
  compile without modification.

  Direct commit to stable/10, since stable/11 and head already have this
  change.

  Reported by:	yuri@rawbw.com
  PR:		213773

Changes:
_U  stable/9/
_U  stable/9/contrib/
_U  stable/9/contrib/libc++/
  stable/9/contrib/libc++/include/__functional_03
  stable/9/contrib/libc++/include/__functional_base
  stable/9/contrib/libc++/include/__functional_base_03
  stable/9/contrib/libc++/include/functional
Comment 18 Yuri Victorovich freebsd_committer 2016-11-01 06:03:44 UTC
Thanks Dimitry and Jan,

I integrated the patch into the graphics/aseprite update (bug#213756).
Comment 19 commit-hook freebsd_committer 2016-11-01 15:49:21 UTC
A commit references this bug:

Author: jbeich
Date: Tue Nov  1 15:48:29 UTC 2016
New revision: 425055
URL: https://svnweb.freebsd.org/changeset/ports/425055

Log:
  graphics/aseprite: update to 1.1.9 [1]

  - Apply a workaround for old libc++ on 10.x systems [2]

  Changes:	https://www.aseprite.org/release-notes/#aseprite-v1-1
  PR:		213756 [1]
  PR:		213773 [2]
  Submitted by:	yuri@rawbw.com [1]
  Submitted by:	dim [2]

Changes:
  head/graphics/aseprite/Makefile
  head/graphics/aseprite/distinfo
  head/graphics/aseprite/files/patch-src_app_app__menus.cpp
  head/graphics/aseprite/files/patch-src_app_app__menus.h
  head/graphics/aseprite/files/patch-src_base_file__handle.cpp
  head/graphics/aseprite/files/patch-src_base_fs__unix.h
  head/graphics/aseprite/pkg-plist