1057138 - Compiler.h should always define MOZ_MSVC_VERSION_AT_LEAST and MOZ_GCC_VERSION_AT_LEAST

Status: NEW

(Core :: MFBT, defect)

Description

[Seth Fowler [:seth] [:s2h]]

Right now you only get MOZ_MSVC_VERSION_AT_LEAST if MOZ_IS_MSVC is defined, and MOZ_GCC_VERSION_AT_LEAST if MOZ_IS_GCC is defined. We should change that; it's pretty aggravating because you can't write "#if MOZ_IS_MSVC && MOS_MSVC_VERSION_AT_LEAST(11)" since the preprocessor will complain if MOZ_MSVC_VERSION_AT_LEAST is not defined.

To give a concrete example of the costs in terms of readability, I recently found myself writing this code:

> #if MOZ_IS_MSVC
> #  if MOZ_MSVC_VERSION_AT_LEAST(11)
> #    define DECLTYPE(EXPR) decltype(EXPR)
> #  else
>      template<typename T> struct Identity { typedef T type; };
> #    define DECLTYPE(EXPR) Identity<decltype(EXPR)>::type
> #  endif
> #elif MOZ_IS_GCC
> #  if MOZ_GCC_VERSION_AT_LEAST(4,5,0)
> #    define DECLTYPE(EXPR) decltype(EXPR)
> #  else
>      template<typename T> struct Identity { typedef T type; };
> #    define DECLTYPE(EXPR) Identity<decltype(EXPR)>::type
> #  endif
> #else
> #  define DECLTYPE(EXPR) decltype(EXPR)
> #endif

If we always defined the macros mentioned above, this could be rewritten as:

> #if (MOZ_IS_MSVC && !MOZ_MSVC_VERSION_AT_LEAST(11)) || \
>     (MOZ_IS_GCC && !MOZ_GCC_VERSION_AT_LEAST(4,5,0))
>    template<typename T> struct Identity { typedef T type; };
> #  define DECLTYPE(EXPR) Identity<decltype(EXPR)>::type
> #else
> #  define DECLTYPE(EXPR) decltype(EXPR)
> #endif


This is drastically easier to read and write.

Comment 1

[Seth Fowler [:seth] [:s2h]]

Jeff pointed out a downside to this on IRC:

> Waldo: so I resisted this idea initially because it seems it'd have the tendency to let people do checks without first verifying they're on that compiler, as I recall
> Waldo: the particular concern being for gcc version checks, I think, because clang pretends to be gcc 4.x something

To make this safe, we need to ensure that MOZ_MSVC_VERSION_AT_LEAST and MOZ_GCC_VERSION_AT_LEAST always return false is !MOZ_IS_MSVC or !MOZ_IS_GCC respectively.