If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Compiler.h should always define MOZ_MSVC_VERSION_AT_LEAST and MOZ_GCC_VERSION_AT_LEAST

NEW
Unassigned

Status

()

Core
MFBT
3 years ago
3 years ago

People

(Reporter: seth, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

3 years ago
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.
(Reporter)

Comment 1

3 years ago
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.
You need to log in before you can comment on or make changes to this bug.