mm2 / Little-CMS

A free, open source, CMM engine. It provides fast transforms between ICC profiles.
https://www.littlecms.com
MIT License
571 stars 176 forks source link

register keyword is deprecated in C++11 and removed in C++17 #144

Closed VojtechFried closed 6 years ago

VojtechFried commented 6 years ago

When I try to compile our software that uses Little-CMS with a C++17 enabled Visual Studio I get warnings, that "register" keyword was removed from the standard. There are many ways how to deal with this warning, but the easiest for us would be that register usage is removed in lcms headers. What do you think?

mm2 commented 6 years ago

Hi, from the tutorial: "Little CMS 2 is meant to be portable to any C99 compliant compiler."

lcms is C, not C++. It can be compiled as C++, but this is not a requirement. BTW, some C compilers for embedded systems makes extensive use of "register" keyword so I cannot just remove it.

DominikDeak commented 6 years ago

Sorry to bump the issue. However, I do feel that conditionally exposing the register keyword upon complier checking would be very useful -- in the public facing API at least. Little CMS is a great library and thus many C++ projects rely on it. The lcms2.h header already implements tests and macros to accommodate platform specific details, such as CMSAPI, or CMSEXPORT. The register keyword could be given a similar treatment, as shown in the example below:

#ifdef __cplusplus 
#   define CMSREGISTER
#else
#   define CMSREGISTER register
#endif

// Sampling
typedef cmsInt32Number (* cmsSAMPLER16)   (CMSREGISTER const cmsUInt16Number In[],
                                            CMSREGISTER cmsUInt16Number Out[],
                                            CMSREGISTER void * Cargo);

typedef cmsInt32Number (* cmsSAMPLERFLOAT)(CMSREGISTER const cmsFloat32Number In[],
                                            CMSREGISTER cmsFloat32Number Out[],
                                            CMSREGISTER void * Cargo);

I'm happy to create a pull request, if the above strategy is acceptable.

bnason-nf commented 6 years ago

Hi @mm2,

I would also like to see the change suggested by @DominikDeak. The issue for me is that I am compiling a C++ source file using the C++ 17 standard, and it includes lcms2.h, which results in a compile failure. It's possible to work around this with a #define register before the include, but that's pretty ugly, and I'd prefer if this was addressed inside the lcms2 header. Can we make a pull request for this?

Thanks, Ben

DominikDeak commented 6 years ago

It's possible to work around this with a #define register before the include

@bnason-nf: If you are using clang, you can temporarily work-around the problem using one of the methods:

  1. Use the --system-header-prefix=lcms2 switch to tell the compiler that lcms2 is a system header. It will ignore warnings and potential errors emitted by the header. I prefer this option, as it doesn't affect other libraries and code, except lcms2.

  2. Use the -Wno-deprecated-register switch to globally suppress the error. It's not ideal, but at least you don't need to hack fixes directly into your code.

  3. Alternatively, edit your sources directly and use pragmas to suppress the error in clang:

      #pragma clang diagnostic push
      #pragma clang diagnostic ignored "-Wdeprecated-register"
      #include <lcms2.h>
      #pragma clang diagnostic pop

    For GCC:

      #pragma gcc diagnostic push
      #pragma gcc diagnostic ignored "-Wdeprecated-register"
      #include <lcms2.h>
      #pragma gcc diagnostic pop