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

Jiri Malak malak.jiri at gmail.com
Mon Mar 17 14:49:09 EDT 2014


Bred,

I enclosed corrected patches.

Open Watcom didn't use last digit up to now.
For Watcom I looked into history and it was used as patch version. I correct patch to use last
digit as patch version.
I suppress patch number for base version (0 patch) to follow generaly used Watcom/Open Watcom
versions.

I add for second patch more platforms.

Regards

Jiri


> 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
>
> --
>
> Powered by www.kitware.com
>
> Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ
>
> Kitware offers various services to support the CMake community. For more information on each
offering, please visit:
>
> CMake Support: http://cmake.org/cmake/help/support.html
> CMake Consulting: http://cmake.org/cmake/help/consulting.html
> CMake Training Courses: http://cmake.org/cmake/help/training.html
>
> Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html
>
> Follow this link to subscribe/unsubscribe:
> http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Fill-ARCHITECTURE_ID-and-PLATFORM_ID-for-Open-Watcom.patch
Type: text/x-patch
Size: 1354 bytes
Desc: not available
URL: <http://public.kitware.com/pipermail/cmake-developers/attachments/20140317/986cbd46/attachment-0004.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Corrections-for-Open-Watcom-compiler-version.patch
Type: text/x-patch
Size: 4270 bytes
Desc: not available
URL: <http://public.kitware.com/pipermail/cmake-developers/attachments/20140317/986cbd46/attachment-0005.bin>


More information about the cmake-developers mailing list