opj_includes.h shouldn't define __attribute__

[nico]

opj_includes.h has this snippet:

/* Ignore GCC attributes if this is not GCC */
#ifndef __GNUC__
    #define __attribute__(x) /* __attribute__(x) */
#endif

That's not valid, things starting with two underscores are reserved for the implementation and shouldn't be redefined. In practice, it's even actively harmful: When building openjpeg with clang-cl, __GNUC__ isn't defined, so __attribute__ gets defined to nothing, and then the same file includes <intrin.h> a bit futher down. clang-cl's intrin.h header uses __attribute__ internally, and since it's included after this redefine, clang-cl gets very confused.

One approach would be to do

#ifndef __GNUC__
  #define ATTRIB(x) __attribute__(x)
#else
  #define ATTRIB(x)
#endif

And then use ATTRIB everywhere -- but from what I can tell, all uses of __attribute__ are already guarded by other diffs and just deleting these 4 lines should probably be fine too.

(This blocks https://crbug.com/592745)

[stweil]

At least Debian's clang also defines __GNUC__, so I assume that normally there is no problem. clang-cl tries to be compatible to Microsoft's compiler cl which of course does not define __GNUC__. Maybe for this special case #if defined(__GNUC__) || defined(__clang__) would be better.

[nico]

Well, clang defines __GNUC__ when it's in gcc mode. In clang-cl mode, when it emulates cl.exe and not gcc, it doesn't but defines _MSC_VER instead.

For this case, not defining something starting with __ is best, as that's not allowed per standard :-)

(I work on clang.)

[nico]

FWIW, I just removed these 4 lines in the version of openjpeg used in pdfium (the library Chromium uses to render pdf files), and at least on the platforms we build on everything kept working. So maybe just deleting these 4 lines is an option. (https://codereview.chromium.org/1810373002/diff/20001/third_party/libopenjpeg20/opj_includes.h)

[nico]

Thanks! :-)