[cmake-developers] [PATCH] Correct platform related variables for Open Watcom compilers

[Brad King]

On 03/14/2014 02:27 PM, Jiri Malak wrote:
> I enclosed both patches.

Thanks.  Here are some more comments.

The version extraction logic is currently:

> +   /* __WATCOMC__ = VVRR + 1100 Open Watcom */
> +#  define COMPILER_VERSION_MAJOR DEC((__WATCOMC__ - 1100 ) / 100)
> +#  define COMPILER_VERSION_MINOR DEC((__WATCOMC__ / 10) % 10)

The comment does not match the implementation.  It looks like
the least-significant digit is not part of the minor version
number according to the implementation.  The comment should
then read "VVR0" instead of "VVRR".  What is the purpose of
the least-significant digit in Open Watcom?  Is it a patch level?

I see several updates of the form:

> -if ("${CMAKE_CXX_COMPILER_ID}" MATCHES Watcom)
> +if ("${CMAKE_CXX_COMPILER_ID}" MATCHES Watcom
> +    OR "${CMAKE_CXX_COMPILER_ID}" MATCHES OpenWatcom)

but the MATCHES test is already a regex so you do not need
to test both variants.  Most of these updates can be dropped.

There is a lingering problem from the old code that this
hunk indents:

> +    if("${_compiler_version}" LESS 1.7)
> +      set(WATCOM16 1)
> +    endif()
> +    if("${_compiler_version}" EQUAL 1.7)
> +      set(WATCOM17 1)
> +    endif()
> +    if("${_compiler_version}" EQUAL 1.8)
> +      set(WATCOM18 1)
> +    endif()
> +    if("${_compiler_version}" EQUAL 1.9)
> +      set(WATCOM19 1)
> +    endif()

The version comparisons should use VERSION_LESS and
VERSION_EQUAL to get component-wise tests.

In InstallRequiredSystemLibraries:

> +  if(_compiler_id MATCHES "Watcom")
> +    math(EXPR _watcom_minor "${_watcom_minor} / 10")

should use STREQUAL instead of MATCHES or OpenWatcom will match.

Thanks,
-Brad