906783 - Detect standard library in mfbt/Compiler.h

Status: RESOLVED FIXED

(Core :: MFBT, defect)

Description

[Joshua Cranmer [:jcranmer]]

Attachment: Attempt to discover STL versions

We support 3 compilers and 4 standard libraries in an insane matrix of support for each other, and trying to figure out who is usable where is difficult. Just look at the complexity of Atomics.h for proof. For simplicity, let's just define macros to detect the version of the standard library to know when we can add support for somebody.

Except standard library authors hate us and don't provide neat, simple macros (except for STLport). For libc++, I'm going to assume that we'd require a new enough version of libc++ that version checking won't be necessary. For libstdc++, I'm assuming that gcc version = libstdc++, and if not, then I grepped through the different versions of gcc to find new macros added to bits/c++config.h and dropped those in. Hopefully the C++ feature-test recommendations get supported by everybody so newer versions won't require insane hacks like this.

This doesn't actually work because apparently our systems header wrapper thing is too dumb to figure out how to include <cstddef> properly?

Comment 1

[Joshua Cranmer [:jcranmer]]

Oh, on further inspection, the issue was that we were trying to include <cstddef> in a C file, which doesn't work at all.

[Why do we have C files including MFBT? :-/ ]

Comment 2

[Landry Breuil (:gaston)]

I'd like to test it on OpenBSD but it doesnt apply as-is...

Comment 3

[Jeff Walden [:Waldo]]

Comment on attachment 792302 [details] [diff] [review]
Attempt to discover STL versions

Review of attachment 792302 [details] [diff] [review]:
-----------------------------------------------------------------

This seems not the worst thing in the world at first glance, although to be honest I'm not sure my instincts here constitute a reliable indicator of sensibility.  Probably worth asking a few other mfbt-patching people about whether or not this is a not-that-bad idea.

::: mfbt/Compiler.h
@@ +59,5 @@
> + * major/minor combinations by looking for newly-defined config macros.
> + */
> +#  if MOZ_IS_GCC
> +#    define MOZ_LIBSTDCXX_VERSION_AT_LEAST(major, minor, patch) \
> +        MOZ_GCC_VERSION_AT_LEAST

This definition looks wrong.

@@ +62,5 @@
> +#    define MOZ_LIBSTDCXX_VERSION_AT_LEAST(major, minor, patch) \
> +        MOZ_GCC_VERSION_AT_LEAST
> +#  elif defined(_GLIBCXX_THROW_OR_ABORT)
> +#    define MOZ_LIBSTDCXX_VERSION_AT_LEAST(major, minor, patch) \
> +        ((major) > 4 || (minor) >= 8)

Presumably this wants to be

  ((major) > 4 || ((major) == 4 && (minor) >= 8))

and all the others want changes along similar lines?

Comment 4

[Jan Beich]

Comment on attachment 792302 [details] [diff] [review]
Attempt to discover STL versions

Review of attachment 792302 [details] [diff] [review]:
-----------------------------------------------------------------

::: mfbt/Compiler.h
@@ +41,5 @@
> +#ifdef _STLPORT_MAJOR
> +#  define MOZ_USING_STLPORT 1
> +#  define MOZ_STLPORT_VERSION_AT_LEAST(major, minor, patch) \
> +     (_STLPORT_VERSION >= (major) << 8 | (minor) << 4 | (patch))
> +#elif defined(__LIBCPP_VERSION)

libc++ macros have only one underscore 

http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/__config?view=co

Comment 5

[Joshua Cranmer [:jcranmer]]

Attachment: Attempt to discover STL versions

The most confusing part is that the libstdc++ version guessing list looks different from the "normal" form of version macros.

Passes try: <https://tbpl.mozilla.org/?tree=Try&rev=0fc6a1e37d7e> (not that says a whole lot here).

Comment 6

[Landry Breuil (:gaston)]

seems to build fine w/ clang 3.3/libstdc++ 4.2. Will try with gcc/libstdc++ 4.6..

Comment 7

[Landry Breuil (:gaston)]

Comment on attachment 796795 [details] [diff] [review]
Attempt to discover STL versions

also builds fine with gcc/libstdc++ 4.6.4 (still on OpenBSD, ofc)

Comment 8

[Jeff Walden [:Waldo]]

Comment on attachment 796795 [details] [diff] [review]
Attempt to discover STL versions

Review of attachment 796795 [details] [diff] [review]:
-----------------------------------------------------------------

::: mfbt/Compiler.h
@@ +56,5 @@
> +   /*
> +    * libstdc++ is also annoying and doesn't give us useful versioning macros
> +    * for the library. If we're using gcc, then assume that libstdc++ matches
> +    * the compiler version. If we're using clang, we're going to have to fake
> +    * major/minor combinations by looking for newly-defined config macros.

Gnarly.

Comment 9

[Joshua Cranmer [:jcranmer]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/195e0c04ae28

Comment 10

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/195e0c04ae28