Page MenuHomeFreeBSD

shells/fish: fix the build with Clang-6.0.0
ClosedPublic

Authored by asomers on Jan 26 2018, 4:22 PM.

Details

Summary

shells/fish: fix the build with Clang-6.0.0

Clang-6.0.0 on FreeBSD 12 doesn't define __cpp_lib_make_unique even though it
should. Patch fish to always assume that symbol is defined on FreeBSD 12.

Test Plan

Poudriere on 10.4 and 11.1. Manual build and test on 12.0.

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

I'm not entirely sure that this macro is defined by any C++ standard, though there appears to be a "recommendation" note that refers to it. It would be easier to just test for the C++ version instead, I think. But if #if 1 also works, I'm fine with it.

I don't see what the OSVERSION check is for? Since libc++ never defined __cpp_lib_make_unique, this can't have worked on earlier versions either. Why is this suddenly needed now, because we're defaulting to gnu++14? In that case you could also just crank down the standard to e.g. -std=gnu++11 or even lower.

In D14058#295310, @dim wrote:

I don't see what the OSVERSION check is for? Since libc++ never defined __cpp_lib_make_unique, this can't have worked on earlier versions either. Why is this suddenly needed now, because we're defaulting to gnu++14? In that case you could also just crank down the standard to e.g. -std=gnu++11 or even lower.

The problem is that while __cpp_lib_make_unique isn't defined, std::make_unique is. That causes error: call to 'make_unique' is ambiguous errors, because fish defines its own make_unique if the standard library doesn't provide one. I'm guessing that earlier versions of Clang didn't provide make_unique, at least not with our default --std option. Does libc++ have a different feature test macro for "supports all C++14 features" or something like that?

Using -std=c+=11 works. However, it feels wrong. We have a C++14 compiler; we should be able to use it.

In D14058#295310, @dim wrote:

I don't see what the OSVERSION check is for? Since libc++ never defined __cpp_lib_make_unique, this can't have worked on earlier versions either. Why is this suddenly needed now, because we're defaulting to gnu++14? In that case you could also just crank down the standard to e.g. -std=gnu++11 or even lower.

The problem is that while __cpp_lib_make_unique isn't defined, std::make_unique is. That causes error: call to 'make_unique' is ambiguous errors, because fish defines its own make_unique if the standard library doesn't provide one. I'm guessing that earlier versions of Clang didn't provide make_unique, at least not with our default --std option. Does libc++ have a different feature test macro for "supports all C++14 features" or something like that?

The compiler itself supplies the built-in macro __cplusplus for that. If the value is 201402L, we are compiling for -std=c++14 or later, and in that case std::make_unique will be available.

E.g. just #if __cplusplus >= 201402L is the correct conditional.

key off of __cplusplus instead of OSVERSION

jbeich added a subscriber: jbeich.

Can you land it?

This revision is now accepted and ready to land.Jan 27 2018, 1:23 PM
This revision was automatically updated to reflect the committed changes.