This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Remove _LIBCPP_HAS_NO_LONG_LONG in favour of using_if_exists
ClosedPublic

Authored by ldionne on Aug 24 2021, 7:33 AM.

Details

Summary

_LIBCPP_HAS_NO_LONG_LONG was only defined on FreeBSD. Instead, use the
using_if_exists attribute to skip over declarations that are not available
on the base system. Note that there's an annoying limitation that we can't
conditionally define a function based on whether the base system provides
a function, so for example we still need preprocessor logic to define the
abs() and div() overloads.

Diff Detail

Event Timeline

ldionne created this revision.Aug 24 2021, 7:33 AM
ldionne requested review of this revision.Aug 24 2021, 7:33 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 24 2021, 7:33 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
ldionne added inline comments.Aug 24 2021, 7:35 AM
libcxx/include/stdlib.h
151

@emaste @dim

What does !defined(__LONG_LONG_SUPPORTED) mean exactly? It doesn't mean that the long long type isn't provided by the compiler, because there are several other uses of long long in the library that are not guarded by that check. Is it only about the C Standard Library on FreeBSD not providing ::lldiv? What about __builtin_llabs() -- I would that if long long is a valid type, the compiler does implement __builtin_llabs as used above and we don't need to guard the above use of it?

dim added inline comments.Aug 24 2021, 8:16 AM
libcxx/include/stdlib.h
151

@emaste @dim

What does !defined(__LONG_LONG_SUPPORTED) mean exactly? It doesn't mean that the long long type isn't provided by the compiler, because there are several other uses of long long in the library that are not guarded by that check. Is it only about the C Standard Library on FreeBSD not providing ::lldiv? What about __builtin_llabs() -- I would that if long long is a valid type, the compiler does implement __builtin_llabs as used above and we don't need to guard the above use of it?

It's a historic thing in our https://github.com/freebsd/freebsd-src/blob/main/include/stdlib.h#L126 and various other standard headers:

/*
 * Functions added in C99 which we make conditionally available in the
 * BSD^C89 namespace if the compiler supports `long long'.
 * The #if test is more complicated than it ought to be because
 * __BSD_VISIBLE implies __ISO_C_VISIBLE == 1999 *even if* `long long'
 * is not supported in the compilation environment (which therefore means
 * that it can't really be ISO C99).
 *
 * (The only other extension made by C99 in thie header is _Exit().)
 */

The definition of __LONG_LONG_SUPPORTED comes from https://github.com/freebsd/freebsd-src/blob/main/sys/sys/cdefs.h#L391 :

#if (defined(__GNUC__) && __GNUC__ >= 2) && !defined(__STRICT_ANSI__) || __STDC_VERSION__ >= 199901
#define	__LONG_LONG_SUPPORTED
#endif

/* C++11 exposes a load of C99 stuff */
#if defined(__cplusplus) && __cplusplus >= 201103L
#define	__LONG_LONG_SUPPORTED
#ifndef	__STDC_LIMIT_MACROS
#define	__STDC_LIMIT_MACROS
#endif
#ifndef	__STDC_CONSTANT_MACROS
#define	__STDC_CONSTANT_MACROS
#endif
#endif

Ergo, C++ standards before C++11 have __LONG_LONG_SUPPORTED undefined. Unfortunately there are still quite a few programs requiring -std=c++98 (or more usually -std=gnu++98)...

ldionne added inline comments.Aug 24 2021, 9:06 AM
libcxx/include/stdlib.h
151

Okay, thanks a lot for the background. In that case, I think we can even remove the guard from around the use of __builtin_llabs and this patch should be non-breaking.

Would it be possible to add build bots for FreeBSD so we can test changes like those automatically?

ldionne updated this revision to Diff 368365.Aug 24 2021, 9:07 AM

Remove #if around the use of __builtin_llabs

Would it be possible to add build bots for FreeBSD so we can test changes like those automatically?

Yes, this is on my TODO list. I cannot get to it this week but expect to work on it next week (Aug 30 2021-)

Would it be possible to add build bots for FreeBSD so we can test changes like those automatically?

Yes, this is on my TODO list. I cannot get to it this week but expect to work on it next week (Aug 30 2021-)

Awesome, thanks! In the meantime, does this patch look good to you? I don't think it should break anything if you're compiling with a Clang that supports using_if_exists.

Gentle ping.

dim added a comment.Sep 2 2021, 2:48 AM

