Closed Bug 906783 Opened 6 years ago Closed 6 years ago

Detect standard library in mfbt/Compiler.h

Categories

(Core :: MFBT, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla26

People

(Reporter: jcranmer, Assigned: jcranmer)

References

Details

Attachments

(1 file, 1 obsolete file)

Attached patch Attempt to discover STL versions (obsolete) — Splinter Review
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?
Attachment #792302 - Flags: feedback?(jwalden+bmo)
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? :-/ ]
I'd like to test it on OpenBSD but it doesnt apply as-is...
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?
Attachment #792302 - Flags: feedback?(jwalden+bmo) → feedback+
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
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).
Attachment #792302 - Attachment is obsolete: true
Attachment #796795 - Flags: review?(jwalden+bmo)
Attachment #796795 - Flags: feedback?(landry)
seems to build fine w/ clang 3.3/libstdc++ 4.2. Will try with gcc/libstdc++ 4.6..
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)
Attachment #796795 - Flags: feedback?(landry) → feedback+
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.
Attachment #796795 - Flags: review?(jwalden+bmo) → review+
Depends on: 911140
https://hg.mozilla.org/mozilla-central/rev/195e0c04ae28
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
You need to log in before you can comment on or make changes to this bug.