Hmm I have applied this on top of our existing libc++ headers, but if I compile a simple program using e.g. -std=c++98 I immediately get errors due to missing declarations:

% cat helloworld.cpp
#include <iostream>

int main(void)
{
  std::cout << "Hello World!" << std::endl;
  return 0;
}

% clang++ -std=c++98 -c helloworld.cpp
In file included from helloworld.cpp:1:
In file included from /usr/include/c++/v1/iostream:37:
In file included from /usr/include/c++/v1/ios:215:
In file included from /usr/include/c++/v1/__locale:15:
In file included from /usr/include/c++/v1/string:511:
In file included from /usr/include/c++/v1/string_view:179:
In file included from /usr/include/c++/v1/__string:57:
In file included from /usr/include/c++/v1/algorithm:653:
In file included from /usr/include/c++/v1/memory:670:
In file included from /usr/include/c++/v1/typeinfo:61:
In file included from /usr/include/c++/v1/exception:83:
/usr/include/c++/v1/cstdlib:102:9: error: no member named 'lldiv_t' in the global namespace; did you mean 'ldiv_t'?
using ::lldiv_t _LIBCPP_USING_IF_EXISTS;
      ~~^
/usr/include/stdlib.h:71:3: note: 'ldiv_t' declared here
} ldiv_t;
  ^
In file included from helloworld.cpp:1:
In file included from /usr/include/c++/v1/iostream:37:
In file included from /usr/include/c++/v1/ios:215:
In file included from /usr/include/c++/v1/__locale:15:
In file included from /usr/include/c++/v1/string:511:
In file included from /usr/include/c++/v1/string_view:179:
In file included from /usr/include/c++/v1/__string:57:
In file included from /usr/include/c++/v1/algorithm:653:
In file included from /usr/include/c++/v1/memory:670:
In file included from /usr/include/c++/v1/typeinfo:61:
In file included from /usr/include/c++/v1/exception:83:
/usr/include/c++/v1/cstdlib:106:9: error: no member named 'atoll' in the global namespace; did you mean 'atol'?
using ::atoll _LIBCPP_USING_IF_EXISTS;
      ~~^
/usr/include/stdlib.h:95:7: note: 'atol' declared here
long     atol(const char *);
         ^
In file included from helloworld.cpp:1:
In file included from /usr/include/c++/v1/iostream:37:
In file included from /usr/include/c++/v1/ios:215:
In file included from /usr/include/c++/v1/__locale:15:
In file included from /usr/include/c++/v1/string:511:
In file included from /usr/include/c++/v1/string_view:179:
In file included from /usr/include/c++/v1/__string:57:
In file included from /usr/include/c++/v1/algorithm:653:
In file included from /usr/include/c++/v1/memory:670:
In file included from /usr/include/c++/v1/typeinfo:61:
In file included from /usr/include/c++/v1/exception:83:
/usr/include/c++/v1/cstdlib:111:9: error: no member named 'strtoll' in the global namespace
using ::strtoll _LIBCPP_USING_IF_EXISTS;
      ~~^
/usr/include/c++/v1/cstdlib:113:9: error: no member named 'strtoull' in the global namespace; did you mean 'strtoul'?
using ::strtoull _LIBCPP_USING_IF_EXISTS;
      ~~^
/usr/include/stdlib.h:121:3: note: 'strtoul' declared here
         strtoul(const char * __restrict, char ** __restrict, int);
         ^
In file included from helloworld.cpp:1:
In file included from /usr/include/c++/v1/iostream:37:
In file included from /usr/include/c++/v1/ios:215:
In file included from /usr/include/c++/v1/__locale:15:
In file included from /usr/include/c++/v1/string:511:
In file included from /usr/include/c++/v1/string_view:179:
In file included from /usr/include/c++/v1/__string:57:
In file included from /usr/include/c++/v1/algorithm:653:
In file included from /usr/include/c++/v1/memory:670:
In file included from /usr/include/c++/v1/typeinfo:61:
In file included from /usr/include/c++/v1/exception:83:
/usr/include/c++/v1/cstdlib:132:9: error: no member named 'llabs' in the global namespace; did you mean 'labs'?
using ::llabs _LIBCPP_USING_IF_EXISTS;
      ~~^
/usr/include/stdlib.h:104:7: note: 'labs' declared here
long     labs(long) __pure2;
         ^
In file included from helloworld.cpp:1:
In file included from /usr/include/c++/v1/iostream:37:
In file included from /usr/include/c++/v1/ios:215:
In file included from /usr/include/c++/v1/__locale:15:
In file included from /usr/include/c++/v1/string:511:
In file included from /usr/include/c++/v1/string_view:179:
In file included from /usr/include/c++/v1/__string:57:
In file included from /usr/include/c++/v1/algorithm:653:
In file included from /usr/include/c++/v1/memory:670:
In file included from /usr/include/c++/v1/typeinfo:61:
In file included from /usr/include/c++/v1/exception:83:
/usr/include/c++/v1/cstdlib:135:9: error: no member named 'lldiv' in the global namespace; did you mean 'ldiv'?
using ::lldiv _LIBCPP_USING_IF_EXISTS;
      ~~^
/usr/include/stdlib.h:105:9: note: 'ldiv' declared here
ldiv_t   ldiv(long, long) __pure2;
         ^
In file included from helloworld.cpp:1:
In file included from /usr/include/c++/v1/iostream:37:
In file included from /usr/include/c++/v1/ios:215:
In file included from /usr/include/c++/v1/__locale:15:
In file included from /usr/include/c++/v1/string:511:
In file included from /usr/include/c++/v1/string_view:179:
In file included from /usr/include/c++/v1/__string:60:
/usr/include/c++/v1/cwchar:140:9: error: no member named 'wcstoll' in the global namespace
using ::wcstoll _LIBCPP_USING_IF_EXISTS;
      ~~^
/usr/include/c++/v1/cwchar:142:9: error: no member named 'wcstoull' in the global namespace; did you mean 'wcstoul'?
using ::wcstoull _LIBCPP_USING_IF_EXISTS;
      ~~^
/usr/include/wchar.h:170:3: note: 'wcstoul' declared here
         wcstoul(const wchar_t * __restrict, wchar_t ** __restrict, int);
         ^
8 errors generated.

But I think this is because my current clang is still 12.0.1? Was using_if_exists only added after 13.0.0 ?

dim accepted this revision.Sep 2 2021, 4:20 AM

Hmm I have applied this on top of our existing libc++ headers, but if I compile a simple program using e.g. -std=c++98 I immediately get errors due to missing declarations:

...

But I think this is because my current clang is still 12.0.1? Was using_if_exists only added after 13.0.0 ?

Ah yes indeed, with clang 13 it does work (it's a pity there's no fallback attribute). In any case, if FreeBSD updates to the next version of libc++ it will always be in combination with the corresponding version of clang (and the rest of llvm-project), so in practice it shouldn't be problem, at least for clang users.

Maybe this could give issues for gcc users though, as we sometimes tell people to use libc++ headers so as to avoid incompatibilities between linking libstdc++ and libc++ compiled code (which is isn't supported). Does gcc have any using_if_exists attribute? @emaste is the gcc user case important enough? (I can imagine ports maintainers complaining about this)

dim added a comment.Sep 2 2021, 6:27 AM

Ah, gcc doesn't work at all with libc++ before C+11 :) I tried with gcc 12.0.0 20210711, and it spews hundreds of errors, starting off with:

In file included from /usr/include/c++/v1/iostream:36,
                 from helloworld.cpp:1:
/usr/include/c++/v1/__config:238:2: error: #error "libc++ does not support using GCC with C++03. Please enable C++11"
  238 | #error "libc++ does not support using GCC with C++03. Please enable C++11"
      |  ^~~~~
In file included from /usr/include/c++/v1/iostream:36,
                 from helloworld.cpp:1:
/usr/include/c++/v1/__config:445:3: error: #error "We don't know a correct way to implement alignof(T) in C++03 outside of Clang"
  445 | # error "We don't know a correct way to implement alignof(T) in C++03 outside of Clang"
      |   ^~~~~
In file included from /usr/include/c++/v1/iostream:36,
                 from helloworld.cpp:1:

so I guess that use case is out anyway... And in case of >= C++11, we can just assume long long is available, and __LONG_LONG_SUPPORTED will be defined.

ldionne accepted this revision as: Restricted Project.Sep 3 2021, 11:26 AM

Does gcc have any using_if_exists attribute?

Nope, it doesn't. The idea is that if folks want to use GCC, they need to provide a compliant C Standard Library, or else we have to jump through a bunch of hoops (basically the world as it were before using_if_exists).

So if you update libc++ and Clang in lockstep, I don't think this is going to cause you any trouble.

This revision is now accepted and ready to land.Sep 3 2021, 11:26 AM